Re: [PATCH 21/29] grep: factor test for \0 in grep patterns into a function
- From
- Brandon Williams <bmwill@google.com>
- Date
- May 11, 2017, 20:15 UTC
- Message-ID
- <20170511201517.GI83655@google.com>
- In-Reply-To
- <20170511091829.5634-22-avarab@gmail.com>
On 05/11, Ævar Arnfjörð Bjarmason wrote:
Show 43 quoted lines
> Factor the test for \0 in grep patterns into a function. Since commit
> 9eceddeec6 ("Use kwset in grep", 2011-08-21) any pattern containing a
> \0 is considered fixed as regcomp() can't handle it.
>
> This limitation was never documented, and other some regular
> expression engines are capable of compiling a pattern containing a
> \0. Factoring this out makes a subsequent change which does that
> smaller.
>
> See a previous commit in this series ("grep: add tests to fix blind
> spots with \0 patterns", 2017-04-21) for further details & rationale.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> grep.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index bf6c2494fd..27de615209 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -394,12 +394,6 @@ static int is_fixed(const char *s, size_t len)
> {
> size_t i;
>
> - /* regcomp cannot accept patterns with NULs so we
> - * consider any pattern containing a NUL fixed.
> - */
> - if (memchr(s, 0, len))
> - return 1;
> -
> for (i = 0; i < len; i++) {
> if (is_regex_special(s[i]))
> return 0;
> @@ -408,6 +402,17 @@ static int is_fixed(const char *s, size_t len)
> return 1;
> }
>
> +static int has_null(const char *s, size_t len)
> +{
> + /* regcomp cannot accept patterns with NULs so when using it
> + * we consider any pattern containing a NUL fixed.
> + */I commented on a later patch but really the comment should be fixed here. And why not simply move this to where you intend it to be at the end of the series now?
Show 21 quoted lines
> + if (memchr(s, 0, len))
> + return 1;
> +
> + return 0;
> +}
> +
> static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)
> {
> struct strbuf sb = STRBUF_INIT;
> @@ -451,7 +456,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
> * simple string match using kws. p->fixed tells us if we
> * want to use kws.
> */
> - if (opt->fixed || is_fixed(p->pattern, p->patternlen))
> + if (opt->fixed || has_null(p->pattern, p->patternlen) || is_fixed(p->pattern, p->patternlen))
> p->fixed = !icase || ascii_only;
> else
> p->fixed = 0;
> --
> 2.11.0
> -- Brandon Williams