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

On Tue, Sep 21, 2021 at 01:43:05AM -0400, Jeff King wrote:

So yeah, I'm sure it could be rewritten around memrchr() or something,
but I doubt it would be much shorter, and the chance of introducing an
off-by-one seems non-trivial. :)

Considering I am writing it, it is most likely warranted ;)

but it doesn't look that bad IMHO

Carlo

PS. I tested it in macOS with the compatibility layer that will be needed
------ 8> -------
Subject: [PATCH] grep: retire strip_timestamp()

After recent changes, the name is no longer valid, as the function
doesn't strip anything.

Having the code in the main function also helps with readability

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
 grep.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/grep.c b/grep.c
index 5b1f2da4d3..56fd86a7d8 100644
--- a/grep.c
+++ b/grep.c

@@ -922,18 +922,6 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,
 	return hit;
 }
 
-static void strip_timestamp(char *bol, char **eol_p)
-{
-	char *eol = *eol_p;
-
-	while (bol < --eol) {
-		if (*eol != '>')
-			continue;
-		*eol_p = ++eol;
-		break;
-	}
-}
-
 static struct {
 	const char *field;
 	size_t len;

@@ -965,9 +953,12 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 		bol += len;
 		switch (p->field) {
 		case GREP_HEADER_AUTHOR:
-		case GREP_HEADER_COMMITTER:
-			strip_timestamp(bol, &eol);
+		case GREP_HEADER_COMMITTER: {
+			char *em = memrchr(bol, '>', eol - bol);
+			if (em)
+				eol = em + 1;
 			break;
+		}
 		default:
 			break;
 		}

-- 
2.33.0.911.gbe391d4e11