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 <meyer...@redhat.com> 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]); }