Subject: bug#11927: closed (Re: bug#11927: shred.c i686-specific warning from gcc-4.7 on fedora 17)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 15 Jul 2012 09:43:01 +0000

Your bug report

#11927: shred.c i686-specific warning from gcc-4.7 on fedora 17

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
11927: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11927
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#11927: shred.c i686-specific warning from gcc-4.7 on fedora 17
From: Jim Meyering <[email-masked]>
To: Paul Eggert <[email-masked]>
Cc: [email-masked]
Date: Sun, 15 Jul 2012 11:36:16 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Paul Eggert wrote:
> On 07/12/2012 02:27 PM, Jim Meyering wrote:
>> I didn't really understand the cause
>
> That one is one of my least favorite warnings
> because sometimes it warns about real bugs but
> often, as in your case, the warning is either
> bogus or so hard to figure out that it might as
> well be bogus.  I see that you and I and others have
> been filing GCC bug reports in this area:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51309
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48267
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34515
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52560
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234
>
> In this particular case the shred.c rewrite seems harmless.

Thanks for the review.
I've just hit it again, so have pushed this:

>From b255b9a68a0dcec113a193daa602bdcbda2658db Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Sun, 15 Jul 2012 11:31:18 +0200
Subject: [PATCH] build: shred.c: avoid i686-specific gcc -Wstrict-overflow
 warning

* src/shred.c: Avoid gcc -Wstrict-overflow warning.
Addresses http://bugs.gnu.org/11927
---
 src/shred.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/shred.c b/src/shred.c
index 7a28260..322bcf1 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -485,10 +485,11 @@ dopass (int fd, char const *qname, off_t *sizep, int type,

       offset += soff;

+      bool done = offset == size;
+
       /* Time to print progress? */
-      if (n
-          && ((offset == size && *previous_human_offset)
-              || thresh <= (now = time (NULL))))
+      if (n && ((done && *previous_human_offset)
+                || thresh <= (now = time (NULL))))
         {
           char offset_buf[LONGEST_HUMAN_READABLE + 1];
           char size_buf[LONGEST_HUMAN_READABLE + 1];
@@ -498,8 +499,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
             = human_readable (offset, offset_buf,
                               human_floor | human_progress_opts, 1, 1);

-          if (offset == size
-              || !STREQ (previous_human_offset, human_offset))
+          if (done || !STREQ (previous_human_offset, human_offset))
             {
               if (size < 0)
                 error (0, 0, _("%s: pass %lu/%lu (%s)...%s"),
@@ -516,7 +516,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
                     = human_readable (size, size_buf,
                                       human_ceiling | human_progress_opts,
                                       1, 1);
-                  if (offset == size)
+                  if (done)
                     human_offset = human_size;
                   error (0, 0, _("%s: pass %lu/%lu (%s)...%s/%s %d%%"),
                          qname, k, n, pass_string, human_offset, human_size,
--
1.7.11.2.194.g7bdb748

Subject: shred.c i686-specific warning from gcc-4.7 on fedora 17
From: Jim Meyering <[email-masked]>
To: [email-masked]
Date: Thu, 12 Jul 2012 23:27:15 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

[mostly for the record, and so I don't forget.
 I'm not ready to push this just yet. ]

Building shred with fedora 17 and its stock gcc 4.7 on i686,
I see this warning/error:

    CC     shred.o
  shred.c: In function 'dopass':
  shred.c:501:14: error: assuming signed overflow does not occur when simplifying conditional to constant [-Werror=strict-overflow]
  shred.c:504:18: error: assuming signed overflow does not occur when simplifying conditional to constant [-Werror=strict-overflow]
  cc1: all warnings being treated as errors
  make[3]: *** [shred.o] Error 1
  make[3]: Leaving directory `/f12/home/meyering/w/co/cu/src'
  make[2]: *** [all] Error 2
  make[2]: Leaving directory `/f12/home/meyering/w/co/cu/src'
  make[1]: *** [all-recursive] Error 1
  make[1]: Leaving directory `/f12/home/meyering/w/co/cu'
  make: *** [all] Error 2

I didn't really understand the cause but ended up
writing the following patch to avoid the problem
while making the code slightly more readable:

diff --git a/src/shred.c b/src/shred.c
index 7a28260..322bcf1 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -485,10 +485,11 @@ dopass (int fd, char const *qname, off_t *sizep, int type,

       offset += soff;

+      bool done = offset == size;
+
       /* Time to print progress? */
-      if (n
-          && ((offset == size && *previous_human_offset)
-              || thresh <= (now = time (NULL))))
+      if (n && ((done && *previous_human_offset)
+                || thresh <= (now = time (NULL))))
         {
           char offset_buf[LONGEST_HUMAN_READABLE + 1];
           char size_buf[LONGEST_HUMAN_READABLE + 1];
@@ -498,8 +499,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
             = human_readable (offset, offset_buf,
                               human_floor | human_progress_opts, 1, 1);

-          if (offset == size
-              || !STREQ (previous_human_offset, human_offset))
+          if (done || !STREQ (previous_human_offset, human_offset))
             {
               if (size < 0)
                 error (0, 0, _("%s: pass %lu/%lu (%s)...%s"),
@@ -516,7 +516,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
                     = human_readable (size, size_buf,
                                       human_ceiling | human_progress_opts,
                                       1, 1);
-                  if (offset == size)
+                  if (done)
                     human_offset = human_size;
                   error (0, 0, _("%s: pass %lu/%lu (%s)...%s/%s %d%%"),
                          qname, k, n, pass_string, human_offset, human_size,