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

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/17/2015 09:07 AM, Florian Weimer wrote:
> I'm attaching what I hope is the final version.
> 
> Regarding attribution in NEWS, we can add that once strlcat is committed. :)

v6 looks perfect to me.

I think this should get committed and strlcat worked on next.

Have all objections to this patch been addressed?

> diff --git a/debug/strlcpy_chk.c b/debug/strlcpy_chk.c
> index a13bcfe..94bc0a2 100644
> --- a/debug/strlcpy_chk.c
> +++ b/debug/strlcpy_chk.c
> @@ -19,6 +19,8 @@
>  #include <string.h>
>  #include <memcopy.h>
>  
> +/* Check that the user-supplied size does not exceed the
> +   compiler-determined size, and then forward to strlcpy.  */

OK.

>  size_t
>  __strlcpy_chk (char *__restrict s1, const char *__restrict s2,
>  	       size_t n, size_t s1len)
> diff --git a/manual/string.texi b/manual/string.texi
> index b7f94c5..ce4880a 100644
> --- a/manual/string.texi
> +++ b/manual/string.texi
> @@ -590,29 +590,31 @@ destination buffer does not have length zero.
>  @comment string.h
>  @comment BSD
>  @deftypefun size_t strlcpy (char *restrict @var{to}, const char *restrict @var{from}, size_t @var{size})
> -@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> -This function is similar to @code{strcpy}, but copies at most @var{size}
> -characters into @var{to}, including the terminating null character.
> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} This function is similar
> +to @code{strcpy}, but copies at most @var{size} characters into the
> +destination buffer @var{to}, including the terminating null character.
> +
> +If @var{size} is zero, nothing is written to @var{to}.
>  
> -If the length of @var{from} is equal to or more than @var{size}, then
> -@code{strlcpy} copies just the first @samp{@var{size} - 1} characters.
> -As a special case, if @var{size} is zero, no bytes are written to
> -@var{to}.
> +If the length of the string @var{from} is equal to or greater than
> +@var{size}, @code{strlcpy} copies only the first @samp{@var{size} - 1}
> +characters to the destination buffer @var{to}, and writes a terminating
> +null character to the last character in the buffer.

OK. Perfect, this is exactly what I wanted to see here.

> -If the length of @var{from} is less than @var{size}, then @code{strlcpy}
> -copies all of @var{from}, followed by a single null character.  Like
> -other string functions such as @code{strcpy}, but unlike @code{strncpy},
> -the remaining characters in the destination buffer, if any, remain
> -unchanged.
> +If the length of @var{from} is less than @var{size}, @code{strlcpy}
> +copies all of the string @var{from} to the destination buffer @var{to},
> +followed by a single null character.  Like other string functions such
> +as @code{strcpy}, but unlike @code{strncpy}, the remaining characters in
> +the destination buffer, if any, remain unchanged.

OK.

>  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 string @var{from} must be null-terminated even if its length exceeds
> -that of the destination buffer.  The behavior of @code{strlcpy} is
> -undefined if the strings overlap or if the source or destination are
> -null pointers.
> +that of the destination buffer specified as @var{size}.  The behavior of
> +@code{strlcpy} is undefined if the strings overlap or if the source or
> +destination are null pointers.

OK.

>  Using @code{strlcpy} as opposed to @code{strcpy} is a way to avoid bugs
>  relating to writing past the end of the allocated space for @var{to}.
> diff --git a/string/tst-strlcpy.c b/string/tst-strlcpy.c
> index b97f591..77b7677 100644
> --- a/string/tst-strlcpy.c
> +++ b/string/tst-strlcpy.c
> @@ -35,24 +35,34 @@ do_test (void)
>      char buf2[16];
>    } s;
>  
> +  /* Nothing is written to the destiantion if its size is 0.  */
>    memset (&s, '@', sizeof (s));
>    CHECK (strlcpy (s.buf1, "Hello!", 0) == 6);
>    CHECK (memcmp (&s, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@", sizeof (s)) == 0);
>  
> +  /* No bytes are are modified in the target buffer if the source
> +     string is short enough.  */
>    memset (&s, '@', sizeof (s));
>    CHECK (strlcpy (s.buf1, "Hello!", sizeof (s.buf1)) == 6);
>    CHECK (memcmp (&s, "Hello!\0@@@@@@@@@@@@@@@@@@@@@@@@@", sizeof (s)) == 0);
>  
> +  /* A source string which fits exactly into the destination buffer is
> +     not truncated.  */
>    memset (&s, '@', sizeof (s));
>    CHECK (strlcpy (s.buf1, "Hello, world!!!", sizeof (s.buf1)) == 15);
>    CHECK (memcmp (&s, "Hello, world!!!\0@@@@@@@@@@@@@@@@@@@@@@@@@",
>  		 sizeof (s)) == 0);
>  
> +  /* A source string one character longer than the destination buffer
> +     is truncated by one character.  The untruncated source length is
> +     returned.  */
>    memset (&s, '@', sizeof (s));
>    CHECK (strlcpy (s.buf1, "Hello, world!!!!", sizeof (s.buf1)) == 16);
>    CHECK (memcmp (&s, "Hello, world!!!\0@@@@@@@@@@@@@@@@@@@@@@@@@",
>  		 sizeof (s)) == 0);
>  
> +  /* An even longer source string is truncated as well, and the
> +     original length is returned.  */
>    memset (&s, '@', sizeof (s));
>    CHECK (strlcpy (s.buf1, "Hello, world!!!!!!!!", sizeof (s.buf1)) == 20);
>    CHECK (memcmp (&s, "Hello, world!!!\0@@@@@@@@@@@@@@@@@@@@@@@@@",

OK.

Cheers,
Carlos.