Re: [PATCH v2 00/10] Code hygiene fixes from grub
On 12/1/21 18:00, Bruno Haible wrote: I will go through your patches this Friday, except for the regex ones, that I'll leave to Paul. Thanks for the re-submission and the patience. Yes, thanks for going to all that work. Some of the other patches look good but I'll let Bruno take a look.
Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
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.
- 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 (, (const char *) buf, p - buf,
+ if ((ssize_t)__mbrtowc (, (const char *) buf, p - buf,
) == 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;
else
{
- 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.
Re: [PATCH v2 05/10] Make gnulib's regcomp not abort()
On 12/1/21 13:02, Robbie Harwood wrote: @@ -1099,7 +1099,7 @@ optimize_utf8 (re_dfa_t *dfa) } break; default: - abort (); + break; } Likewise, it's not clear why this change is needed. The 'abort' should not be reachable. Is the intent to make the code a bit smaller by avoding calls to 'abort'?
Re: [PATCH v2 05/10] Make gnulib's regcomp not abort()
It's not clear why this change is needed. POSIX says that for regerror "the application shall ensure is the last non-zero value returned by regcomp() or regexec() with the given value of preg". If an application violates a "shall" requirement, behavior is undefined so it's OK for regerror to abort in that case.
Re: [PATCH v2 03/10] gnulib/regexec: Resolve unused variable
On 12/1/21 13:01, Robbie Harwood wrote: The reason for this issue is that we are not building with DEBUG set and this in turn means that the assert() that reads the value of the variable match_last is being processed out. Could you explain what goes wrong? regexec_internal.h does this: #if defined DEBUG && DEBUG != 0 # include # define DEBUG_ASSERT(x) assert (x) #else # define DEBUG_ASSERT(x) assume (x) #endif so the later 'DEBUG_ASSERT (match_last != -1);' expands to 'assume (match_last != -1);'. And verify.h defines 'assume(R)' to evaluate R so match_last is used there (unless you're using Microsoft's compiler in which case it is equivalent to '__assume (R)'; are you having problems with Microsoft's compiler?).
Re: [PATCH v2 02/10] gnulib/regexec: Fix possible null-dereference
On 12/1/21 13:01, Robbie Harwood wrote: It appears to be possible that the mctx->state_log field may be NULL, I don't see how. re_search_internal sets mctx.state_log to a non-null value if dfa->has_mb_node, and clean_state_log_if_needed should be called only if dfa->has_mb_node is true. What am I missing?
Re: [PATCH v2 00/10] Code hygiene fixes from grub
Hello Robbie, I will go through your patches this Friday, except for the regex ones, that I'll leave to Paul. Thanks for the re-submission and the patience. Bruno
[PATCH v2 02/10] gnulib/regexec: Fix possible null-dereference
From: Darren Kenny
It appears to be possible that the mctx->state_log field may be NULL,
and the name of this function, clean_state_log_if_needed(), suggests
that it should be checking that it is valid to be cleaned before
assuming that it does.
Fixes: CID 86720
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
Signed-off-by: Robbie Harwood
---
lib/regexec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/regexec.c b/lib/regexec.c
index 085bf27b0..d63b8800c 100644
--- a/lib/regexec.c
+++ b/lib/regexec.c
@@ -1657,6 +1657,9 @@ clean_state_log_if_needed (re_match_context_t *mctx, Idx
next_state_log_idx)
{
Idx top = mctx->state_log_top;
+ if (mctx->state_log == NULL)
+return REG_NOERROR;
+
if ((next_state_log_idx >= mctx->input.bufs_len
&& mctx->input.bufs_len < mctx->input.len)
|| (next_state_log_idx >= mctx->input.valid_len
--
2.33.0
[PATCH v2 04/10] Fix width computation
From: Vladimir 'phcoder' Serbinenko
Signed-off-by: Robbie Harwood
---
lib/argp-fmtstream.c | 80 +---
lib/argp-help.c | 3 +-
lib/mbswidth.c | 15 +
lib/mbswidth.h | 4 +++
4 files changed, 88 insertions(+), 14 deletions(-)
diff --git a/lib/argp-fmtstream.c b/lib/argp-fmtstream.c
index 197eebab8..f679751b9 100644
--- a/lib/argp-fmtstream.c
+++ b/lib/argp-fmtstream.c
@@ -28,9 +28,11 @@
#include
#include
#include
+#include
#include "argp-fmtstream.h"
#include "argp-namefrob.h"
+#include "mbswidth.h"
#ifndef ARGP_FMTSTREAM_USE_LINEWRAP
@@ -115,6 +117,51 @@ weak_alias (__argp_fmtstream_free, argp_fmtstream_free)
#endif
#endif
+
+/* Return the pointer to the first character that doesn't fit in l columns. */
+static inline const ptrdiff_t
+add_width (const char *ptr, const char *end, size_t l)
+{
+ mbstate_t ps;
+ const char *ptr0 = ptr;
+
+ memset (, 0, sizeof (ps));
+
+ while (ptr < end)
+{
+ wchar_t wc;
+ size_t s, k;
+
+ s = mbrtowc (, ptr, end - ptr, );
+ if (s == (size_t) -1)
+ break;
+ if (s == (size_t) -2)
+ {
+ if (1 >= l)
+ break;
+ l--;
+ ptr++;
+ continue;
+ }
+
+ if (wc == '\e' && ptr + 3 < end
+ && ptr[1] == '[' && (ptr[2] == '0' || ptr[2] == '1')
+ && ptr[3] == 'm')
+ {
+ ptr += 4;
+ continue;
+ }
+
+ k = wcwidth (wc);
+
+ if (k >= l)
+ break;
+ l -= k;
+ ptr += s;
+}
+ return ptr - ptr0;
+}
+
/* Process FS's buffer so that line wrapping is done from POINT_OFFS to the
end of its buffer. This code is mostly from glibc stdio/linewrap.c. */
void
@@ -168,13 +215,15 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
if (!nl)
{
/* The buffer ends in a partial line. */
+ size_t display_width = mbsnwidth (buf, fs->p - buf,
+MBSW_STOP_AT_NUL);
- if (fs->point_col + len < fs->rmargin)
+ if (fs->point_col + display_width < fs->rmargin)
{
/* The remaining buffer text is a partial line and fits
within the maximum line width. Advance point for the
characters to be written and stop scanning. */
- fs->point_col += len;
+ fs->point_col += display_width;
break;
}
else
@@ -182,14 +231,18 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
the end of the buffer. */
nl = fs->p;
}
- else if (fs->point_col + (nl - buf) < (ssize_t) fs->rmargin)
-{
- /* The buffer contains a full line that fits within the maximum
- line width. Reset point and scan the next line. */
- fs->point_col = 0;
- buf = nl + 1;
- continue;
-}
+ else
+ {
+ size_t display_width = mbsnwidth (buf, nl - buf, MBSW_STOP_AT_NUL);
+ if (display_width < (ssize_t) fs->rmargin)
+ {
+ /* The buffer contains a full line that fits within the maximum
+line width. Reset point and scan the next line. */
+ fs->point_col = 0;
+ buf = nl + 1;
+ continue;
+ }
+ }
/* This line is too long. */
r = fs->rmargin - 1;
@@ -225,7 +278,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
char *p, *nextline;
int i;
- p = buf + (r + 1 - fs->point_col);
+ p = buf + add_width (buf, fs->p, (r + 1 - fs->point_col));
while (p >= buf && !isblank ((unsigned char) *p))
--p;
nextline = p + 1; /* This will begin the next line. */
@@ -243,7 +296,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
{
/* A single word that is greater than the maximum line width.
Oh well. Put it on an overlong line by itself. */
- p = buf + (r + 1 - fs->point_col);
+ p = buf + add_width (buf, fs->p, (r + 1 - fs->point_col));
/* Find the end of the long word. */
if (p < nl)
do
@@ -277,7 +330,8 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
&& fs->p > nextline)
{
/* The margin needs more blanks than we removed. */
- if (fs->end - fs->p > fs->wmargin + 1)
+ if (mbsnwidth (fs->p, fs->end - fs->p, MBSW_STOP_AT_NUL)
+ > fs->wmargin + 1)
/* Make some space for them. */
{
size_t mv = fs->p - nextline;
diff --git a/lib/argp-help.c b/lib/argp-help.c
index 80cdb4493..84013b002 100644
--- a/lib/argp-help.c
+++ b/lib/argp-help.c
@@ -52,6 +52,7 @@
#include "argp.h"
#include "argp-fmtstream.h"
#include "argp-namefrob.h"
+#include "mbswidth.h"
[PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
From: Peter Jones
[rharw...@redhat.com: rebase conflicts in regexec, regcomp]
Signed-off-by: Robbie Harwood
---
lib/argp-fmtstream.c | 2 +-
lib/regcomp.c| 28 +++-
lib/regex_internal.c | 6 +++---
lib/regexec.c| 36
lib/vasnprintf.c | 4 ++--
5 files changed, 41 insertions(+), 35 deletions(-)
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. */
diff --git a/lib/regcomp.c b/lib/regcomp.c
index a8cefebf1..b2ab8f9a3 100644
--- a/lib/regcomp.c
+++ b/lib/regcomp.c
@@ -286,7 +286,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t
*init_state,
bool icase = (dfa->mb_cur_max == 1 && (bufp->syntax & RE_ICASE));
for (node_cnt = 0; node_cnt < init_state->nodes.nelem; ++node_cnt)
{
- Idx node = init_state->nodes.elems[node_cnt];
+ size_t node = init_state->nodes.elems[node_cnt];
re_token_type_t type = dfa->nodes[node].type;
if (type == CHARACTER)
@@ -306,7 +306,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t
*init_state,
&& dfa->nodes[node].mb_partial)
*p++ = dfa->nodes[node].opr.c;
memset (, '\0', sizeof (state));
- if (__mbrtowc (, (const char *) buf, p - buf,
+ if ((ssize_t)__mbrtowc (, (const char *) buf, p - buf,
) == p - buf
&& (__wcrtomb ((char *) buf, __towlower (wc), )
!= (size_t) -1))
@@ -558,7 +558,8 @@ static const bitset_t utf8_sb_map =
static void
free_dfa_content (re_dfa_t *dfa)
{
- Idx i, j;
+ size_t i;
+ Idx j;
if (dfa->nodes)
for (i = 0; i < dfa->nodes_len; ++i)
@@ -860,7 +861,8 @@ init_dfa (re_dfa_t *dfa, size_t pat_len)
dfa->sb_char = (re_bitset_ptr_t) utf8_sb_map;
else
{
- int i, j, ch;
+ int i, j;
+ wint_t ch;
dfa->sb_char = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1);
if (__glibc_unlikely (dfa->sb_char == NULL))
@@ -1045,7 +1047,7 @@ create_initial_state (re_dfa_t *dfa)
static void
optimize_utf8 (re_dfa_t *dfa)
{
- Idx node;
+ size_t node;
int i;
bool mb_chars = false;
bool has_period = false;
@@ -1139,12 +1141,12 @@ analyze (regex_t *preg)
dfa->subexp_map = re_malloc (Idx, preg->re_nsub);
if (dfa->subexp_map != NULL)
{
- Idx i;
+ size_t i;
for (i = 0; i < preg->re_nsub; i++)
dfa->subexp_map[i] = i;
preorder (dfa->str_tree, optimize_subexps, dfa);
for (i = 0; i < preg->re_nsub; i++)
- if (dfa->subexp_map[i] != i)
+ if (dfa->subexp_map[i] != (ssize_t)i)
break;
if (i == preg->re_nsub)
{
@@ -1589,7 +1591,7 @@ duplicate_node (re_dfa_t *dfa, Idx org_idx, unsigned int
constraint)
static reg_errcode_t
calc_inveclosure (re_dfa_t *dfa)
{
- Idx src, idx;
+ size_t src, idx;
bool ok;
for (idx = 0; idx < dfa->nodes_len; ++idx)
re_node_set_init_empty (dfa->inveclosures + idx);
@@ -1597,7 +1599,7 @@ calc_inveclosure (re_dfa_t *dfa)
for (src = 0; src < dfa->nodes_len; ++src)
{
Idx *elems = dfa->eclosures[src].elems;
- for (idx = 0; idx < dfa->eclosures[src].nelem; ++idx)
+ for (idx = 0; (ssize_t)idx < dfa->eclosures[src].nelem; ++idx)
{
ok = re_node_set_insert_last (dfa->inveclosures + elems[idx], src);
if (__glibc_unlikely (! ok))
@@ -1613,7 +1615,7 @@ calc_inveclosure (re_dfa_t *dfa)
static reg_errcode_t
calc_eclosure (re_dfa_t *dfa)
{
- Idx node_idx;
+ size_t node_idx;
bool incomplete;
DEBUG_ASSERT (dfa->nodes_len > 0);
incomplete = false;
@@ -2666,9 +2668,9 @@ build_range_exp (bitset_t sbcset, re_charset_t *mbcset,
Idx *range_alloc,
: 0));
wint_t
start_wc = ((start_elem->type == SB_CHAR || start_elem->type == COLL_SYM)
- ? parse_byte (start_ch, dfa) : start_elem->opr.wch),
+ ? parse_byte (start_ch, dfa) : (wint_t) start_elem->opr.wch),
end_wc = ((end_elem->type == SB_CHAR || end_elem->type == COLL_SYM)
- ? parse_byte (end_ch, dfa) : end_elem->opr.wch);
+ ? parse_byte (end_ch, dfa) : (wint_t) end_elem->opr.wch);
if (start_wc == WEOF || end_wc == WEOF)
return REG_ECOLLATE;
@@ -2717,7 +2719,7 @@ build_range_exp (bitset_t sbcset, re_charset_t *mbcset,
Idx *range_alloc,
}
/* Build the table for single byte characters. */
- for
[PATCH v2 05/10] Make gnulib's regcomp not abort()
From: Vladimir 'phcoder' Serbinenko
[rharw...@redhat.com: we wrote a commit message]
Signed-off-by: Robbie Harwood
---
lib/regcomp.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/regcomp.c b/lib/regcomp.c
index 6a97fdee4..a8cefebf1 100644
--- a/lib/regcomp.c
+++ b/lib/regcomp.c
@@ -506,9 +506,9 @@ regerror (int errcode, const regex_t *__restrict preg, char
*__restrict errbuf,
to this routine. If we are given anything else, or if other regex
code generates an invalid error code, then the program has a bug.
Dump core so we can fix it. */
-abort ();
-
- msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
+msg = gettext ("unknown regexp error");
+ else
+msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
msg_size = strlen (msg) + 1; /* Includes the null. */
@@ -1099,7 +1099,7 @@ optimize_utf8 (re_dfa_t *dfa)
}
break;
default:
- abort ();
+ break;
}
if (mb_chars || has_period)
--
2.33.0
gnulib fails for build on Mac OS X 10.4: get_ppid_of.c:36:22: error: libproc.h: No such file or directory
Hi, gnulib fails to build on Mac OS X 10.4 Tiger: get_ppid_of.c:36:22: error: libproc.h: No such file or directory libproc.h was introduced in Mac OS X 10.5 Leopard. This problem affects gettext 0.21 which includes this version of gnulib. This is a problem because so many things depend on gettext. This was reported to MacPorts here: https://trac.macports.org/ticket/64102 Previous versions of gnulib and gettext did not use libproc.h and so did not have this problem. Perhaps some solution can be found to make the use of libproc.h optional so that systems without that header can still be used?
[PATCH v2 06/10] Make CFLAGS less painful
From: Peter Jones Signed-off-by: Peter Jones [rharw...@redhat.com: make rpm gunk conditional] Signed-off-by: Robbie Harwood --- gnulib-tool | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/gnulib-tool b/gnulib-tool index 9c4a6c17e..24cdbe2da 100755 --- a/gnulib-tool +++ b/gnulib-tool @@ -3841,18 +3841,23 @@ func_emit_lib_Makefile_am () cppflags_part1= fi if $for_test; then -cppflags_part2=" -DGNULIB_STRICT_CHECKING=1" +cppflags_part2=" \$(HOST_CPPFLAGS) -DGNULIB_STRICT_CHECKING=1 -D_GLIBCXX_ASSERTIONS " else -cppflags_part2= +cppflags_part2=" \$(HOST_CPPFLAGS) -D_GLIBCXX_ASSERTIONS " + fi + rpm_extra_cflags= + if test -f "/usr/lib/rpm/redhat/redhat-hardened-cc1" && test -f "/usr/lib/rpm/redhat/redhat-annobin-cc1"; then +rpm_extra_cflags="-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" fi if test -z "$makefile_name"; then echo echo "AM_CPPFLAGS =$cppflags_part1$cppflags_part2" -echo "AM_CFLAGS =" +echo "AM_CFLAGS = \$(HOST_CFLAGS) -fexceptions -fstack-protector-strong -fno-strict-aliasing $rpm_extra_cflags -Wp,-D_GLIBCXX_ASSERTIONS -Wp,-DGNULIB_STRICT_CHECKING=1 -W -Wall -Wextra -Wno-undef -Wno-missing-field-initializers -Wno-unused-parameter -Werror -Wno-error=vla -Wno-error=type-limits -Werror=format-security -Werror=trampolines -Wno-error=format-nonliteral -Wno-error=cast-align " else if test -n "$cppflags_part1$cppflags_part2"; then echo echo "AM_CPPFLAGS +=$cppflags_part1$cppflags_part2" + echo "AM_CFLAGS = \$(HOST_CFLAGS) -fexceptions -fstack-protector-strong -fno-strict-aliasing $rpm_extra_cflags -Wp,-D_GLIBCXX_ASSERTIONS -Wp,-DGNULIB_STRICT_CHECKING=1 -W -Wall -Wextra -Wno-undef -Wno-missing-field-initializers -Wno-unused-parameter -Werror -Wno-error=vla -Wno-error=type-limits -Werror=format-security -Werror=trampolines -Wno-error=format-nonliteral -Wno-error=cast-align " fi fi echo -- 2.33.0
[PATCH v2 10/10] Fixup for -Werror=ignored-qualifiers issues
From: Peter Jones
Signed-off-by: Peter Jones
Signed-off-by: Robbie Harwood
---
lib/argp-fmtstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/argp-fmtstream.c b/lib/argp-fmtstream.c
index 4aa401e2d..d870716da 100644
--- a/lib/argp-fmtstream.c
+++ b/lib/argp-fmtstream.c
@@ -119,7 +119,7 @@ weak_alias (__argp_fmtstream_free, argp_fmtstream_free)
/* Return the pointer to the first character that doesn't fit in l columns. */
-static inline const ptrdiff_t
+static inline ptrdiff_t
add_width (const char *ptr, const char *end, size_t l)
{
mbstate_t ps;
--
2.33.0
[PATCH v2 03/10] gnulib/regexec: Resolve unused variable
From: Darren Kenny This is a really minor issue where a variable is being assigned to but not checked before it is overwritten again. The reason for this issue is that we are not building with DEBUG set and this in turn means that the assert() that reads the value of the variable match_last is being processed out. The solution, move the assignment to match_last in to an ifdef DEBUG too. Fixes: CID 292459 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper Signed-off-by: Robbie Harwood --- lib/regexec.c | 4 1 file changed, 4 insertions(+) diff --git a/lib/regexec.c b/lib/regexec.c index d63b8800c..1aad68175 100644 --- a/lib/regexec.c +++ b/lib/regexec.c @@ -807,7 +807,11 @@ re_search_internal (const regex_t *preg, const char *string, Idx length, break; if (__glibc_unlikely (err != REG_NOMATCH)) goto free_return; +#ifdef DEBUG + /* Only used for assertion below when DEBUG is set, otherwise +it will be over-written when we loop around. */ match_last = -1; +#endif } else break; /* We found a match. */ -- 2.33.0
[PATCH v2 09/10] Paper over a stringop-overflow warning about wide char handling
From: Peter Jones
[rharw...@redhat.com: rewrote commit message]
Signed-off-by: Robbie Harwood
---
lib/vasnprintf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c
index 360ca6948..7faa11753 100644
--- a/lib/vasnprintf.c
+++ b/lib/vasnprintf.c
@@ -234,8 +234,11 @@
static size_t
local_strnlen (const char *string, size_t maxlen)
{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic warning "-Wstringop-overflow"
const char *end = memchr (string, '\0', maxlen);
return end ? (size_t) (end - string) : maxlen;
+#pragma GCC diagnostic pop
}
# endif
# endif
--
2.33.0
[PATCH v2 01/10] argp-parse.c (__argp_input): Don't crash if pstate is NULL
From: Colin Watson
[rharw...@redhat.com: tweaked commit message]
Signed-off-by: Robbie Harwood
---
lib/argp-parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/argp-parse.c b/lib/argp-parse.c
index 053495ec0..4f1c65d73 100644
--- a/lib/argp-parse.c
+++ b/lib/argp-parse.c
@@ -940,7 +940,7 @@ weak_alias (__argp_parse, argp_parse)
void *
__argp_input (const struct argp *argp, const struct argp_state *state)
{
- if (state)
+ if (state && state->pstate)
{
struct group *group;
struct parser *parser = state->pstate;
--
2.33.0
[PATCH v2 07/10] Fix __argp_fmtstream_point()'s return type and comparisons with it
From: Peter Jones
gcc -Werror=sign-compare produces:
argp-help.c:1545:15: error: comparison of integer expressions of different
signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
1545 | > __argp_fmtstream_lmargin (stream))
| ^
Some code seems to assume the point_col field (which
__argp_fmtstream_point() returns) can be -1, and it's an ssize_t, so
this makes the function correctly return ssize_t in all cases, and also
fixes the comparison to check for the -1 case.
Signed-off-by: Peter Jones
Signed-off-by: Robbie Harwood
---
lib/argp-fmtstream.h | 6 +++---
lib/argp-help.c | 7 +--
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/lib/argp-fmtstream.h b/lib/argp-fmtstream.h
index 3384a0012..f47e24c5c 100644
--- a/lib/argp-fmtstream.h
+++ b/lib/argp-fmtstream.h
@@ -164,8 +164,8 @@ extern size_t __argp_fmtstream_set_wmargin
(argp_fmtstream_t __fs,
size_t __wmargin);
/* Return the column number of the current output point in __FS. */
-extern size_t argp_fmtstream_point (argp_fmtstream_t __fs);
-extern size_t __argp_fmtstream_point (argp_fmtstream_t __fs);
+extern ssize_t argp_fmtstream_point (argp_fmtstream_t __fs);
+extern ssize_t __argp_fmtstream_point (argp_fmtstream_t __fs);
#endif
/* Internal routines. */
@@ -272,7 +272,7 @@ __argp_fmtstream_set_wmargin (argp_fmtstream_t __fs, size_t
__wmargin)
}
/* Return the column number of the current output point in __FS. */
-ARGP_FS_EI size_t
+ARGP_FS_EI ssize_t
__argp_fmtstream_point (argp_fmtstream_t __fs)
{
if ((size_t) (__fs->p - __fs->buf) > __fs->point_offs)
diff --git a/lib/argp-help.c b/lib/argp-help.c
index 84013b002..93e40cad7 100644
--- a/lib/argp-help.c
+++ b/lib/argp-help.c
@@ -1631,7 +1631,9 @@ argp_doc (const struct argp *argp, const struct
argp_state *state,
else
__argp_fmtstream_puts (stream, text);
- if (__argp_fmtstream_point (stream) > __argp_fmtstream_lmargin (stream))
+ if (__argp_fmtstream_point (stream) < 0 ||
+ (size_t)__argp_fmtstream_point (stream)
+ > __argp_fmtstream_lmargin (stream))
__argp_fmtstream_putc (stream, '\n');
anything = 1;
@@ -1652,7 +1654,8 @@ argp_doc (const struct argp *argp, const struct
argp_state *state,
__argp_fmtstream_putc (stream, '\n');
__argp_fmtstream_puts (stream, text);
free ((char *) text);
- if (__argp_fmtstream_point (stream)
+ if (__argp_fmtstream_point (stream) < 0 ||
+ (size_t)__argp_fmtstream_point (stream)
> __argp_fmtstream_lmargin (stream))
__argp_fmtstream_putc (stream, '\n');
anything = 1;
--
2.33.0
[PATCH v2 00/10] Code hygiene fixes from grub
Hello gnulib, This is a slightly updated version of our collected grub2 patches to gnulib. In addition to rebasing around changes to the development branch, I've also dropped the base64 patch since it was preferred to make the change in grub. While that generated additional dicsussion that I hope will be productive, it's not related to the rest of the patches, which can be considered separately. Be well, --Robbie Colin Watson (1): argp-parse.c (__argp_input): Don't crash if pstate is NULL Darren Kenny (2): gnulib/regexec: Fix possible null-dereference gnulib/regexec: Resolve unused variable Peter Jones (5): Make CFLAGS less painful Fix __argp_fmtstream_point()'s return type and comparisons with it Fix up a bunch of "gcc -Werror=sign-compare" complaints Paper over a stringop-overflow warning about wide char handling Fixup for -Werror=ignored-qualifiers issues Vladimir 'phcoder' Serbinenko (2): Fix width computation Make gnulib's regcomp not abort() gnulib-tool | 11 -- lib/argp-fmtstream.c | 80 +--- lib/argp-fmtstream.h | 6 ++-- lib/argp-help.c | 10 -- lib/argp-parse.c | 2 +- lib/mbswidth.c | 15 + lib/mbswidth.h | 4 +++ lib/regcomp.c| 36 ++-- lib/regex_internal.c | 6 ++-- lib/regexec.c| 43 +++- lib/vasnprintf.c | 7 ++-- 11 files changed, 159 insertions(+), 61 deletions(-) -- 2.33.0
