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

[Alexander Cherepanov <ch3root at openwall dot com>]

On 2015-12-29 22:22, Florian Weimer wrote:
> +size_t
> +strlcat (char *__restrict dest, const char *__restrict src, size_t size)
> +{
> +  size_t src_length = strlen (src);
> +
> +  /* Our implementation strlcat supports dest == NULL if size == 0
> +     (for consistency with snprintf and strlcpy), but strnlen does
> +     not, so we have to cover this case explicitly.  */
> +  if (size == 0)
> +    return src_length;
> +
> +  size_t dest_length = __strnlen (dest, size);
> +  if (dest_length != size)
> +    {
> +      /* Copy at most the remaining number of characters in the
> +	 destination buffer.  Leave for the NUL terminator.  */
> +      size_t to_copy = size - dest_length - 1;
> +      /* But not more than what is available in the source string.  */
> +      if (to_copy > src_length)
> +	to_copy = src_length;
> +
> +      char *target = dest + dest_length;
> +      memcpy (target, src, to_copy);
> +      target[to_copy] = '\0';
> +    }
> +
> +  /* The sum cannot wrap around because both strings would be larger
> +     than half of the address space, which is not possible due to
> +     the restrict qualifier.  */
> +  _Static_assert (sizeof (uintptr_t) == sizeof (size_t),
> +		  "theoretical maximum object size covers address space");
> +  return dest_length + src_length;
> +}

Is there any reason why it doesn't just call strlcpy? Or, if calling a 
function is not desirable, why it doesn't just mirror implementation of 
strlcpy? Is added complexity worth it?

> +size_t
> +strlcpy (char *__restrict dest, const char *__restrict src, size_t size)
> +{
> +  size_t src_length = strlen (src);
> +
> +  if (__glibc_unlikely (src_length >= size))
> +    {
> +      if (size > 0)
> +	{
> +	  /* Copy the leading portion of the string.  The last
> +	     character is subsequently overwritten with the NUL
> +	     terminator, but the destination size is usually a
> +	     multiple of a small power of two, so writing it twice
> +	     should be more efficient than copying an odd number of
> +	     bytes.  */
> +	  memcpy (dest, src, size);
> +	  dest[size - 1] = '\0';
> +	}
> +    }
> +  else
> +      /* Copy the string and its terminating NUL character.  */
> +      memcpy (dest, src, src_length + 1);
> +  return src_length;
> +}

This looks good to me in that it uses minimal assumptions about its 
input and should work fit any reasonable description you with Paul come 
up later.

--
Alexander Cherepanov