On Sat, Sep 29, 2012 at 11:41:27AM +0700, Nguyen Thai Ngoc Duy wrote:
> diff --git a/grep.c b/grep.c
> index 898be6e..8d73995 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -720,7 +720,14 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
> if (strncmp(bol, field, len))
> return 0;
> bol += len;
> - saved_ch = strip_timestamp(bol, &eol);
> + switch (p->field) {
> + case GREP_HEADER_AUTHOR:
> + case GREP_HEADER_COMMITTER:
> + saved_ch = strip_timestamp(bol, &eol);
> + break;
> + default:
> + break;
> + }
Reading this hunk, I wondered what happens to saved_ch if we do not set
it here. Fortunately it is initialized to 0, as we already have to
handle the non-header case. Then later we do this, which does introduce
a new condition (saved_ch was not set, but we trigger the first half of
the conditional):
if (p->token == GREP_PATTERN_HEAD && saved_ch)
*eol = saved_ch;
However, the second half of that conditional (which previously was only
triggered when we tried to split the timestamp, but there was a bogus
line with no ">" on it) prevents us from overwriting *eol.
So I think it is good, but it was non-obvious enough that I wanted to
save other reviewers from investigating it. The rest of the patch looks
good to me, as well.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html