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

From: Jeff King <peff@peff.net>
Date: Sep 21, 2021, 14:24 UTC
Message-ID: <YUnrI10DXLElenKZ@coredump.intra.peff.net>
In-Reply-To: <7d791c04-d122-1eb9-a84c-939294817395@web.de>

On Tue, Sep 21, 2021 at 09:37:23AM +0200, René Scharfe wrote:

Show 14 quoted lines

> > @@ -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;
> 
> The old code documents the intent via the function name.  The new one
> goes into the nitty-gritty without further explanation, which I find
> harder to read.

Agreed. I do think the conversion is functionally correct, but it doesn't strike me as worth the change.

-Peff