[PATCH 2/2] wcrtomb: Make behavior POSIX compliant

[Paul Eggert]

On 5/5/22 11:43, Siddhesh Poyarekar wrote:
> +      else if (__glibc_unlikely (result > 1))
> +	*((uint16_t *) s) = *((uint16_t *) buf);

Shouldn't this be protected by "#if _STRING_ARCH_unaligned"? Also, 
unnecessary parens.

But better yet, just call memcpy as the compiler will figure it out; see 
below.


> -    result = data.__outbuf - (unsigned char *) s;
> +    result = data.__outbuf - (unsigned char *) buf;
>    else
>      {
>        result = (size_t) -1;
>        __set_errno (EILSEQ);
>      }
>  
> +  if (result != (size_t) -1 && s != NULL)

The 'result != (size_t) -1' can be omitted if you move that 'if' into 
the previous if's then-part.


> +  data.__outbufend = (unsigned char *) buf + MB_CUR_MAX;

This'd be a bit faster (and less confusing) if we replace 'MB_CUR_MAX' 
with 'sizeof buf'.


> +      if (__glibc_unlikely (result > 2))
> +	memcpy (s, buf, result);
> +      else if (__glibc_unlikely (result > 1))
> +	*((uint16_t *) s) = *((uint16_t *) buf);
> +      else
> +	*s = *buf;

If the likely path is result == 1, shouldn't that be checked first? 
Something like this:

      if (__glibc_likely (result < 2))
        *s = *buf;
      else if (__glibc_likely (result == 2))
        memcpy (s, buf, result); /* Help the compiler.  */
      else
        memcpy (s, buf, result);