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

On Nov  8 15:37, Corinna Vinschen wrote:
> On Nov  8 12:41, Christian Franke wrote:
> > 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]
> 
> Drat.  Looks like I'm still living in K&R country...

So then, what about

  if (l[dp]-U'1'<9U && r[dp]-U'1'<9U) {

At least

  static_assert(std::is_same_v<decltype(l[dp]-U'1'), unsigned int>);

is happy.


Corinna