On Mon, Apr 25, 2016 at 8:50 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> @@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img, >> int match_beginning, int match_end) >> { >> int i; >> - char *fixed_buf, *buf, *orig, *target; >> + char *fixed_buf, *orig, *target; >> struct strbuf fixed; >> size_t fixed_len, postlen; >> int preimage_limit; >> @@ -2312,6 +2380,7 @@ static int match_fragment(struct image *img, >> * There must be one non-blank context line that match >> * a line before the end of img. >> */ >> + char *buf; > > patches 1-4 looking good, with no comment from me. Here is the first spot to > comment on. > > It's not clear why we need to declare buf here? Oh wait it is. It's just > moved from the start of the function. But why do it in this patch? > It seems unrelated to the general intent of the patch. No need to reroll > for this nit alone, it just took me a while to figure out it was an unrelated > thing. Yeah, I agree it's a bit unrelated. But rather than add another patch to an already long series just for this, I added the following to the commit message: While at it, let's reduce the scope of "char *buf" in match_fragment(). -- 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