From: Hamza Mahfooz <someguy@effective-light.com>
Date: Fri, 17 Sep 2021 21:14:56 GMT
Subject: Re: [PATCH v5 2/2] pretty: colorize pattern matches in commit messages
Message-ID: <WCKLZQ.YCP2HWKW7YBB2@effective-light.com>
In-Reply-To: <CAPig+cRfFZ=GAevJ0b9oBGTR8P3MnSg+R43ujSP0dZJL08riqA@mail.gmail.com>


On Fri, Sep 17 2021 at 03:10:12 AM -0400, Eric Sunshine 
<sunshine@sunshineco.com> wrote:
> `buf` and `eol` seem like an accident waiting to happen...
> 
>>  +       line_color = opt->colors[GREP_COLOR_SELECTED];
>>  +       match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
>>  +
>>  +       while (grep_next_match(opt, buf, eol, ctx, &match, field, 
>> eflags)) {
>>  +               if (match.rm_so == match.rm_eo)
>>  +                       break;
>>  +
>>  +               strbuf_grow(sb, strlen(line_color) + 
>> strlen(match_color) +
>>  +                           (2 * strlen(GIT_COLOR_RESET)));
> 
> ... because strbuf_grow() may reallocate the underlying buffer, which
> means that `buf` and `eol` will end up pointing at freed memory, which
> will be accessed by the next call to grep_next_match().

I don't see how it's problematic, since `tmp_sb` isn't modified after 
`buf`
is initialized (until strbuf_release() is called, of course).