Re: [PATCH v4 02/11] lib/charset: add u16_strlcat() function

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 3/24/22 14:54, Masahisa Kojima wrote:
> Provide u16 string version of strlcat().
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v4:
> - add blank line above the return statement
>
> Changes in v2:
> - implement u16_strlcat(with the destination buffer size in argument)
>    instead of u16_strcat
>
>   include/charset.h | 15 +++++++++++++++
>   lib/charset.c     | 21 +++++++++++++++++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/include/charset.h b/include/charset.h
> index b93d023092..dc5fc275ec 100644
> --- a/include/charset.h
> +++ b/include/charset.h
> @@ -259,6 +259,21 @@ u16 *u16_strcpy(u16 *dest, const u16 *src);
>    */
>   u16 *u16_strdup(const void *src);
>
> +/**
> + * u16_strlcat() - Append a length-limited, %NUL-terminated string to another
> + *
> + * Append the src string to the dest string, overwriting the terminating
> + * null word at the end of dest, and then adds a terminating null word.
> + * It will append at most size - u16_strlen(dst) - 1 bytes, NUL-terminating the result.

Why "- 1"?

If size is even, we append up to size - u16_strlen(dst) - 2 bytes. The
two extra bytes used for 0x0000.
If size is odd, we append up to size - u16_strlen(dst) - 3 bytes leaving
one byte of the buffer unused.

> + *
> + * @dest:		destination buffer (null terminated)
> + * @src:		source buffer (null terminated)
> + * @size:		destination buffer size in bytes

s/$/ including the trailing 0x0000/

> + * Return:		total size of the created string in bytes.
> + *			If return value >= size, truncation occurred.
> + */
> +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size);
> +
>   /**
>    * utf16_to_utf8() - Convert an utf16 string to utf8
>    *
> diff --git a/lib/charset.c b/lib/charset.c
> index f44c58d9d8..47997eca7d 100644
> --- a/lib/charset.c
> +++ b/lib/charset.c
> @@ -428,6 +428,27 @@ u16 *u16_strdup(const void *src)
>   	return new;
>   }
>
> +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size)
> +{

If you start the function with

     size >>= 1;

or

     size /= sizeof(u16);

this might simplify the code.

> +	size_t dstrlen = u16_strnlen(dest, size >> 1);
> +	size_t dlen = dstrlen * sizeof(u16);
> +	size_t len = u16_strlen(src) * sizeof(u16);
> +	size_t ret = dlen + len;

This misses the  trailing 0x0000.

Best regards

Heinrich

> +
> +	if (dlen >= size)
> +		return ret;
> +
> +	dest += dstrlen;
> +	size -= dlen;
> +	if (len >= size)
> +		len = size - sizeof(u16);
> +
> +	memcpy(dest, src, len);
> +	dest[len >> 1] = u'\0';
> +
> +	return ret;
> +}
> +
>   /* Convert UTF-16 to UTF-8.  */
>   uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size)
>   {