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

[PATCH 1/2] grep: stop modifying buffer in strip_timestamp

From
Jeff King <peff@peff.net>
Date
Sep 21, 2021, 01:41 UTC
Message-ID
<YUk4R040RBc0aBIF@coredump.intra.peff.net>
In-Reply-To
<YUk3zwuse56v76ze@coredump.intra.peff.net>

When grepping for headers in commit objects, we receive individual lines (e.g., "author Name <email> 1234 -0000"), and then strip off the timestamp to do our match. We do so by writing a NUL byte over the whitespace separator, and then remembering to restore it later.

We had to do it this way when this was added back in a4d7d2c6db (log --author/--committer: really match only with name part, 2008-09-04), because we fed the result directly to regexec(), which expects a NUL-terminated string. But since b7d36ffca0 (regex: use regexec_buf(), 2016-09-21), we have a function which can match on part of a buffer.

So instead of modifying the string, we can instead just move the "eol" pointer, and the rest of the code will do the right thing. This will let the next patch make more use of "const" in grep functions.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this is fairly safe. It would fail subtly if somebody _did_ rely
on the NUL, but we just call into patmatch(), which calls functions
which handle the length-delimited buffer.
 grep.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/grep.c b/grep.c
index 35708ce973..3b372ec29d 100644
--- a/grep.c
+++ b/grep.c
@@ -922,20 +922,16 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,
 	return hit;
 }
 
-static int strip_timestamp(char *bol, char **eol_p)
+static void strip_timestamp(char *bol, char **eol_p)
 {
 	char *eol = *eol_p;
-	int ch;
 
 	while (bol < --eol) {
 		if (*eol != '>')
 			continue;
 		*eol_p = ++eol;
-		ch = *eol;
-		*eol = '\0';
-		return ch;
+		break;
 	}
-	return 0;
 }
 
 static struct {
@@ -952,7 +948,6 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 			     regmatch_t *pmatch, int eflags)
 {
 	int hit = 0;
-	int saved_ch = 0;
 	const char *start = bol;
 
 	if ((p->token != GREP_PATTERN) &&
@@ -968,7 +963,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 		switch (p->field) {
 		case GREP_HEADER_AUTHOR:
 		case GREP_HEADER_COMMITTER:
-			saved_ch = strip_timestamp(bol, &eol);
+			strip_timestamp(bol, &eol);
 			if (eol == end)
 				goto again;
 			break;
@@ -981,7 +976,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 		len = header_field[p->field].len;
 
 		if (strncmp(bol, field, len))
-			goto restore;
+			return 0;
 
 		bol += len;
 	}
@@ -1035,11 +1030,6 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 		pmatch[0].rm_eo += bol - start;
 	}
 
-restore:
-	if (p->token == GREP_PATTERN_HEAD && saved_ch)
-		*eol = saved_ch;
-
-
 	return hit;
 }
 
-- 
2.33.0.1023.gc687d0d3c8
Previous: Jeff KingNext: Jeff King
Message 5 of 10 in “grep: refactor next_match() and match_one_pattern() for external use”
  1. 1/2 grep: refactor next_match() and match_one_pattern() for external useHamza Mahfooz, Sep 21, 2021
  2. 2/2 pretty: colorize pattern matches in commit messagesHamza Mahfooz, Sep 21, 2021
  3. Jeff KingSep 21, 2021
  4. Jeff KingSep 21, 2021
  5. 1/2 grep: stop modifying buffer in strip_timestampJeff King, Sep 21, 2021
  6. 2/2 grep: mark "haystack" buffers as constJeff King, Sep 21, 2021
  7. Jeff KingSep 21, 2021
  8. Hamza MahfoozSep 21, 2021
  9. Jeff KingSep 21, 2021
  10. Jeff KingSep 21, 2021

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.