[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: Wed, 1 Dec 2021 19:36:22 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

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)
          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.

-      Idx node = init_state->nodes.elems[node_cnt];
+      size_t node = init_state->nodes.elems[node_cnt];

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.

-             if (__mbrtowc (&wc, (const char *) buf, p - buf,
+             if ((ssize_t)__mbrtowc (&wc, (const char *) buf, p - buf,
                             &state) == p - buf

Let's leave this alone as well. The code is OK as-is, and I'd rather not insert casts (they're too powerful and error-prone in C).

@@ -860,7 +861,8 @@ init_dfa (re_dfa_t *dfa, size_t pat_len)
        dfa->sb_char = (re_bitset_ptr_t) utf8_sb_map;
-         int i, j, ch;
+         int i, j;
+          wint_t ch;

ch can be at most 65536 so 'int' should be OK here and it's better to use a signed integer as mentioned earlier.

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.

reply via email to

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