Re: [PATCH 21/29] grep: factor test for \0 in grep patterns into a function

On Thu, May 11, 2017 at 10:15 PM, Brandon Williams [off-list ref] wrote:

On 05/11, Ævar Arnfjörð Bjarmason wrote:

quoted

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 <redacted>
---
 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?

Just losing the forest for the trees in rebasing this giant, willdo in
v2, i.e. just make this a function in the right place in this change.

quoted

+     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