Re: [RFC/PATCH 4/7] grep: make the behavior for \0 in patterns sane
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jun 27, 2019, 02:03 UTC
- Message-ID
- <20190627020259.GB9224@genre.crustytoothpaste.net>
- In-Reply-To
- <20190626000329.32475-5-avarab@gmail.com>
On 2019-06-26 at 00:03:26, Ævar Arnfjörð Bjarmason wrote:
Show 11 quoted lines
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > index 2d27969057..c89fb569e3 100644 > --- a/Documentation/git-grep.txt > +++ b/Documentation/git-grep.txt > @@ -271,6 +271,23 @@ providing this option will cause it to die. > > -f <file>:: > Read patterns from <file>, one per line. > ++ > +Passing the pattern via <file> allows for providing a search pattern > +containing a \0.
In this case, I think it's easier if we write this as "NUL" or "NUL byte", since I think you mean a literal byte with value 0 and not the literal string "\0". I certainly find myself a bit confused, at least, and I expect others will as well.
Show 43 quoted lines
> diff --git a/grep.c b/grep.c
> index d3e6111c46..261bd3a342 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -368,18 +368,6 @@ 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.
> - */
> - if (memchr(s, 0, len))
> - return 1;
> -
> - return 0;
> -}
> -
> #ifdef USE_LIBPCRE1
> static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)
> {
> @@ -668,9 +656,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 ||
> - has_null(p->pattern, p->patternlen) ||
> - is_fixed(p->pattern, p->patternlen))
> + if (opt->fixed || is_fixed(p->pattern, p->patternlen))
> p->fixed = !p->ignore_case || !has_non_ascii(p->pattern);
>
> if (p->fixed) {
> @@ -678,7 +664,12 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
> kwsincr(p->kws, p->pattern, p->patternlen);
> kwsprep(p->kws);
> return;
> - } else if (opt->fixed) {
> + }
> +
> + if (memchr(p->pattern, 0, p->patternlen) && !opt->pcre2)
> + die(_("given pattern contains NULL byte (via -f <file>). This is only supported with -P under PCRE v2"));We probably want to write this as "NUL" as well.
Otherwise, I'm okay with this change. I didn't expect Git to handle literal NULs in patterns and I'm surprised that it ever worked.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204