[Top][All Lists]

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

Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" compla

From: Paul Eggert
Subject: Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
Date: Tue, 7 Dec 2021 16:07:41 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

On 12/7/21 11:44, Robbie Harwood wrote:
it's odd to me to hear disabling it being the suggested course
of action (especially given it's in the gcc -Wextra set, not some random

Not odd at all. High-quality static-checking tools have tons of options for a reason, and one shouldn't assume that the tools' defaults are right for your project because (assuming the project is sufficiently complex, which GDB certainly is) they probably aren't.

Part of the problem here is that there are two conflicting attitudes about types like size_t. The traditional attitude is that one should compute sizes using size_t arithmetic, and be very careful about integer overflow because it's well-defined behavior in C so your runtime won't report it and your program will likely misbehave in mysterious ways later. Gnulib was originally written that way; however, some of its modules have been redone to follow a newer approach, in which one should compute size using ptrdiff_t arithmetic, and (while still being very careful about overflow) hope that your runtime will report it right away so that bugs are more likely to be caught and can be fixed more easily. This latter hope is realized with gcc -fsanitize=undefined, so this is a win over the traditional approach.

gcc's -Wsign-compare flag was designed more with the traditional style in mind, and it hasn't really caught up with the newer style. I hope GCC catches up someday; in the meantime, I've found it better to disable -Wsign-compare than to pacify it, as pacification makes code harder to maintain (and so, likely buggier) and can mask some other errors.

As you point out with coreutils, gnulib taking a position like this on
flags has the knock-on effect that, for our grub, we'll need to do one
of the following:

- Carry a gnulib patch to make the flag work (what we've been doing).
- Change flags in the outer work (i.e., change build options for grub)
- Maintain logic to keep flags gnulib-disliked flags out when building
   the gnulib modules

Every other Gnulib-using project I know takes the third approach.

reply via email to

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