coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] maint: accommodate gcc's -Wstrict-overflow option


From: Jim Meyering
Subject: Re: [PATCH 1/2] maint: accommodate gcc's -Wstrict-overflow option
Date: Fri, 27 May 2011 00:36:19 +0200

Pádraig Brady wrote:
> On 26/05/11 17:06, Jim Meyering wrote:
>> There's a nasty bug in gcc, mentioned in log and comments below:
>> (It converts a loop that would obviously terminate into one
>> that never does, just because a signed accumulator may overflow.
>> The crazy part is that the offending accumulator is not related
>> to the loop termination condition. )
>>
>>     http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33498
>>
>> Once you read that and understand the implications, you'll
>> realize why I (the first to reject -Wstrict-overflow as not
>> worth accommodating) have decided that finally it is better
>> to use that option than to risk being bitten by the gcc bug.
>
>> -Wstrict-overflow does detect vulnerable code like the
>> example in the bug report.
>
>> Accommodating it required changes in 4 programs and tweaks
>> to configure.ac so that --enable-gcc-warnings now enables
>> that option in src/, but not in lib/ or in gnulib-tests/.
>> There would be 3 warnings in lib/ -- for two of them I
>> have patches.
>
> Great, something off my todo list :)
> The other option is to specify -fno-strict-overflow,
> though given the relatively minor changes to accommodate
> -Wstrict-overflow[=2], I think this is the way to go.
>
> The changes look good.

Thanks for the review.
There was one other trivial change, in pr.c:
Here's what I've merged into that change set:

commit 419b6c9d42ba643265f802cd150d0b232e43186a
Author: Jim Meyering <address@hidden>
Date:   Wed May 25 12:29:18 2011 +0200

    maint: accommodate gcc's -Wstrict-overflow option

    * src/factor.c (factor_using_pollard_rho): Change type of "i"
    to unsigned to avoid warning from gcc's -Wstrict-overflow.
    * src/expr.c: Use an unsigned intermediate.
    * src/dircolors.c (main): Reorder operations to avoid the risk of
    pointer overflow.
    * src/tr.c (squeeze_filter): Change NOT_A_CHAR from an anonymous
    "enum" to an "int", to avoid this warning:
    tr.c:1624:10: error: assuming signed overflow does not occur when
      simplifying conditional to constant [-Werror=strict-overflow]
    * src/pr.c (main): Make index "i" unsigned.

diff --git a/src/pr.c b/src/pr.c
index 7e6b13c..3995c37 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1165,7 +1165,7 @@ main (int argc, char **argv)
         print_files (n_files, file_names);
       else
         {
-          int i;
+          unsigned int i;
           for (i = 0; i < n_files; i++)
             print_files (1, &file_names[i]);
         }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]