bug-gnulib
[Top][All Lists]
Advanced

[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: Robbie Harwood
Subject: Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
Date: Tue, 07 Dec 2021 14:44:43 -0500

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 12/1/21 13:02, Robbie Harwood wrote:
>
>> diff --git a/lib/argp-fmtstream.c b/lib/argp-fmtstream.c
>> index f679751b9..4aa401e2d 100644
>> --- a/lib/argp-fmtstream.c
>> +++ b/lib/argp-fmtstream.c
>> @@ -234,7 +234,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
>>         else
>>      {
>>        size_t display_width = mbsnwidth (buf, nl - buf, MBSW_STOP_AT_NUL);
>> -      if (display_width < (ssize_t) fs->rmargin)
>> +      if (display_width < fs->rmargin)
>>          {
>>            /* The buffer contains a full line that fits within the maximum
>>               line width.  Reset point and scan the next line.  */
>
> This fixes a problem introduced in PATCH v2 04/10 "Fix width 
> computation". Please fix that patch instead.

Willdo, thanks.

> Let's not make this sort of change. We are moving to a coding style that 
> prefers signed values, since they allow us to use better runtime checks. 
> If -Wsign-compare complains, let's disable -Wsign-compare instead of 
> changing carefully-written signed integers to be unsigned.
>
> The remaining changes in this patch also seem to be unnecessary; they're 
> put in only to pacify -Wsign-compare, so the solution is to not use 
> -Wsign-compare. That's what Coreutils does, for example.

I'm not going to pretend it's the most important flag or anything like
that, but 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
flag).

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

Be well,
--Robbie

Attachment: signature.asc
Description: PGP signature


reply via email to

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