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

On Mon, Sep 20, 2021 at 10:24 PM Eric Sunshine [off-list ref] wrote:

On Tue, Sep 21, 2021 at 1:18 AM Carlo Arenas [off-list ref] wrote:

quoted

On Mon, Sep 20, 2021 at 9:09 PM Jeff King [off-list ref] wrote:

quoted

@@ -971,7 +966,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);

Why not something like (plus added error handling, even if it seems
the original didn't have them)?

  eol = strrchr(bol, '>');

strrchr() would search backward from the NUL, not from `eol`, thus
would not be a faithful conversion (and might not be safe, though I
didn't dig through all the callers).

of course; I meant memrchr, which I guess will need to have also a
compat version[1]
this is the only caller for strip_timestamp()

[1] https://www.gnu.org/software/gnulib/manual/html_node/memrchr.html