Re: [PATCH 12/26] checkout: fix memory leak
Hi Junio, On Wed, 26 Apr 2017, Junio C Hamano wrote: > Johannes Schindelin writes: > > > Discovered via Coverity. > > > > Signed-off-by: Johannes Schindelin > > --- > > 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
Re: [PATCH 12/26] checkout: fix memory leak
Johannes Schindelin writes: > Discovered via Coverity. > > Signed-off-by: Johannes Schindelin > --- > 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.
[PATCH 12/26] checkout: fix memory leak
Discovered via Coverity. Signed-off-by: Johannes Schindelin --- 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; } -- 2.12.2.windows.2.800.gede8f145e06