Re: [PATCH v3 1/4] Fix width computation
Bruno Haible writes:
> Robbie Harwood wrote:
>> From: Vladimir 'phcoder' Serbinenko
>>
>> [rharw...@redhat.com: merge with later fix from pjo...@redhat.com]
>> 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(-)
>
> This patch will need several more iterations, as it mixes two good
> ideas, with - so far - imperfect implementations:
>
> * The need to use wcwidth in argp-fmtstream arises because the
> help strings are internationalized. But for internationalized strings,
> line breaking by looking for spaces is just wrong. (It works for
> Russian and Greek but not for Chinese.) A much better algorithm
> (that works for most languages, except Thai) is found in
> GNU libunistring 1.0. But the code in glibc cannot use libunistring;
> so there likely will need to be some '#ifdef _LIBC'.
>
> * The idea to parse escape sequences in a special way, before invoking
> wcwidth, it nice. But's it's far from complete. IMO one needs to look
> at GNU teseq, the ISO 2022 standard, and other de-facto standards
> when implementing that. And it should then be implemented in wcswidth,
> mbswidth, etc. uniformly.
I've put together at using libunistring (attached). Unfortunately it
doesn't seem to find ulc_width_linebreaks() at link time, and I'm not
familiar enough with gnulib to understand why. Feedback on that (or
other parts of it) appreciated.
Be well,
--Robbie
signature.asc
Description: PGP signature
>From 12c09fc6632e295101ee6957c952bbf1b5c0165a Mon Sep 17 00:00:00 2001
From: Robbie Harwood
Date: Mon, 10 Jan 2022 17:17:39 -0500
Subject: [PATCH] argp-fmtstream: use ulc_width_linebreaks()
ulc_width_linebreaks() can't be used in _LIBC, so fall back to a
best-effort solution there that won't work with Chinese.
Signed-off-by: Robbie Harwood
---
lib/argp-fmtstream.c | 361 +++
modules/argp | 1 +
2 files changed, 161 insertions(+), 201 deletions(-)
diff --git a/lib/argp-fmtstream.c b/lib/argp-fmtstream.c
index 78c29e38c..12d26effb 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
@@ -42,6 +44,8 @@
# include
# include
# define __vsnprintf(s, l, f, a) _IO_vsnprintf (s, l, f, a)
+#else
+# include
#endif
#define INIT_BUF_SIZE 200
@@ -115,233 +119,188 @@ weak_alias (__argp_fmtstream_free, argp_fmtstream_free)
#endif
#endif
-/* 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
-__argp_fmtstream_update (argp_fmtstream_t fs)
+/* Insert suffix and left margin at POINT_OFFS, flushing as needed. */
+static void
+line_at_point (argp_fmtstream_t fs, const char *suffix)
{
- char *buf, *nl;
- size_t len;
+ size_t i, space_needed = fs->wmargin;
+ char *nl, *queue = fs->buf + fs->point_offs;
- /* Scan the buffer for newlines. */
- buf = fs->buf + fs->point_offs;
- while (buf < fs->p)
+ if (suffix)
+space_needed += strlen(suffix);
+
+ while (fs->p + space_needed > fs->end)
{
- size_t r;
-
- if (fs->point_col == 0 && fs->lmargin != 0)
+ /* Output the first line so we can use the space. */
+ nl = memchr (fs->buf, '\n', fs->point_offs);
+ if (nl == NULL)
{
- /* We are starting a new line. Print spaces to the left margin. */
- const size_t pad = fs->lmargin;
- if (fs->p + pad < fs->end)
-{
- /* We can fit in them in the buffer by moving the
- buffer text up and filling in the beginning. */
- memmove (buf + pad, buf, fs->p - buf);
- fs->p += pad; /* Compensate for bigger buffer. */
- memset (buf, ' ', pad); /* Fill in the spaces. */
- buf += pad; /* Don't bother searching them. */
-}
- else
-{
- /* No buffer space for spaces. Must flush. */
- size_t i;
- for (i = 0; i < pad; i++)
-{
+ /* Line longer than buffer - shouldn't happen. Truncate. */
+ nl = queue - 1;
+ *nl = '\n';
+}
+
#ifdef _LIBC
- if (_IO_fwide (fs->stream, 0) > 0)
-putwc_unlocked (L' ', fs->stream);
- else
+ __fxprintf (fs->stream, "%.*s\n",
Re: [PATCH v3 1/4] Fix width computation
Bruno Haible writes: > Robbie Harwood wrote: >> From: Vladimir 'phcoder' Serbinenko >> >> [rharw...@redhat.com: merge with later fix from pjo...@redhat.com] >> 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(-) > > This patch will need several more iterations, as it mixes two good > ideas, with - so far - imperfect implementations: > > * The need to use wcwidth in argp-fmtstream arises because the help > strings are internationalized. But for internationalized strings, > line breaking by looking for spaces is just wrong. (It works for > Russian and Greek but not for Chinese.) A much better algorithm > (that works for most languages, except Thai) is found in GNU > libunistring 1.0. But the code in glibc cannot use libunistring; so > there likely will need to be some '#ifdef _LIBC'. > > * The idea to parse escape sequences in a special way, before > invoking wcwidth, it nice. But's it's far from complete. IMO one > needs to look at GNU teseq, the ISO 2022 standard, and other > de-facto standards when implementing that. And it should then be > implemented in wcswidth, mbswidth, etc. uniformly. Thank you for the feedback. I will give this a shot, but can't make any guarantees: from your comments, it seems like a large rework of something I don't have background in. Given that, and that the patches in this series are not dependent on each other, it would be nice if this change did not hold up the rest. Be well, --Robbie signature.asc Description: PGP signature
[PATCH v3 2/4] 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 a91847f55a..e4f6486a51 100755 --- a/gnulib-tool +++ b/gnulib-tool @@ -3944,18 +3944,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.34.1
[PATCH v3 1/4] Fix width computation
From: Vladimir 'phcoder' Serbinenko
[rharw...@redhat.com: merge with later fix from pjo...@redhat.com]
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 78c29e38c3..0de1ac374e 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 < 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
[PATCH v3 4/4] 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 0de1ac374e..57fc955a20 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.34.1
[PATCH v3 3/4] 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 485745243f..122b224259 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.34.1
[PATCH v3 0/4] Code hygiene fixes from grub
Hello gnulib, Based on our previous conversations, this is a reduced set of the changes from our grub2. Note that the sign-compare changes are not being propsed this time due to the ongoing conversation about gnulib-specific CFLAGS. Be well, --Robbie Peter Jones (3): Make CFLAGS less painful Paper over a stringop-overflow warning about wide char handling Fixup for -Werror=ignored-qualifiers issues Vladimir 'phcoder' Serbinenko (1): Fix width computation gnulib-tool | 11 -- lib/argp-fmtstream.c | 80 +--- lib/argp-help.c | 3 +- lib/mbswidth.c | 15 + lib/mbswidth.h | 4 +++ lib/vasnprintf.c | 3 ++ 6 files changed, 99 insertions(+), 17 deletions(-) -- 2.34.1
Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
Bruno Haible writes: > Paul Eggert replied: > >> Every other Gnulib-using project I know takes the third approach. > > So, how about adding the third approach to gnulib-tool? > > Rationale: > Gnulib does not want to dictate their preferred GCC flags to coreutils, > grub, etc. > And similarly, we don't want coreutils, grub, etc. to dictate the coding > style in which gnulib is written. > > Implementation idea: > Since 2021-06-10, gnulib-tool already ensures that the Gnulib modules are > compiled with '-Wno-error'. This code could be extended to add > '-Wno-sign-compare' and other warning-disabling options. Given the spirit of the proposal, I'd feel weird commenting on the chosen warning set, but I do like the idea itself, thanks! Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH v2 02/10] gnulib/regexec: Fix possible null-dereference
Paul Eggert writes: > On 12/7/21 09:38, Robbie Harwood wrote: > >> My*guess* is that Coverity has noticed that `mctx->state_log` is >> checked against NULL in many other places in that file, and was unable >> to prove to itself that it couldn't be NULL there too. If that's the >> case, a DEBUG_ASSERT would presumably do the trick better. > > Yes, I can see why Coverity can't deduce the code is safe. > > I installed the attached patch into Gnulib; it adds a DEBUG_ASSERT which > should be a reasonable prophylactic even if we don't use Coverity. I > hope this also suffices to pacify Coverity. Thanks! Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH v2 01/10] argp-parse.c (__argp_input): Don't crash if pstate is NULL
Colin Watson writes:
> On Tue, Dec 07, 2021 at 11:59:01AM -0500, Robbie Harwood wrote:
>> Bruno Haible writes:
>> > I don't think this patch is needed, because:
>> >
>> > 1) The application cannot construct a 'struct argp_state' by itself, since
>> > [1]
>> >says that the 'struct argp_state' contains a member 'pstate' that is
>> >"Private, for use by the argp implementation.".
>> >
>> > 2) The only place in the gnulib / glibc code where a 'struct argp_state' is
>> >being constructed, is in function parser_init, invoked from
>> > 'argp_parse',
>> >and there a non-NULL value is assigned.
>> >
>> > In other words, there is no way, compliant with the documented API, that a
>> > NULL pointer can arise as state->pstate.
>> >
>> > Bruno
>> >
>> > [1]
>> > https://www.gnu.org/software/libc/manual/html_node/Argp-Parsing-State.html
>>
>> Thanks for taking a look. I don't have more information on this one
>> beyond the little that's in the commit, so unless Colin remembers why
>> this was needed in 2011, I'll propose grub drop it at some point.
>
> The original problem was:
>
> https://bugs.debian.org/612692
>
> At the time, __argp_help looked like this:
>
> void __argp_help (const struct argp *argp, FILE *stream,
> unsigned flags, char *name)
> {
> struct argp_state state;
> memset (, 0, sizeof state);
> state.root_argp = argp;
> _help (argp, , stream, flags, name);
> }
>
> As a result it was possible at the time to encounter the case where
> state was non-NULL but state->pstate was NULL. However, this seems to
> have been fixed differently some time ago:
>
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=14a582531c
>
> So I think it's probably fine to drop this patch from GRUB.
Appreciate the follow-up! That sounds reasonable to me.
Be well,
--Robbie
signature.asc
Description: PGP signature
Re: [PATCH v2 08/10] Fix up a bunch of "gcc -Werror=sign-compare" complaints
Paul Eggert 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
signature.asc
Description: PGP signature
Re: [PATCH v2 05/10] Make gnulib's regcomp not abort()
Paul Eggert writes: > On 12/1/21 19:20, Paul Eggert wrote: >> 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'? > > A followup idea: would it help to replace 'abort ()' with 'DEBUG_ASSERT > (false)', or to replace 'if (!X) abort ();' with 'DEBUG_ASSERT (X);'? Unfortunately Vladimir has not so far been responding to gnulib emails, However, I don't believe we have an implementation of abort() that can be called. (We have grub_abort() instead.) If that's the correct reason, then DEBUG_ASSERT would work and I can make that change. Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH v2 03/10] gnulib/regexec: Resolve unused variable
Paul Eggert writes: > 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?). (Same framing as the previous one - Coverity reported, etc.) I believe this change predates your 79f8ee4e389f8cb1339f8abed9a7d29816e2a2d4 by several months (grub currently starts at d271f868a8df9bbec29049d01e056481b7a1a263) and should be therefore safe for grub to drop if it ever rolls forward. Thanks! Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH v2 02/10] gnulib/regexec: Fix possible null-dereference
Paul Eggert writes: > 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? Having a CID number means this is fixing a Coverity issue. I don't have access to that, so maybe Darren/Daniel can provide more information here. My *guess* is that Coverity has noticed that `mctx->state_log` is checked against NULL in many other places in that file, and was unable to prove to itself that it couldn't be NULL there too. If that's the case, a DEBUG_ASSERT would presumably do the trick better. Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH v2 01/10] argp-parse.c (__argp_input): Don't crash if pstate is NULL
Bruno Haible writes:
> Robbie Harwood wrote:
>> 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;
>
> I don't think this patch is needed, because:
>
> 1) The application cannot construct a 'struct argp_state' by itself, since [1]
>says that the 'struct argp_state' contains a member 'pstate' that is
>"Private, for use by the argp implementation.".
>
> 2) The only place in the gnulib / glibc code where a 'struct argp_state' is
>being constructed, is in function parser_init, invoked from 'argp_parse',
>and there a non-NULL value is assigned.
>
> In other words, there is no way, compliant with the documented API, that a
> NULL pointer can arise as state->pstate.
>
> Bruno
>
> [1] https://www.gnu.org/software/libc/manual/html_node/Argp-Parsing-State.html
Thanks for taking a look. I don't have more information on this one
beyond the little that's in the commit, so unless Colin remembers why
this was needed in 2011, I'll propose grub drop it at some point.
Be well,
--Robbie
signature.asc
Description: PGP signature
[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/a
[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 == WE
[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
[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
Re: [PATCH 01/11] Fix base64 module to work with grub codebase
Paul Eggert writes: > On 10/28/21 12:32, Robbie Harwood wrote: > >> I don't know why Patrick chose to >> not use that instead, but a local test seems to work. > > Is grub2 intended to be portable to compilers that don't support > ? If that's the issue, I suggest that grub2 stop worrying > that. Surely every compiler of interest to grub2 supports > already. And if you really need to support older compilers, the Gnulib > stdbool module should suffice. > >> grub2 shims out config.h for some build targets (e.g., when not building >> utilities). > > Why does it need to do that? Is this because of cross-building, and > where is for the utilities platform which is not the same as > the target platform? If so, that suggests that you should run two > 'configure' instances, one for the utilities and one for the target, and > compile the base64 module twice if it's used in both places. I'll defer to Daniel on why things are the way they are, but I don't disagree with you. Be well, --Robbie signature.asc Description: PGP signature
Re: [PATCH 01/11] Fix base64 module to work with grub codebase
Simon Josefsson writes: > Robbie Harwood writes: > >> The gnulib module makes use of booleans via the header. As >> GRUB does not provide any POSIX wrapper header for this, but instead >> implements support for bool in , we need to patch >> base64.h to not use anymore. We unfortunately cannot include >> instead, as it would then use gnulib's internal header >> while compiling the gnulib object but our own when >> including it in a GRUB module. Because of this, the patch replaces the >> include with a direct typedef. > > Thanks for trying to upstream diverged gnulib code! > > I think this patch is wrong -- gnulib includes a stdbool.h replacement > you can you use, and the base64 module depends on the stdbool module > already. Is there a problem with the stdbool module? If so, let's fix > that. This is helpful feedback, thank you. I don't know why Patrick chose to not use that instead, but a local test seems to work. >> A second fix is required to make available _GL_ATTRIBUTE_CONST, which >> is provided by the configure script. As base64.h does not include >> , it is thus not available and results in a compile error. >> This is fixed by adding an include of . > > I think I agree that this is a problem, but your solutions seems wrong. > There are plenty of header files in gnulib that relies on definitions in > config.h created by the m4 macro that came with the hader file, yet the > header file do not #include config.h as usually that is supposed to be > done by the .c file that include the (say) base64.h header file. I'm > not sure assumption is documented anywhere, and if so we should document > it. I think this is how it is supposed to work, but if it isn't, we > should try to come up with a solution for it and document that. I probably would have been better served by editing the patch description a bit more here from the original. To start over from having poked at it more: grub2 shims out config.h for some build targets (e.g., when not building utilities). So the bits that gnulib's configuration have created end up in config-util.h - which only sometimes is what config.h is. Base64 is used in the luks2 code, which is not always a utility. It would be nice to have the files gnulib gives us be "complete" and not need to know things about the compiler, but honestly this seems a bit like a problem of grub2's making. So I think our way forward is to move where we nerf _GL_ATTRIBUTE_CONST in grub2. I've tested that this works and will submit to grub2. Longer-term, this problem could be avoided by dropping the const attribute from isbase64(). Since uchar_in_range is a macro, b64 is const, and to_uchar() doesn't do anything, the compiler should be able to infer this anyway. (Adding an inline marker to to_uchar() might help with this.) What do you think? grub2 proposed change: https://lists.gnu.org/archive/html/grub-devel/2021-10/msg00208.html Assuming grub2 accepts this in some form, I think we can consider this patch dropped from the series. Be well, --Robbie signature.asc Description: PGP signature
[PATCH 04/11] 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 e48fe5333..90330ef39 100644 --- a/lib/regexec.c +++ b/lib/regexec.c @@ -815,7 +815,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 11/11] 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 03/11] 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 6aeba3c0b..e48fe5333 100644
--- a/lib/regexec.c
+++ b/lib/regexec.c
@@ -1675,6 +1675,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 06/11] 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 887e5b506..4a106ff59 100644
--- a/lib/regcomp.c
+++ b/lib/regcomp.c
@@ -528,9 +528,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. */
@@ -1136,7 +1136,7 @@ optimize_utf8 (re_dfa_t *dfa)
}
break;
default:
- abort ();
+ break;
}
if (mb_chars || has_period)
--
2.33.0
[PATCH 01/11] Fix base64 module to work with grub codebase
From: Patrick Steinhardt
The gnulib module makes use of booleans via the header. As
GRUB does not provide any POSIX wrapper header for this, but instead
implements support for bool in , we need to patch
base64.h to not use anymore. We unfortunately cannot include
instead, as it would then use gnulib's internal header
while compiling the gnulib object but our own when
including it in a GRUB module. Because of this, the patch replaces the
include with a direct typedef.
A second fix is required to make available _GL_ATTRIBUTE_CONST, which
is provided by the configure script. As base64.h does not include
, it is thus not available and results in a compile error.
This is fixed by adding an include of .
Signed-off-by: Patrick Steinhardt
Reviewed-by: Daniel Kiper
[rharw...@redhat.com: squished commit messages, wrote subject]
Signed-off-by: Robbie Harwood
---
lib/base64.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/base64.h b/lib/base64.h
index e58ccfb1f..fd108fb35 100644
--- a/lib/base64.h
+++ b/lib/base64.h
@@ -21,8 +21,14 @@
/* Get idx_t. */
# include
-/* Get bool. */
-# include
+#ifndef GRUB_POSIX_BOOL_DEFINED
+typedef enum { false = 0, true = 1 } bool;
+#define GRUB_POSIX_BOOL_DEFINED 1
+#endif
+
+#ifndef _GL_ATTRIBUTE_CONST
+# define _GL_ATTRIBUTE_CONST /* empty */
+#endif
# ifdef __cplusplus
extern "C" {
--
2.33.0
[PATCH 02/11] 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 10/11] 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 05/11] 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/a
[PATCH 08/11] 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 09/11] Fix up a bunch of "gcc -Werror=sign-compare" complaints
From: Peter Jones
[rharw...@redhat.com: rebase conflicts in regexec]
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 4a106ff59..a33a74488 100644
--- a/lib/regcomp.c
+++ b/lib/regcomp.c
@@ -300,7 +300,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)
@@ -321,7 +321,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))
@@ -582,7 +582,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)
@@ -893,7 +894,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))
@@ -1082,7 +1084,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;
@@ -1177,12 +1179,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)
{
@@ -1627,7 +1629,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);
@@ -1635,7 +1637,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))
@@ -1651,7 +1653,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;
@@ -2717,7 +2719,7 @@ build_range_exp (const reg_syntax_t syntax,
# ifdef RE_ENABLE_I18N
{
-wchar_t wc;
+wint_t wc;
wint_t start_wc;
wint_t end_wc;
@@ -2728,9 +2730,9 @@ build_range_exp (const reg_syntax_t syntax,
: ((end_elem->type == COLL_SYM) ? end_elem->opr.name[0]
: 0));
start_wc = ((start_elem->type == SB_CHAR || start_elem->type == COLL_SYM)
- ? parse_byte (start_ch, mbcset) : start_elem->opr.wch);
+ ? parse_byte (start_ch, mbcset) : (wint_t)start_elem->opr.wch);
end_wc = ((end_elem->type == SB_CHAR || end_elem->type == C
[PATCH 00/11] Code hygiene fixes from grub
Hello gnulib, grub2 uses a patched gnulib, on top of d271f868a8df9bbec29049d01e056481b7a1a263 (from 2019-01-05). There are nine patches carried in the grub2 source tree. Three have been resolved upstream since divergence; the reamining six are the first in this series. Downstream in Fedora, we add an additional five patches, which constitute the remainder of the series. (Conveniently, this is all of the ones by Peter Jones, if that helps to keep track.) I've done my best to reconstruct commit messages and authorship information for these, though the way grub2 stored them before 2019 does not always preserve commit information. I'm also just a messenger trying to reduce codebase divergence, and am not familiar enough with gnulib to have given them serious review. 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 Patrick Steinhardt (1): Fix base64 module to work with grub codebase 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/base64.h | 10 -- 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 ++-- 12 files changed, 167 insertions(+), 63 deletions(-) -- 2.33.0
[PATCH 07/11] 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
