This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v7] Implement strlcpy, strlcat [BZ #178]
- From: Alexander Cherepanov <ch3root at openwall dot com>
- To: Florian Weimer <fweimer at redhat dot com>, Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 5 Jan 2016 03:02:44 +0300
- Subject: Re: [PATCH v7] Implement strlcpy, strlcat [BZ #178]
- Authentication-results: sourceware.org; auth=none
- References: <5682DD7E dot 6000301 at redhat dot com> <56839678 dot 8040304 at cs dot ucla dot edu> <568ADC5F dot 5010608 at redhat dot com>
On 2016-01-04 23:55, Florian Weimer wrote:
One suggestion is to make a non-terminated buffer undefined, but that
breaks
the snprintf analogy for size 0 inputs.
What happens in this case is more or less documented but "in practice
this should not happen as it means that either dstsize is incorrect or
that dst is not a proper string"[1] so it's not clear if glibc should
support it officially. Making it UB seems to be a valid choice.
[1]
http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/strlcat.3?query=strlcpy&sec=3#x4445534352495054494f4e
Sorry, what analogy is that? snprintf does not concatenate to a buffer
directly. What is the practical use case here? How does the use case
ignore the principle that strlcpy should null-terminate its output?
We can probably ditch the size-0 documented special case for strlcat
(where it is just extremely confusing and not very helpful),
Yeah, this is strange. The full analogy with snprintf is documented for
strlcpy only but I don't think it's implied for strlcat.
Plus, the implementation[2] in openbsd (the most upstream IIUC) does
pointer arithmetic on the destination pointer which is UB for NULL. I
guess this means that dst=NULL is not supposed to be supported.
[2]
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/strlcat.c?rev=1.16&content-type=text/x-cvsweb-markup
but not for
strlcpy, where it is part of the specification.
Apart from that, my only objection to your strlcpy text is this:
+The behavior of @code{strlcpy} is undefined if @var{size} is zero, or
+if the source and destination strings overlap.
I think it should say âif the source string and destination array
overlapâ. There is no destination string before the call. I don't
think it's necessary to make the special case defined where the source
string is part of the destination array, but not located close to the
beginning so that the actual copy will not overlap. This is simply too
subtle. I think it is undefined for snprintf (âIf copying takes place
between objects that overlap, the behavior is undefined.â),although
this does not necessarily follow from the restrict qualify (based on my
somewhat shaky understanding of restrict).
IIUC you are saying that this code:
char s[10];
snprintf(s, -1, "%s", "abc");
is invalid. IMHO this is wrong. The description of snprintf in C11
doesn't mention size of the destination array or talk about any
connection between n and s at all, it just says that characters beyond
the (n-1)st are discarded.
I think both strlcpy and strlcat should work just fine with size=-1.
This is not a subtle point.
I would like to make a similar change to your strlcat text:
+The behavior is undefined if @var{to} does not contain a null byte in
+its first @var{size} bytes, or if the source and resulting destination
+strings overlap.
There, it does make sense to speak of a string, but supporting the
special case where the source overlaps with the destination array, but
not the part that is written, is again very subtle and difficult to tell
apart from the regular overlap.
Yeah, something like this:
char *s = "abc";
strlcat(s, s, 2);
:-)
--
Alexander Cherepanov