Re: [PATCH v3] Implement strlcpy [BZ #178]

[Paul Eggert <eggert at cs dot ucla dot edu>]

I'd rather we didn't add strlcpy to glibc, for reasons already 
discussed. If consensus goes against me, the patch still needs some 
work. I don't recall the details of our previous discussion; at the risk 
of reraising old issues here are some comments:

* If strlen(SRC) < DESTLEN, the documentation should clearly state that 
the contents of the bytes DEST[strlen(SRC) + 1] through DEST[DESTLEN - 
1] are preserved.  The current documentation can be plausibly read that 
way, but it's not explicit.

* The proposed documentation can easily be misread as implying that 
strlcpy (DEST, SRC, DESTLEN) does O(DESTLEN) work, which is incorrect: 
strlcpy always does O(strlen(SRC)) work.  This point should be made 
clearly.  This is not merely a performance issue: it should be made 
crystal-clear that SRC must be null-terminated even if the source's 
trailing null byte is way after the bytes that strlcpy copies to DEST.

* strlcpy's name should be prefixed by '__' by default. The names 
strlcpy and strlcpy_chk are both in the implementation namespace, and 
it's odd to have one without leading underscores and the other with 
them. I suggest a more cautious approach, in which both names are 
prefixed with '__' and unprefixed strlcpy is provided to the user only 
when GNU or BSD extensions are requested via _GNU_SOURCE etc. This is 
less likely to break existing applications. It's true that POSIX and the 
C standard allow glibc to add str* names of all sorts, but it doesn't 
hurt to be realistic here about the user code that's undoubtedly out there.

* strlcpy's implementation should use memmove instead of memcpy. The 
main motivations for strlcpy are safety and consistency and avoiding 
errors.  memmove obviously supports these goals better than memcpy 
does.  Efficiency is not a major concern with strlcpy (if it were, 
strlcpy wouldn't be O(strlen(SRC))).

> +Not guaranteeing null termination and always overwriting the entire
> +destination buffer makes @code{strncpy} rarely useful, but this behavior
> +is specified by the @w{ISO C} standard.  See @code{strlcpy} below for an
> +alternative.
This quote is confusing, as it imples that strlcpy guarantees null 
termination, which strlcpy does not. I suggest rewording it to something 
like the following: "Often @code{strncpy} is not what you want, because 
it does not null-terminate the destination if the destination is smaller 
than the source, it always overwrites the entire destination buffer, it 
may truncate the destination, and it has undefined behavior if the 
source and destination overlap.  For alternatives, see the documentation 
below for @code{strlcpy}."

We can pair this with similar phrasing under strlcpy -- something like 
the following perhaps (this wording assumes strlcpy is changed to use 
memmove):"Often @code{strlcpy} is not what you want, because it does not 
null-terminate the destination if the destination's size is zero,it can 
leave junk data behind in the destination, it can do useless work when 
the source is long and the destination short, and it may truncate the 
destination.  Although one alternative is @code{strncpy}, it is usually 
better to use dynamic memory allocation and functions such as 
@code{strdup} or @code{asprintf} to construct strings."