Re: git grep -P fatal: pcre_exec failed with error code -8

From: Jeff King <peff@peff.net>
Date: Nov 6, 2017, 10:31 UTC
Message-ID: <20171106103125.fwtrxv6zycrbihcv@sigill.intra.peff.net>
In-Reply-To: <4e2bc579-429f-9927-4502-5929e5235740@aegee.org>

On Sun, Nov 05, 2017 at 10:41:17AM +0100, Дилян Палаузов wrote:

> I understand that the PCRE's stack can get exhausted for some files, but in
> such cases, git grep shall proceed with the other files, and print at the
> end/stderr for which files the pattern was not applied.  Such behaviour
> would be more usefull than the current one.

Yes, I had a similar thought. It does feel a little funny for us to basically treat an error as "no match" for non-interactive use, but then the current behavior works out to be more or less the same (we return an error code which most shell scripts would interpret as failure).

IOW, I think something like this is probably the right direction:

diff --git a/grep.c b/grep.c
index ce6a48e634..2c152e5908 100644
--- a/grep.c
+++ b/grep.c
@@ -427,7 +427,7 @@ static int pcre1match(struct grep_pat *p, const char *line, const char *eol,
 	}
 
 	if (ret < 0 && ret != PCRE_ERROR_NOMATCH)
-		die("pcre_exec failed with error code %d", ret);
+		warning("pcre_exec failed with error code %d", ret);
 	if (ret > 0) {
 		ret = 0;
 		match->rm_so = ovector[0];

but possibly:

  1. It would be nice to report the filename that we couldn't match on.
     But we don't know it at this level of the code (and it might not be
     a file at all that we are matching). So probably we'd want to pass
     the error much further up the call stack. This is tricky as there
     are multiple regex libraries we can use, and the return value gets
     normalized to 1/0 for hit/not-hit long before we get as far as
     something that knows the filename.

     We might need to do something invasive like adding an extra
     parameter to hold the error message, and passing it through the
     whole stack.

  2. We should still try to exit with an exit code other than "1" to
     indicate we hit an error besides "no lines were found".

  3. Other regex libraries might need similar treatment. Probably
     pcre2match() needs it. It doesn't look like regexec() can ever
     return an error besides REG_NOMATCH.

-Peff