From: Jeff King <peff@peff.net>
Date: Wed, 30 Mar 2016 01:31:28 GMT
Subject: Re: [PATCH 4/6] abbrev_sha1_in_line: don't leak memory
Message-ID: <20160330013127.GA6680@sigill.intra.peff.net>
In-Reply-To: <CAPig+cR=Zc2QAQTPAqTt4M-nJRw4StCoqT2hYtmHOvi6LseN_A@mail.gmail.com>

On Tue, Mar 29, 2016 at 09:30:38PM -0400, Eric Sunshine wrote:

> On Tue, Mar 29, 2016 at 9:11 PM, Jeff King <peff@peff.net> wrote:
> > On Tue, Mar 29, 2016 at 05:38:51PM -0700, Stefan Beller wrote:
> >> `split` is of type `struct strbuf **` and just before the new free,
> >> we release the inner strbufs. Make sure to also release the memory
> >> containing the pointers to the individual strbufs.
> >>
> >> Signed-off-by: Stefan Beller <sbeller@google.com>
> >> ---
> >> diff --git a/wt-status.c b/wt-status.c
> >> @@ -1065,7 +1065,7 @@ static void abbrev_sha1_in_line(struct strbuf *line)
> >>       }
> >>       for (i = 0; split[i]; i++)
> >>               strbuf_release(split[i]);
> >> -
> >> +     free(split);
> >>  }
> >
> > I think this can just combine with the for-loop above to become
> > strbuf_list_free().
> 
> The implementation of strbuf_list_free() is this:
> 
>     struct strbuf **s = sbs;
>     while (*s) {
>         strbuf_release(*s);
>         free(*s++);
>     }
>     free(sbs);
> 
> which means that wt-status.c is leaking not only 'split', but also
> each element of split[], right?

Yeah, I didn't notice that, but I think you're right.

-Peff