This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, Oct 29, 2015 at 02:50:54PM -0700, Paul Eggert wrote:
> 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.

I don't think this needs to be explicit. There is a general principle,
which needs to be promoted rather than undermined, that a function
cannot have side effects/alter the value of objects outside of its
documented behavior.

> * 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.

I'm not opposed to making a more visible warning, but the
documentation of the return value should make it clear. Requiring the
argument to point to "a string" also expresses the null termination
requirement. This is the same language used in the C standard for
string functions.

> * 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.

The function should only be exposed at all when extensions are exposed
(_DEFAULT_SOURCE/_ALL_SOURCE) but __-prefixed functions should never
be a public API intended for applications to use. Also exposing the
function with a name different from widespread usage would not help
application portability.

> * 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))).

No, the fortify version should just abort when this contract is
broken. Invalid usage contrary to documentation should not be silently
supported. This encourages non-portable programs which would break on
other strl* implementations.

Rich


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]