Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

It doesn’t make any sense.

The way a string is terminated in C is by a null character. ‘\0’ is a mnemonic. strlen() returns the length of a strong. It does this by doing a linear scan down the array until it finds the null, and the returns that index as the length.

That line of code does nothing useful. It means, “scan through the array until you find a null, then write a null there.” Which is exactly what you had to start.

So why would someone write this? Well, they were told to make sure that you always terminate your C strings with a null, otherwise you’ll run off the end of the array and corrupt memory. This is true, but this isn’t how you need to find the end of the array. They should have used the number from strlen(src), not strlen(dest). Even worse, strncpy() will put the null terminator in dest in this case.

The line belies any sense that the programmer had any understanding of what the code was actually doing. It’s a amateur mistake.



using strlen(src) is also an amateur mistake, because the whole point of doing that (stncpy family, ensuring dst ends with null) is that dst might not be large enough to fit string from src.

It has to be the size of dst, which can only be known at the site of allocation in C (except for a non-clean way of looking at heap metadata).


There’s no reason to believe from these two lines that the memory is insufficiently allocated. It’s just two lines. The point wasn’t to share a safe string copy routine, the point of the post was to illustrate a single mistake.


I'm not assuming that, but then you can't assume the memory is sufficient either. If the string is from network or user, then you can never assume the buffer to be of sufficient size (which they definitely do, because they aren't tracking how much of src was copied into allocated dst).


Each line, individually, is total nonsense.

It never makes sense to use strlen() for an argument to strncpy(). Only the size of the destination buffer makes sense for strncpy() (usually sizeof(dst)). For other cases you would probably use memcpy().


pace @zeusk:

I thought the point was that they didn't know if the dest is long enough to copy source, so they copy as much of it as they can.

The reason for the '\0' is in the case that src is shorter than dest.

A more explicit way may be to strlen dest > src then just use lenght(src) otherwise use str(dest).

But this might actually be more efficient.

Edit: And in this case Spolsky doesn't apply because it seems the developer doesn't get to allocate the dest length.


Well...

If dst is larger than src, the strcpy* family of functions will also copy over the null byte.

If dst is shorter than src, and if strlen(src) is used as arg for strncpy then the function will overflow in dst and your null byte will also be outside dst buffer. In hardened environments you wouldn't trust strings to contain null byte if it comes from network or user and use strlcpy with known size of dst then append a null byte at end of dst anyway to ensure it is null terminated.

Using strlen to compare buffer sizes is totally wrong and is the source for many bugs (hint, strlen doesn't actually return size of anything but the distance of the first null byte from the address you provide it).


Thanks. Great explanation.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: