Hi Junio,
On Wed, 26 Apr 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > Discovered via Coverity.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > builtin/checkout.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/builtin/checkout.c b/builtin/checkout.c
> > index bfa5419f335..98f98256608 100644
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -251,6 +251,7 @@ static int checkout_merged(int pos, const struct checkout *state)
> > if (!ce)
> > die(_("make_cache_entry failed for path '%s'"), path);
> > status = checkout_entry(ce, state, NULL);
> > + free(ce);
> > return status;
> > }
>
> Thanks for spotting this one and fixing it.
>
> In case anybody is wondering what the "only to leak" comment before
> this part of the code is about (which by the way may need to be
> updated, as the bulk of its reasoning still applies but at least we
> are no longer leaking with this patch), back when this code was
> written in 2008 or so it wasn't kosher to free cache_entry under
> certain conditions, but it has been a long time since it became OK
> to free any cache entries in later 2011---we should have done this a
> long time ago.
Thanks for the background. The next iteration will change that comment,
too (simply removing "just to leak" and rewrapping to 74 columns/line.
Ciao,
Dscho