Re: [PATCH v7] Implement strlcpy, strlcat [BZ #178]

[Florian Weimer <fweimer at redhat dot com>]

On 12/30/2015 09:31 AM, Paul Eggert wrote:

>> One suggestion is to make a non-terminated buffer undefined, but that
>> breaks
>> the snprintf analogy for size 0 inputs.
> 
> 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), 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).

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.

I'm attaching a new version with the proposed manual edits, on top of
your version.

Has anyone reviewed the current implementation and has comments about that?

Thanks,
Florian

diff --git a/manual/string.texi b/manual/string.texi
index 65166b8..f0fe7d3 100644
--- a/manual/string.texi
+++ b/manual/string.texi
@@ -1118,19 +1118,23 @@ If @var{size} is nonzero and less than or equal to the the length of the string
 bytes to the destination array @var{to}, and writes a terminating null
 byte to the last byte of the array.
 
+If @var{size} is zero, @code{strlcpy} does not modify the destination
+array, and @var{to} can be a null pointer.
+
 The return value @var{result} of @code{strlcpy} is the length of the
 string @var{from}.  This means that @samp{@var{result} >= @var{size}} is
 true whenever truncation occurs.
 
-The behavior of @code{strlcpy} is undefined if @var{size} is zero, or
-if the source and destination strings overlap.
+The behavior of @code{strlcpy} is undefined if @var{size} is nonzero and
+the source string and the first @var{size} bytes of the destination
+array overlap.
 
 As noted below, this function is generally a poor choice for processing
 text.  Unlike @code{strncpy}, @code{strlcpy} requires @var{size} to be
 nonzero and the source string to be null-terminated, computes the
 source string's length, ensures that the destination is
-null-terminated, and does not fill the remaining part of the destination
-with null bytes.
+null-terminated (assuming that @var{size} is nonzero), and does not
+fill the remaining part of the destination with null bytes.
 
 This function is derived from BSD.
 @end deftypefun
@@ -1154,9 +1158,9 @@ This function returns the sum of the original length of @var{to} and
 the length of @var{from}.  This means that truncation occurs unless
 the returned value is less than @var{size}.
 
-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.
+The behavior is undefined if the array at @var{to} does not contain a
+null byte in its first @var{size} bytes, or if the source string and the
+first @var{size} bytes of @var{to} overlap.
 
 As noted below, this function is generally a poor choice for processing
 text.  Also, this function has significant performance issues.