Re: PATCH: New 64 bit wcscmp implementation

[Dmitrieva Liubov <liubov dot dmitrieva at gmail dot com>]

> I think changing the if condition to "c2 == L'\0'" will suffice to avoid
> the overflow possibility here.

I added this to the patch (new version is attached), and checked again
that "git apply" works as the patch is rather old.

Change Log:
2011-09-22  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>

       * sysdeps/x86_64/wcscmp.S: Update.
       Fix wrong comparison semantics.
       Wcscmp shall use signed comparison not unsigned.
       Don't use substraction ti avoid overflow bug.

       * sysdeps/i386/i686/multiarch/wcscmp-sse2.S: Likewise.
       * wcsmbc/wcscmp.c: Likewise.
       * string/test-strcmp.c: Likewise.
       Add new tests to check cases with negative values.

--
Liubov Dmitrieva
Intel Corporation

2011/9/22 Joseph S. Myers <joseph@codesourcery.com>:
> I think there's still one overflow case left:
>
>> +int
>> +simple_wcscmp (const wchar_t *s1, const wchar_t *s2)
>> +{
>> + ?wchar_t c1, c2;
>> + ?do
>> + ? ?{
>> + ? ? ?c1 = *s1++;
>> + ? ? ?c2 = *s2++;
>> + ? ? ?if (c1 == L'\0')
>> + ? ? ?return c1 - c2;
>
> Here, if c2 is WCHAR_MIN then c1 - c2 will overflow (most probably
> resulting in the return value ending up as negative when it should be
> positive. ?The same applies to the main C implementation of wcscmp.
>
> I think changing the if condition to "c2 == L'\0'" will suffice to avoid
> the overflow possibility here.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

Attachment: wcscmp.patch
Description: Binary data