* [PATCH 00/11] Code hygiene fixes from grub @ 2021-10-25 21:55 Robbie Harwood 2021-10-25 21:55 ` [PATCH 01/11] Fix base64 module to work with grub codebase Robbie Harwood ` (10 more replies) 0 siblings, 11 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Robbie Harwood 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 01/11] Fix base64 module to work with grub codebase 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-27 11:15 ` Simon Josefsson via Gnulib discussion list 2021-10-25 21:55 ` [PATCH 02/11] argp-parse.c (__argp_input): Don't crash if pstate is NULL Robbie Harwood ` (9 subsequent siblings) 10 siblings, 1 reply; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Patrick Steinhardt, Daniel Kiper, Robbie Harwood From: Patrick Steinhardt <ps@pks.im> The gnulib module makes use of booleans via the <stdbool.h> header. As GRUB does not provide any POSIX wrapper header for this, but instead implements support for bool in <sys/types.h>, we need to patch base64.h to not use <stdbool.h> anymore. We unfortunately cannot include <sys/types.h> instead, as it would then use gnulib's internal header while compiling the gnulib object but our own <sys/types.h> 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 <config.h>, it is thus not available and results in a compile error. This is fixed by adding an include of <config-util.h>. Signed-off-by: Patrick Steinhardt <ps@pks.im> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> [rharwood@redhat.com: squished commit messages, wrote subject] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 <idx.h> -/* Get bool. */ -# include <stdbool.h> +#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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-10-25 21:55 ` [PATCH 01/11] Fix base64 module to work with grub codebase Robbie Harwood @ 2021-10-27 11:15 ` Simon Josefsson via Gnulib discussion list 2021-10-28 19:32 ` Robbie Harwood 0 siblings, 1 reply; 19+ messages in thread From: Simon Josefsson via Gnulib discussion list @ 2021-10-27 11:15 UTC (permalink / raw) To: Robbie Harwood; +Cc: bug-gnulib, Patrick Steinhardt, Daniel Kiper [-- Attachment #1: Type: text/plain, Size: 1688 bytes --] Robbie Harwood <rharwood@redhat.com> writes: > The gnulib module makes use of booleans via the <stdbool.h> header. As > GRUB does not provide any POSIX wrapper header for this, but instead > implements support for bool in <sys/types.h>, we need to patch > base64.h to not use <stdbool.h> anymore. We unfortunately cannot include > <sys/types.h> instead, as it would then use gnulib's internal header > while compiling the gnulib object but our own <sys/types.h> 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. > A second fix is required to make available _GL_ATTRIBUTE_CONST, which > is provided by the configure script. As base64.h does not include > <config.h>, it is thus not available and results in a compile error. > This is fixed by adding an include of <config-util.h>. 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. /Simon [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 255 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-10-27 11:15 ` Simon Josefsson via Gnulib discussion list @ 2021-10-28 19:32 ` Robbie Harwood 2021-11-09 18:35 ` Simon Josefsson via Gnulib discussion list 2021-11-09 18:52 ` Paul Eggert 0 siblings, 2 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-28 19:32 UTC (permalink / raw) To: Simon Josefsson; +Cc: Patrick Steinhardt, bug-gnulib, Daniel Kiper [-- Attachment #1: Type: text/plain, Size: 3166 bytes --] Simon Josefsson <simon@josefsson.org> writes: > Robbie Harwood <rharwood@redhat.com> writes: > >> The gnulib module makes use of booleans via the <stdbool.h> header. As >> GRUB does not provide any POSIX wrapper header for this, but instead >> implements support for bool in <sys/types.h>, we need to patch >> base64.h to not use <stdbool.h> anymore. We unfortunately cannot include >> <sys/types.h> instead, as it would then use gnulib's internal header >> while compiling the gnulib object but our own <sys/types.h> 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 >> <config.h>, it is thus not available and results in a compile error. >> This is fixed by adding an include of <config-util.h>. > > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 861 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-10-28 19:32 ` Robbie Harwood @ 2021-11-09 18:35 ` Simon Josefsson via Gnulib discussion list 2021-11-09 18:52 ` Paul Eggert 1 sibling, 0 replies; 19+ messages in thread From: Simon Josefsson via Gnulib discussion list @ 2021-11-09 18:35 UTC (permalink / raw) To: Robbie Harwood; +Cc: Patrick Steinhardt, bug-gnulib, Daniel Kiper [-- Attachment #1: Type: text/plain, Size: 1062 bytes --] Robbie Harwood <rharwood@redhat.com> writes: > 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? I'm not really sure what you suggest -- please post a patch for review -- but I'm wondering if you are aware of how to use local patches for gnulib sources? Sometimes this is the best compromise when you can't come up with something that is suitable for gnulib generally, and you don't want to fix your code to use the existing gnulib API. https://www.gnu.org/software/gnulib/manual/html_node/Extending-Gnulib.html I sometimes use this when I disagree with something that is in gnulib upstream, but still track all other changes from gnulib. /Simon [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 255 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-10-28 19:32 ` Robbie Harwood 2021-11-09 18:35 ` Simon Josefsson via Gnulib discussion list @ 2021-11-09 18:52 ` Paul Eggert 2021-11-09 19:34 ` Robbie Harwood 1 sibling, 1 reply; 19+ messages in thread From: Paul Eggert @ 2021-11-09 18:52 UTC (permalink / raw) To: Robbie Harwood, Simon Josefsson Cc: Patrick Steinhardt, bug-gnulib, Daniel Kiper 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 <stdbool.h>? If that's the issue, I suggest that grub2 stop worrying that. Surely every compiler of interest to grub2 supports <stdbool.h> 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 <config.h> 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. > 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? This could hurt performance as the const attribute is for users of base64.h, not for base64.c. A compiler that merely includes base64.h couldn't infer that isbase64 is const and therefore couldn't do common subexpression elimination, unless you use a heavyweight flag like gcc -flto that isn't practical for some applications. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-11-09 18:52 ` Paul Eggert @ 2021-11-09 19:34 ` Robbie Harwood 2021-11-23 16:24 ` Daniel Kiper 0 siblings, 1 reply; 19+ messages in thread From: Robbie Harwood @ 2021-11-09 19:34 UTC (permalink / raw) To: Paul Eggert, Simon Josefsson; +Cc: Patrick Steinhardt, bug-gnulib, Daniel Kiper [-- Attachment #1: Type: text/plain, Size: 1087 bytes --] Paul Eggert <eggert@cs.ucla.edu> 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 > <stdbool.h>? If that's the issue, I suggest that grub2 stop worrying > that. Surely every compiler of interest to grub2 supports <stdbool.h> > 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 <config.h> 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 861 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-11-09 19:34 ` Robbie Harwood @ 2021-11-23 16:24 ` Daniel Kiper 2021-12-09 15:43 ` Daniel Kiper 0 siblings, 1 reply; 19+ messages in thread From: Daniel Kiper @ 2021-11-23 16:24 UTC (permalink / raw) To: Robbie Harwood Cc: Simon Josefsson, Patrick Steinhardt, Paul Eggert, bug-gnulib CC-ing Vladimir... On Tue, Nov 09, 2021 at 02:34:25PM -0500, Robbie Harwood wrote: > Paul Eggert <eggert@cs.ucla.edu> 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 > > <stdbool.h>? If that's the issue, I suggest that grub2 stop worrying > > that. Surely every compiler of interest to grub2 supports <stdbool.h> > > 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 <config.h> 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. Vladimir told me once we are doing that because otherwise we would be leaking too many "OS specific things" into the GRUB core and modules which run on top firmware/bare metal instead of the OS. I hope he will be able to tell us more here... Vladimir? Anyway, I would be more than happy if we could find better way generating configs for the GRUB. Daniel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 01/11] Fix base64 module to work with grub codebase 2021-11-23 16:24 ` Daniel Kiper @ 2021-12-09 15:43 ` Daniel Kiper 0 siblings, 0 replies; 19+ messages in thread From: Daniel Kiper @ 2021-12-09 15:43 UTC (permalink / raw) To: Robbie Harwood Cc: Simon Josefsson, Patrick Steinhardt, Paul Eggert, bug-gnulib Bumping the thread... Please take a look below... On Tue, Nov 23, 2021 at 05:24:31PM +0100, Daniel Kiper wrote: > CC-ing Vladimir... > > On Tue, Nov 09, 2021 at 02:34:25PM -0500, Robbie Harwood wrote: > > Paul Eggert <eggert@cs.ucla.edu> 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 > > > <stdbool.h>? If that's the issue, I suggest that grub2 stop worrying > > > that. Surely every compiler of interest to grub2 supports <stdbool.h> > > > 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 <config.h> 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. > > Vladimir told me once we are doing that because otherwise we would be > leaking too many "OS specific things" into the GRUB core and modules > which run on top firmware/bare metal instead of the OS. I hope he will > be able to tell us more here... > > Vladimir? Vladimir, could you give us examples of leaks? Daniel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 02/11] argp-parse.c (__argp_input): Don't crash if pstate is NULL 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood 2021-10-25 21:55 ` [PATCH 01/11] Fix base64 module to work with grub codebase Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 03/11] gnulib/regexec: Fix possible null-dereference Robbie Harwood ` (8 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Robbie Harwood, Colin Watson From: Colin Watson <cjwatson@ubuntu.com> [rharwood@redhat.com: tweaked commit message] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 03/11] gnulib/regexec: Fix possible null-dereference 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood 2021-10-25 21:55 ` [PATCH 01/11] Fix base64 module to work with grub codebase Robbie Harwood 2021-10-25 21:55 ` [PATCH 02/11] argp-parse.c (__argp_input): Don't crash if pstate is NULL Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 04/11] gnulib/regexec: Resolve unused variable Robbie Harwood ` (7 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Darren Kenny, Daniel Kiper, Robbie Harwood From: Darren Kenny <darren.kenny@oracle.com> 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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 04/11] gnulib/regexec: Resolve unused variable 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (2 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 03/11] gnulib/regexec: Fix possible null-dereference Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 05/11] Fix width computation Robbie Harwood ` (6 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Darren Kenny, Daniel Kiper, Robbie Harwood From: Darren Kenny <darren.kenny@oracle.com> 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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 05/11] Fix width computation 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (3 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 04/11] gnulib/regexec: Resolve unused variable Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 06/11] Make gnulib's regcomp not abort() Robbie Harwood ` (5 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Robbie Harwood From: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 <errno.h> #include <stdarg.h> #include <ctype.h> +#include <wchar.h> #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 \f + +/* 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 (&ps, 0, sizeof (ps)); + + while (ptr < end) + { + wchar_t wc; + size_t s, k; + + s = mbrtowc (&wc, ptr, end - ptr, &ps); + 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" #ifndef SIZE_MAX # define SIZE_MAX ((size_t) -1) @@ -1547,7 +1548,7 @@ argp_args_usage (const struct argp *argp, const struct argp_state *state, /* Manually do line wrapping so that it (probably) won't get wrapped at any embedded spaces. */ - space (stream, 1 + nl - cp); + space (stream, 1 + mbsnwidth (cp, nl - cp, MBSW_STOP_AT_NUL)); __argp_fmtstream_write (stream, cp, nl - cp); } diff --git a/lib/mbswidth.c b/lib/mbswidth.c index 7f599359c..1820f51c1 100644 --- a/lib/mbswidth.c +++ b/lib/mbswidth.c @@ -38,6 +38,14 @@ /* Get INT_MAX. */ #include <limits.h> +#ifndef FALLTHROUGH +# if __GNUC__ < 7 +# define FALLTHROUGH ((void) 0) +# else +# define FALLTHROUGH __attribute__ ((__fallthrough__)) +# endif +#endif + /* Returns the number of columns needed to represent the multibyte character string pointed to by STRING. If a non-printable character occurs, and MBSW_REJECT_UNPRINTABLE is specified, -1 is returned. @@ -90,6 +98,10 @@ mbsnwidth (const char *string, size_t nbytes, int flags) p++; width++; break; + case '\0': + if (flags & MBSW_STOP_AT_NUL) + return width; + FALLTHROUGH; default: /* If we have a multibyte sequence, scan it up to its end. */ { @@ -168,6 +180,9 @@ mbsnwidth (const char *string, size_t nbytes, int flags) { unsigned char c = (unsigned char) *p++; + if (c == 0 && (flags & MBSW_STOP_AT_NUL)) + return width; + if (isprint (c)) { if (width == INT_MAX) diff --git a/lib/mbswidth.h b/lib/mbswidth.h index ec39d7339..744e48eea 100644 --- a/lib/mbswidth.h +++ b/lib/mbswidth.h @@ -40,6 +40,10 @@ extern "C" { control characters and 1 otherwise. */ #define MBSW_REJECT_UNPRINTABLE 2 +/* If this bit is set \0 is treated as the end of string. + Otherwise it's treated as a normal one column width character. */ +#define MBSW_STOP_AT_NUL 4 + /* Returns the number of screen columns needed for STRING. */ #define mbswidth gnu_mbswidth /* avoid clash with UnixWare 7.1.1 function */ -- 2.33.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 06/11] Make gnulib's regcomp not abort() 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (4 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 05/11] Fix width computation Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 07/11] Make CFLAGS less painful Robbie Harwood ` (4 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Robbie Harwood From: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> [rharwood@redhat.com: we wrote a commit message] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 07/11] Make CFLAGS less painful 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (5 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 06/11] Make gnulib's regcomp not abort() Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 08/11] Fix __argp_fmtstream_point()'s return type and comparisons with it Robbie Harwood ` (3 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Peter Jones, Robbie Harwood From: Peter Jones <pjones@redhat.com> Signed-off-by: Peter Jones <pjones@redhat.com> [rharwood@redhat.com: make rpm gunk conditional] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 08/11] Fix __argp_fmtstream_point()'s return type and comparisons with it 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (6 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 07/11] Make CFLAGS less painful Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 09/11] Fix up a bunch of "gcc -Werror=sign-compare" complaints Robbie Harwood ` (2 subsequent siblings) 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Peter Jones, Robbie Harwood From: Peter Jones <pjones@redhat.com> 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 <pjones@redhat.com> Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 09/11] Fix up a bunch of "gcc -Werror=sign-compare" complaints 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (7 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 08/11] Fix __argp_fmtstream_point()'s return type and comparisons with it Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 10/11] Paper over a stringop-overflow warning about wide char handling Robbie Harwood 2021-10-25 21:55 ` [PATCH 11/11] Fixup for -Werror=ignored-qualifiers issues Robbie Harwood 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Peter Jones, Robbie Harwood From: Peter Jones <pjones@redhat.com> [rharwood@redhat.com: rebase conflicts in regexec] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 (&state, '\0', sizeof (state)); - if (__mbrtowc (&wc, (const char *) buf, p - buf, + if ((ssize_t)__mbrtowc (&wc, (const char *) buf, p - buf, &state) == p - buf && (__wcrtomb ((char *) buf, __towlower (wc), &state) != (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 == COLL_SYM) - ? parse_byte (end_ch, mbcset) : end_elem->opr.wch); + ? parse_byte (end_ch, mbcset) : (wint_t)end_elem->opr.wch); if (start_wc == WEOF || end_wc == WEOF) return REG_ECOLLATE; else if (__glibc_unlikely ((syntax & RE_NO_EMPTY_RANGES) diff --git a/lib/regex_internal.c b/lib/regex_internal.c index aefcfa2f5..405a97267 100644 --- a/lib/regex_internal.c +++ b/lib/regex_internal.c @@ -146,7 +146,7 @@ re_string_realloc_buffers (re_string_t *pstr, Idx new_buf_len) /* Avoid overflow in realloc. */ const size_t max_object_size = MAX (sizeof (wint_t), sizeof (Idx)); - if (__glibc_unlikely (MIN (IDX_MAX, SIZE_MAX / max_object_size) + if (__glibc_unlikely ((ssize_t)MIN (IDX_MAX, SIZE_MAX / max_object_size) < new_buf_len)) return REG_ESPACE; @@ -403,7 +403,7 @@ build_wcs_upper_buffer (re_string_t *pstr) { size_t i; - if (byte_idx + mbcdlen > pstr->bufs_len) + if ((ssize_t)(byte_idx + mbcdlen) > pstr->bufs_len) { pstr->cur_state = prev_st; break; @@ -753,7 +753,7 @@ re_string_reconstruct (re_string_t *pstr, Idx idx, int eflags) memset (&cur_state, 0, sizeof (cur_state)); mbclen = __mbrtowc (&wc2, (const char *) pp, mlen, &cur_state); - if (raw + offset - p <= mbclen + if (raw + offset - p <= (ssize_t)mbclen && mbclen < (size_t) -2) { memset (&pstr->cur_state, '\0', diff --git a/lib/regexec.c b/lib/regexec.c index 90330ef39..9ad488044 100644 --- a/lib/regexec.c +++ b/lib/regexec.c @@ -34,7 +34,7 @@ static void sift_ctx_init (re_sift_context_t *sctx, re_dfastate_t **sifted_sts, static reg_errcode_t re_search_internal (const regex_t *preg, const char *string, Idx length, Idx start, Idx last_start, Idx stop, - size_t nmatch, regmatch_t pmatch[], + ssize_t nmatch, regmatch_t pmatch[], int eflags); static regoff_t re_search_2_stub (struct re_pattern_buffer *bufp, const char *string1, Idx length1, @@ -63,7 +63,7 @@ static reg_errcode_t push_fail_stack (struct re_fail_stack_t *fs, re_node_set *eps_via_nodes); static reg_errcode_t set_regs (const regex_t *preg, const re_match_context_t *mctx, - size_t nmatch, regmatch_t *pmatch, + ssize_t nmatch, regmatch_t *pmatch, bool fl_backtrack); static reg_errcode_t free_fail_stack_return (struct re_fail_stack_t *fs); @@ -484,7 +484,7 @@ re_copy_regs (struct re_registers *regs, regmatch_t *pmatch, Idx nregs, { /* Yes. If we need more elements than were already allocated, reallocate them. If we need fewer, just leave it alone. */ - if (__glibc_unlikely (need_regs > regs->num_regs)) + if (__glibc_unlikely (need_regs > (ssize_t)regs->num_regs)) { regoff_t *new_start = re_realloc (regs->start, regoff_t, need_regs); regoff_t *new_end; @@ -505,7 +505,7 @@ re_copy_regs (struct re_registers *regs, regmatch_t *pmatch, Idx nregs, { DEBUG_ASSERT (regs_allocated == REGS_FIXED); /* This function may not be called with REGS_FIXED and nregs too big. */ - DEBUG_ASSERT (nregs <= regs->num_regs); + DEBUG_ASSERT ((ssize_t)nregs <= regs->num_regs); rval = REGS_FIXED; } @@ -515,7 +515,7 @@ re_copy_regs (struct re_registers *regs, regmatch_t *pmatch, Idx nregs, regs->start[i] = pmatch[i].rm_so; regs->end[i] = pmatch[i].rm_eo; } - for ( ; i < regs->num_regs; ++i) + for ( ; i < (ssize_t)regs->num_regs; ++i) regs->start[i] = regs->end[i] = -1; return rval; @@ -584,7 +584,7 @@ re_exec (const char *s) static reg_errcode_t __attribute_warn_unused_result__ re_search_internal (const regex_t *preg, const char *string, Idx length, - Idx start, Idx last_start, Idx stop, size_t nmatch, + Idx start, Idx last_start, Idx stop, ssize_t nmatch, regmatch_t pmatch[], int eflags) { reg_errcode_t err; @@ -604,7 +604,7 @@ re_search_internal (const regex_t *preg, const char *string, Idx length, ? preg->fastmap : NULL); RE_TRANSLATE_TYPE t = preg->translate; - extra_nmatch = (nmatch > preg->re_nsub) ? nmatch - (preg->re_nsub + 1) : 0; + extra_nmatch = (nmatch > (ssize_t)preg->re_nsub) ? nmatch - (preg->re_nsub + 1) : 0; nmatch -= extra_nmatch; /* Check if the DFA haven't been compiled. */ @@ -653,9 +653,10 @@ re_search_internal (const regex_t *preg, const char *string, Idx length, if (nmatch > 1 || dfa->has_mb_node) { /* Avoid overflow. */ - if (__glibc_unlikely ((MIN (IDX_MAX, SIZE_MAX / sizeof (re_dfastate_t *)) - <= mctx.input.bufs_len))) - { + if (__glibc_unlikely ((ssize_t)MIN (IDX_MAX, + SIZE_MAX / sizeof (re_dfastate_t *)) + <= mctx.input.bufs_len)) + { err = REG_ESPACE; goto free_return; } @@ -920,7 +921,8 @@ prune_impossible_nodes (re_match_context_t *mctx) halt_node = mctx->last_node; /* Avoid overflow. */ - if (__glibc_unlikely (MIN (IDX_MAX, SIZE_MAX / sizeof (re_dfastate_t *)) + if (__glibc_unlikely ((ssize_t)MIN (IDX_MAX, + SIZE_MAX / sizeof (re_dfastate_t *)) <= match_last)) return REG_ESPACE; @@ -1381,7 +1383,7 @@ pop_fail_stack (struct re_fail_stack_t *fs, Idx *pidx, Idx nregs, static reg_errcode_t __attribute_warn_unused_result__ -set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, +set_regs (const regex_t *preg, const re_match_context_t *mctx, ssize_t nmatch, regmatch_t *pmatch, bool fl_backtrack) { const re_dfa_t *dfa = preg->buffer; @@ -1416,7 +1418,7 @@ set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch, regmatch_t *prev_idx_match = regmatch_list_begin (&prev_match); memcpy (prev_idx_match, pmatch, sizeof (regmatch_t) * nmatch); - for (idx = pmatch[0].rm_so; idx <= pmatch[0].rm_eo ;) + for (idx = pmatch[0].rm_so; (ssize_t)idx <= (long)pmatch[0].rm_eo ;) { update_regs (dfa, pmatch, prev_idx_match, cur_node, idx, nmatch); @@ -2860,7 +2862,8 @@ check_arrival (re_match_context_t *mctx, state_array_t *path, Idx top_node, if (__glibc_unlikely (IDX_MAX - old_alloc < incr_alloc)) return REG_ESPACE; new_alloc = old_alloc + incr_alloc; - if (__glibc_unlikely (SIZE_MAX / sizeof (re_dfastate_t *) < new_alloc)) + if (__glibc_unlikely ((ssize_t)(SIZE_MAX / sizeof (re_dfastate_t *)) + < new_alloc)) return REG_ESPACE; new_array = re_realloc (path->array, re_dfastate_t *, new_alloc); if (__glibc_unlikely (new_array == NULL)) @@ -3996,7 +3999,8 @@ extend_buffers (re_match_context_t *mctx, int min_len) re_string_t *pstr = &mctx->input; /* Avoid overflow. */ - if (__glibc_unlikely (MIN (IDX_MAX, SIZE_MAX / sizeof (re_dfastate_t *)) / 2 + if (__glibc_unlikely ((ssize_t)MIN (IDX_MAX, + SIZE_MAX / sizeof (re_dfastate_t *)) / 2 <= pstr->bufs_len)) return REG_ESPACE; @@ -4066,7 +4070,7 @@ match_ctx_init (re_match_context_t *mctx, int eflags, Idx n) size_t max_object_size = MAX (sizeof (struct re_backref_cache_entry), sizeof (re_sub_match_top_t *)); - if (__glibc_unlikely (MIN (IDX_MAX, SIZE_MAX / max_object_size) < n)) + if (__glibc_unlikely ((ssize_t)MIN (IDX_MAX, SIZE_MAX / max_object_size) < n)) return REG_ESPACE; mctx->bkref_ents = re_malloc (struct re_backref_cache_entry, n); diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c index d9b669d15..360ca6948 100644 --- a/lib/vasnprintf.c +++ b/lib/vasnprintf.c @@ -5573,7 +5573,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #endif #if !USE_SNPRINTF - if (count >= tmp_length) + if (count >= (ssize_t)tmp_length) /* tmp_length was incorrectly calculated - fix the code above! */ abort (); @@ -5663,7 +5663,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #if DCHAR_IS_TCHAR && !USE_SNPRINTF /* Make room for the result. */ - if (count > allocated - length) + if (count > (ssize_t)(allocated - length)) { /* Need at least count elements. But allocate proportionally. */ -- 2.33.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 10/11] Paper over a stringop-overflow warning about wide char handling 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (8 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 09/11] Fix up a bunch of "gcc -Werror=sign-compare" complaints Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 2021-10-25 21:55 ` [PATCH 11/11] Fixup for -Werror=ignored-qualifiers issues Robbie Harwood 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Peter Jones, Robbie Harwood From: Peter Jones <pjones@redhat.com> [rharwood@redhat.com: rewrote commit message] Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 11/11] Fixup for -Werror=ignored-qualifiers issues 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood ` (9 preceding siblings ...) 2021-10-25 21:55 ` [PATCH 10/11] Paper over a stringop-overflow warning about wide char handling Robbie Harwood @ 2021-10-25 21:55 ` Robbie Harwood 10 siblings, 0 replies; 19+ messages in thread From: Robbie Harwood @ 2021-10-25 21:55 UTC (permalink / raw) To: bug-gnulib; +Cc: Peter Jones, Robbie Harwood From: Peter Jones <pjones@redhat.com> Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Robbie Harwood <rharwood@redhat.com> --- 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) \f /* 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-12-09 15:44 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-25 21:55 [PATCH 00/11] Code hygiene fixes from grub Robbie Harwood 2021-10-25 21:55 ` [PATCH 01/11] Fix base64 module to work with grub codebase Robbie Harwood 2021-10-27 11:15 ` Simon Josefsson via Gnulib discussion list 2021-10-28 19:32 ` Robbie Harwood 2021-11-09 18:35 ` Simon Josefsson via Gnulib discussion list 2021-11-09 18:52 ` Paul Eggert 2021-11-09 19:34 ` Robbie Harwood 2021-11-23 16:24 ` Daniel Kiper 2021-12-09 15:43 ` Daniel Kiper 2021-10-25 21:55 ` [PATCH 02/11] argp-parse.c (__argp_input): Don't crash if pstate is NULL Robbie Harwood 2021-10-25 21:55 ` [PATCH 03/11] gnulib/regexec: Fix possible null-dereference Robbie Harwood 2021-10-25 21:55 ` [PATCH 04/11] gnulib/regexec: Resolve unused variable Robbie Harwood 2021-10-25 21:55 ` [PATCH 05/11] Fix width computation Robbie Harwood 2021-10-25 21:55 ` [PATCH 06/11] Make gnulib's regcomp not abort() Robbie Harwood 2021-10-25 21:55 ` [PATCH 07/11] Make CFLAGS less painful Robbie Harwood 2021-10-25 21:55 ` [PATCH 08/11] Fix __argp_fmtstream_point()'s return type and comparisons with it Robbie Harwood 2021-10-25 21:55 ` [PATCH 09/11] Fix up a bunch of "gcc -Werror=sign-compare" complaints Robbie Harwood 2021-10-25 21:55 ` [PATCH 10/11] Paper over a stringop-overflow warning about wide char handling Robbie Harwood 2021-10-25 21:55 ` [PATCH 11/11] Fixup for -Werror=ignored-qualifiers issues Robbie Harwood
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).