Subject: Re: bug#8732: uinttostr: comparison of unsigned expression < 0 is always false

From: Eric Blake <[email-masked]>
Date: Wed, 25 May 2011 15:48:19 -0600

[adding bug-gnulib]

On 05/25/2011 03:29 PM, Eric Blake wrote:
>>> building coreutils-8.12 with '--enable-gcc-warnings' fails on my SLES 10.3 server:
>>>
>>>   CC       uinttostr.o
>>> cc1: warnings being treated as errors
>>> In file included from uinttostr.c:3:
>>> anytostr.c: In function 'uinttostr':
>>> anytostr.c:39: warning: comparison of unsigned expression < 0 is always false
>>
>> the warning is spurious.  But we don't know how
>> to shut up gcc.
> 
> You're not the first to hit this, either:
> 
> http://lists.gnu.org/archive/html/bug-gnulib/2010-10/msg00435.html
> 
> Since our party line back then was to "upgrade gcc or quit using -Werror
> with old gcc", I'm marking this as a wontfix; but if you can come up
> with a patch, we can reopen it.

Jim, Paul,

This patch silences the gcc warning even for gcc too old to honor the
pragma, but I can't help but feel that it might be too gross (it makes
functions like uinttostr larger in size).  What do you think?

diff --git i/lib/anytostr.c w/lib/anytostr.c
index e23746a..da57259 100644
--- i/lib/anytostr.c
+++ w/lib/anytostr.c
@@ -17,11 +17,6 @@

 /* Written by Paul Eggert */

-/* Tell gcc not to warn about the (i < 0) test, below.  */
-#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__
-# pragma GCC diagnostic ignored "-Wtype-limits"
-#endif
-
 #include <config.h>

 #include "inttostr.h"
@@ -34,15 +29,17 @@ char * __attribute_warn_unused_result__
 anytostr (inttype i, char *buf)
 {
   char *p = buf + INT_STRLEN_BOUND (inttype);
+  inttype ii = i;
   *p = 0;

-  if (i < 0)
+  if (i <= 0)
     {
       do
         *--p = '0' - i % 10;
       while ((i /= 10) != 0);

-      *--p = '-';
+      if (ii)
+        *--p = '-';
     }
   else
     {

The same trick of swapping '< 0' to '<= 0' to foil -Wtype-limits can be
used to reduce (but not eliminate) some of the other warnings generated
by intprops.h.  For example, this one's completely safe, and knocks out
a bunch of the warnings on test-intprops.c if you temporarily revert
9d196fad:

diff --git i/lib/intprops.h w/lib/intprops.h
index d722648..c2e6bf8 100644
--- i/lib/intprops.h
+++ w/lib/intprops.h
@@ -54,7 +54,7 @@

 /* Return 1 if the integer expression E, after integer promotion, has
    a signed type.  E should not have side effects.  */
-#define _GL_INT_SIGNED(e) (_GL_INT_NEGATE_CONVERT (e, 1) < 0)
+#define _GL_INT_SIGNED(e) (_GL_INT_NEGATE_CONVERT (e, 1) <= 0)

 /* Minimum and maximum values for integer types and expressions.  These

-- 
Eric Blake   [email-masked]    +1-801-349-2682
Libvirt virtualization library http://libvirt.org