git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 3/3] regex: use regexec_buf()

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 21, 2016, 18:24 UTC
Message-ID
<53f3609d99c865d59d7bfd8219a5334339e9e6bc.1474482164.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1474482164.git.johannes.schindelin@gmx.de>

The new regexec_buf() function operates on buffers with an explicitly specified length, rather than NUL-terminated strings.

We need to use this function whenever the buffer we want to pass to regexec() may have been mmap()ed (and is hence not NUL-terminated).

Note: the original motivation for this patch was to fix a bug where
`git diff -G <regex>` would crash. This patch converts more callers,
though, some of which explicitly allocated and constructed
NUL-terminated strings (or worse: modified read-only buffers to insert
NULs).

Some of the buffers actually may be NUL-terminated. As regexec_buf() uses REG_STARTEND where available, but has to fall back to allocating and constructing NUL-terminated strings where REG_STARTEND is not available, this makes the code less efficient in the latter case.

However, given the widespread support for REG_STARTEND, combined with the improved ease of code maintenance, we strike the balance in favor of REG_STARTEND.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 diff.c                  |  3 ++-
 diffcore-pickaxe.c      | 18 ++++++++----------
 grep.c                  | 14 ++------------
 t/t4061-diff-pickaxe.sh |  2 +-
 xdiff-interface.c       | 13 ++++---------
 5 files changed, 17 insertions(+), 33 deletions(-)
diff --git a/diff.c b/diff.c
index c6da383..fb99235 100644
--- a/diff.c
+++ b/diff.c
@@ -952,7 +952,8 @@ static int find_word_boundaries(mmfile_t *buffer, regex_t *word_regex,
 {
 	if (word_regex && *begin < buffer->size) {
 		regmatch_t match[1];
-		if (!regexec(word_regex, buffer->ptr + *begin, 1, match, 0)) {
+		if (!regexec_buf(word_regex, buffer->ptr + *begin,
+				 buffer->size - *begin, 1, match, 0)) {
 			char *p = memchr(buffer->ptr + *begin + match[0].rm_so,
 					'\n', match[0].rm_eo - match[0].rm_so);
 			*end = p ? p - buffer->ptr : match[0].rm_eo + *begin;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 55067ca..9795ca1 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -23,7 +23,6 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)
 {
 	struct diffgrep_cb *data = priv;
 	regmatch_t regmatch;
-	int hold;
 
 	if (line[0] != '+' && line[0] != '-')
 		return;
@@ -33,11 +32,8 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)
 		 * caller early.
 		 */
 		return;
-	/* Yuck -- line ought to be "const char *"! */
-	hold = line[len];
-	line[len] = '\0';
-	data->hit = !regexec(data->regexp, line + 1, 1, &regmatch, 0);
-	line[len] = hold;
+	data->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1,
+				 &regmatch, 0);
 }
 
 static int diff_grep(mmfile_t *one, mmfile_t *two,
@@ -50,9 +46,11 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 	xdemitconf_t xecfg;
 
 	if (!one)
-		return !regexec(regexp, two->ptr, 1, &regmatch, 0);
+		return !regexec_buf(regexp, two->ptr, two->size,
+				    1, &regmatch, 0);
 	if (!two)
-		return !regexec(regexp, one->ptr, 1, &regmatch, 0);
+		return !regexec_buf(regexp, one->ptr, one->size,
+				    1, &regmatch, 0);
 
 	/*
 	 * We have both sides; need to run textual diff and see if
@@ -83,8 +81,8 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
 		regmatch_t regmatch;
 		int flags = 0;
 
-		assert(data[sz] == '\0');
-		while (*data && !regexec(regexp, data, 1, &regmatch, flags)) {
+		while (*data &&
+		       !regexec_buf(regexp, data, sz, 1, &regmatch, flags)) {
 			flags |= REG_NOTBOL;
 			data += regmatch.rm_eo;
 			if (*data && regmatch.rm_so == regmatch.rm_eo)
diff --git a/grep.c b/grep.c
index d7d00b8..1194d35 100644
--- a/grep.c
+++ b/grep.c
@@ -898,17 +898,6 @@ static int fixmatch(struct grep_pat *p, char *line, char *eol,
 	}
 }
 
-static int regmatch(const regex_t *preg, char *line, char *eol,
-		    regmatch_t *match, int eflags)
-{
-#ifdef REG_STARTEND
-	match->rm_so = 0;
-	match->rm_eo = eol - line;
-	eflags |= REG_STARTEND;
-#endif
-	return regexec(preg, line, 1, match, eflags);
-}
-
 static int patmatch(struct grep_pat *p, char *line, char *eol,
 		    regmatch_t *match, int eflags)
 {
@@ -919,7 +908,8 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,
 	else if (p->pcre_regexp)
 		hit = !pcrematch(p, line, eol, match, eflags);
 	else
-		hit = !regmatch(&p->regexp, line, eol, match, eflags);
+		hit = !regexec_buf(&p->regexp, line, eol - line, 1, match,
+				   eflags);
 
 	return hit;
 }
diff --git a/t/t4061-diff-pickaxe.sh b/t/t4061-diff-pickaxe.sh
index 5929f2e..f0bf50b 100755
--- a/t/t4061-diff-pickaxe.sh
+++ b/t/t4061-diff-pickaxe.sh
@@ -14,7 +14,7 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m "A 4k file"
 '
-test_expect_failure '-G matches' '
+test_expect_success '-G matches' '
 	git diff --name-only -G "^0{4096}$" HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 3bfc69c..060038c 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -214,11 +214,10 @@ struct ff_regs {
 static long ff_regexp(const char *line, long len,
 		char *buffer, long buffer_size, void *priv)
 {
-	char *line_buffer;
 	struct ff_regs *regs = priv;
 	regmatch_t pmatch[2];
 	int i;
-	int result = -1;
+	int result;
 
 	/* Exclude terminating newline (and cr) from matching */
 	if (len > 0 && line[len-1] == '\n') {
@@ -228,18 +227,16 @@ static long ff_regexp(const char *line, long len,
 			len--;
 	}
 
-	line_buffer = xstrndup(line, len); /* make NUL terminated */
-
 	for (i = 0; i < regs->nr; i++) {
 		struct ff_reg *reg = regs->array + i;
-		if (!regexec(&reg->re, line_buffer, 2, pmatch, 0)) {
+		if (!regexec_buf(&reg->re, line, len, 2, pmatch, 0)) {
 			if (reg->negate)
-				goto fail;
+				return -1;
 			break;
 		}
 	}
 	if (regs->nr <= i)
-		goto fail;
+		return -1;
 	i = pmatch[1].rm_so >= 0 ? 1 : 0;
 	line += pmatch[i].rm_so;
 	result = pmatch[i].rm_eo - pmatch[i].rm_so;
@@ -248,8 +245,6 @@ static long ff_regexp(const char *line, long len,
 	while (result > 0 && (isspace(line[result - 1])))
 		result--;
 	memcpy(buffer, line, result);
- fail:
-	free(line_buffer);
 	return result;
 }
 
-- 
2.10.0.windows.1.10.g803177d
Previous: Johannes SchindelinNext: Junio C Hamano
Message 61 of 66 in “Fix a segfault caused by regexec() being called on mmap()ed data”
  1. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 5, 2016
  2. 2/3 diff_populate_filespec: NUL-terminate buffersJohannes Schindelin, Sep 5, 2016
  3. Jeff KingSep 6, 2016
  4. Johannes SchindelinSep 6, 2016
  5. Jeff KingSep 6, 2016
  6. Junio C HamanoSep 7, 2016
  7. Johannes SchindelinSep 8, 2016
  8. Johannes SchindelinSep 8, 2016
  9. Jeff KingSep 8, 2016
  10. Junio C HamanoSep 8, 2016
  11. Johannes SchindelinSep 8, 2016
  12. Jeff KingSep 8, 2016
  13. 3/3 diff_grep: add assertions verifying that the buffers are NUL-terminatedJohannes Schindelin, Sep 5, 2016
  14. Jeff KingSep 6, 2016
  15. Johannes SchindelinSep 6, 2016
  16. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 5, 2016
  17. Jeff KingSep 6, 2016
  18. Johannes SchindelinSep 8, 2016
  19. Junio C HamanoSep 5, 2016
  20. Jeff KingSep 6, 2016
  21. Johannes SchindelinSep 6, 2016
  22. Jeff KingSep 6, 2016
  23. Johannes SchindelinSep 8, 2016
  24. Jeff KingSep 8, 2016
  25. Johannes SchindelinSep 9, 2016
  26. Junio C HamanoSep 9, 2016
  27. Johannes SchindelinSep 6, 2016
  28. Jeff KingSep 6, 2016
  29. Johannes SchindelinSep 6, 2016
  30. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 8, 2016
  31. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  32. 2/3 Introduce a function to run regexec() on non-NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  33. Jeff KingSep 8, 2016
  34. Johannes SchindelinSep 9, 2016
  35. Jeff KingSep 9, 2016
  36. 3/3 Use the newly-introduced regexec_buf() functionJohannes Schindelin, Sep 8, 2016
  37. Johannes SchindelinSep 8, 2016
  38. Jeff KingSep 8, 2016
  39. Jeff KingSep 8, 2016
  40. Jeff KingSep 8, 2016
  41. Ramsay JonesSep 8, 2016
  42. Jeff KingSep 8, 2016
  43. Junio C HamanoSep 8, 2016
  44. Johannes SchindelinSep 8, 2016
  45. Jeff KingSep 8, 2016
  46. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 8, 2016
  47. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  48. 2/3 Introduce a function to run regexec() on non-NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  49. Junio C HamanoSep 8, 2016
  50. 3/3 Use the newly-introduced regexec_buf() functionJohannes Schindelin, Sep 8, 2016
  51. Junio C HamanoSep 8, 2016
  52. Johannes SchindelinSep 9, 2016
  53. Jeff KingSep 9, 2016
  54. Johannes SchindelinSep 9, 2016
  55. Junio C HamanoSep 9, 2016
  56. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 21, 2016
  57. 1/3 regex: -G<pattern> feeds a non NUL-terminated string to regexec() and failsJohannes Schindelin, Sep 21, 2016
  58. 2/3 regex: add regexec_buf() that can work on a non NUL-terminated stringJohannes Schindelin, Sep 21, 2016
  59. Junio C HamanoSep 21, 2016
  60. Johannes SchindelinSep 22, 2016
  61. 3/3 regex: use regexec_buf()Johannes Schindelin, Sep 21, 2016
  62. Junio C HamanoSep 21, 2016
  63. Junio C HamanoSep 21, 2016
  64. Jeff KingSep 21, 2016
  65. Johannes SchindelinSep 25, 2016
  66. Jeff KingSep 21, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.