Re: [brlcad-devel] comgeom-g bug

[Christopher S. M. <br...@ma...>]

On Apr 6, 2012, at 5:29 PM, Tom Browder wrote:

>> That bu_strlcpy() function is basically an implementation/wrapper for strlcpy(). It IS still a safer way to copy strings than strncpy() simply because it ensures NULL termination and doesn't require the caller to inconsistently subtract a byte for null -- they just pass the actual size.
> 
> Hm, it looks to me like you have to pass size+1 as it is now (i.e.,
> include the null in the string and count it in length), see this in
> bu_strlcpy:

The "actual size" is the actual memory size of the destination, not the string size:

const char *str = "my_string";
char buf[10];
bu_strlcpy(buf, str, sizeof(buf);

Therein was the crux of strncpy()'s error-proneness.  It was the size of the string/bytes to copy, which creates quite a conundrum when that size doesn't accommodate a \0 byte.

> What would be wrong in changing that to:
> 
>   } else {
>       dst[size] = '\0'; /* sanity */
>   }

That would set buf[10] = '\0' which would be a segfault.  We set size-1, i.e. array index size-1=9 in the example, to '\0'.

> (except that it wouldn't mimic strlcpy exactly, except that youe
> aren;t anyway: "Note that a byte for the NUL should be included in
> size")

I'm not following.  We do mimic strlcpy() (in fact we just call it if the compilation supports it).  It'd be a bug if we didn't, but I'd like to see a code snippet that demonstrates it.  We have unit test for that one due to how tricky it was to get right in the first place and they indicate we're (finally) doing the right thing now.

Cheers!
Sean