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

[ricaljasan <ricaljasan at pacific dot net>]

On 11/17/2015 06:07 AM, Florian Weimer wrote:
> diff --git a/manual/string.texi b/manual/string.texi
> index 5f8a17e..ce4880a 100644
> --- a/manual/string.texi
> +++ b/manual/string.texi
> @@ -576,17 +576,57 @@ there is no null terminator written into @var{to}.
>  
>  If the length of @var{from} is less than @var{size}, then @code{strncpy}
>  copies all of @var{from}, followed by enough null characters to add up
> -to @var{size} characters in all.  This behavior is rarely useful, but it
> -is specified by the @w{ISO C} standard.
> +to @var{size} characters in all.
>  
>  The behavior of @code{strncpy} is undefined if the strings overlap.
>  
> -Using @code{strncpy} as opposed to @code{strcpy} is a way to avoid bugs
> +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 which null-terminates the destination string as long as the
> +destination buffer does not have length zero.
> +@end deftypefun
> +
> +@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

The beginning of this line should be moved below the @safety macro.

> +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 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.
> +
> +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.
> +
> +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 specified as @var{size}.  The behavior of
> +@code{strlcpy} is undefined if the strings overlap or if the source or
> +destination are null pointers.
> +
> +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}.
> -However, it can also make your program much slower in one common case:
> -copying a string which is probably small into a potentially large buffer.
> -In this case, @var{size} may be large, and when it is, @code{strncpy} will
> -waste a considerable amount of time copying null characters.
> +Unlike @code{strncpy}, @code{strlcpy} ensures that the destination
> +string is always null-terminated (unless the buffer size is zero), and
> +it does not fill the remaining part of the buffer with null characters.
> +
> +@strong{Note:} GNU programs should not use statically sized buffers for
> +storing strings.  @xref{Semantics, , Writing Robust Programs, standards,
> +The GNU Coding Standards}.  Instead of using @code{strlcpy}, it is
> +usually better to use dynamic memory allocation and functions such as
> +@code{strdup} or @code{asprintf} to construct strings.

A reference to dynamic memory allocation would be suitable here.

>  @end deftypefun
>  
>  @comment wchar.h
> 

Overall, I like this explanation.

> diff --git a/string/tst-strlcpy.c b/string/tst-strlcpy.c
> new file mode 100644
> index 0000000..77b7677
> --- /dev/null
> +++ b/string/tst-strlcpy.c
> @@ -0,0 +1,76 @@

> +#define CHECK(cond)					\
> +  if (!(cond))						\
> +    {							\
> +      printf ("%s:%d: FAIL\n", __FILE__, __LINE__);	\
> +      exit (1);						\
> +    }

The manual taught me to use `exit (EXIT_FAILURE);', but it seems that is
not in vogue:

$ for a in 1 EXIT_FAILURE; do find -name '*.c' | xargs grep "exit ($a)"
| wc -l; done
1896
114

Rical