Re: [PATCH] Add cast to unsigned char to strverscmp

Corinna Vinschen wrote:
> On Nov  7 13:58, Joel Sherrill wrote:
>> On Thu, Nov 7, 2024 at 1:06 PM <[email protected]> wrote:
>>
>>> On 2024-11-07 09:53, Jeremy Bettis wrote:
>>>> GCC 14.2 with -Werror=sign-compare fails on this code.
>>>>
>>>> Signed-off-by: Jeremy Bettis <[email protected] <mailto:
>>> [email protected]>>
>>>> ---
>>>>    newlib/libc/string/strverscmp.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/newlib/libc/string/strverscmp.c
>>> b/newlib/libc/string/strverscmp.c
>>>> index 55966335f..e86718faa 100644
>>>> --- a/newlib/libc/string/strverscmp.c
>>>> +++ b/newlib/libc/string/strverscmp.c
>>>> @@ -76,7 +76,7 @@ int strverscmp(const char *l0, const char *r0)
>>>>     else if (c!='0') z=0;
>>>>     }
>>>>
>>>> - if (l[dp]<'1' + 9U && r[dp]<'1' + 9U) {
> I'm puzzled.  This doesn't look like newlib code.  Newlib's code looks
> like this:
>
>          if (l[dp]-'1'<9U && r[dp]-'1'<9U) {
>
> See https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/string/strverscmp.c;hb=HEAD#l79
>
> Given that l and r are unsigned char anyway, the entire expression
> is unsigned, so there shouldn't be a sign-compare error in newlib's
> version.
>

It's actually signed, IIRC due to changing 'unsigned preserving' (K&R C) 
to 'value preserving' (C89) implicit conversions.

"Proof" using C++17 compile time checks:

$ cat uchar2int.cc
#include <cstddef>
#include <type_traits>

const unsigned char *l;
std::size_t dp;

static_assert(std::is_same_v<decltype(l[dp]), const unsigned char &>);
static_assert(std::is_same_v<decltype(l[dp]-'1'), int>);
static_assert(std::is_same_v<decltype(l[dp]-0x31U), unsigned int>);

$ g++ -S -Wall -W uchar2int.cc
[no failed assert]

-- 
Regards,
Christian