[brlcad-devel] comgeom-g bug

[Tom B. <tom...@gm...>]

I think I've found the problem, but I think it may not all be
comgeom-g's fault but rather a confusing libbu function:

Look at function bu_strlcpym.  It looks like it's supposed to be a
safe version of strncpy/strlcpy by always ensuring a null-terminated
string, but I think a casual user might not realize the consequences,
to wit:

  bu_strcpym(char *dst, const char *src, size_t size, const char *label);

Inside the function the length of src is taken:

  srcsize = strlen(src);

then:

    /* be sure to always null-terminate, contrary to strncpy behavior */
    if (srcsize < size - 1) {
	dst[srcsize] = '\0';
    } else {
	dst[size-1] = '\0'; /* sanity */
    }

The function is used throughout comgeom-g and the size argument looks
like it usually is the intended length of the string to be copied
since we are copying chunks of characters out of an input string and
no null in the vicinity, so an immediate fix is to increase the size
arg by 1 but that isn't intuitive to me.

I would prefer a function that did this inside:

    /* be sure to always null-terminate, contrary to strncpy behavior */
    if (srcsize < size) {
	dst[srcsize] = '\0';
    } else {
	dst[size] = '\0'; /* sanity */
    }

Note that the supposedly safer bu_strlcpym still has the other strlcpy
bug of not checking the destination for sufficient size.

I'm still looking for a similar existing function but am open to suggestions.

Hm, what about using a bu_vls!

Best regards,

-Tom