Re: [musl] [PATCH] fix return value of wcs{,n}cmp for near-limits signed wchar_t values

[Rich Felker <dalias@libc.org>]

On Fri, Jan 06, 2023 at 10:20:10AM +0100, Markus Wichmann wrote:
> On Wed, Jan 04, 2023 at 04:07:19PM +0100, Gabriel Ravier wrote:
> >  int wcscmp(const wchar_t *l, const wchar_t *r)
> >  {
> >  	for (; *l==*r && *l && *r; l++, r++);
> 
> I just noticed this line. Isn't the "&& *r" part superfluous? If r is a
> prefix of l, then *r and *l will be unequal, and the loop will terminate
> because of the first condition alone. (If l is a prefix of r, we need
> the second condition to terminate the loop.)

Yes, but I would assume the compiler would make the same optimization
anyway. The original motivation here may have just been writing it
symmetrically in l and r.

> > -	return *l - *r;
> > +	return *l < *r ? -1 : *l > *r;
> >  }
> 
> Ah, that old bug. The problem is that the difference between two 32-bit
> values takes up 33 bits to save. I wonder if it would be worth it to
> just cast the values to 64 bits, then dividing the result by two. You
> know, like
> 
> return ((int64_t)*l - *r) >> 1;
> 
> Although that does presuppose that wchar_t is smaller than 64 bits,
> which the proposed change does not require.

As you noted later this doesn't work but I think the core flaw here is
not the same as the classic bug. Rather, I probably had a wrong
initial assumption that the function was intended only to work for
valid wide character values of wchar_t not arbitrary integers that fit
in wchar_t arrays. In that case they would be 21-bit and the
difference would never overflow.

Rich