Re: [PATCH] repository: prevent memory leak when releasing ref stores

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > However, I am fuzzy on the existing uses in the backend
> > implementation.  For example:
> >
> >         static void files_ref_store_release(struct ref_store *ref_store)
> >         {
> >                 struct files_ref_store *refs = files_downcast(ref_store, 0, "release");
> >                 free_ref_cache(refs->loose);
> >                 free(refs->gitcommondir);
> >                 ref_store_release(refs->packed_ref_store);
> >         }
> >
> > The packed-ref-store is "released" here, as part of "releasing" the
> > files-ref-store that uses it as a fallback backend.  The caller of
> > files_ref_store_release() is refs.c:ref_store_release()
> >
> >         void ref_store_release(struct ref_store *ref_store)
> >         {
> >                 ref_store->be->release(ref_store);
> >                 free(ref_store->gitdir);
> >         }
> >
> > So if you have a files based ref store, when you are done you'd be
> > calling ref_store_release() on it, releasing the resources held by
> > the files_ref_store structure, but I do not know who frees the
> > packed_ref_store allocated by files_ref_store_init().  Perhaps it is
> > already leaking?  If that is the case then an API update like I
> > suggested above would make even more sense to make it less likely
> > for such a leak to be added to the system in the future, I suspect.
> 
> Ahh, that was the leak that you plugged in a separate patch.
> 
> So it does point us in the other direction to redefine _release with
> a different behaviour that releases the resource held by the
> structure, and frees the structure itself.
> 
> Something along the following line (caution: totally untested) that
> allows your two patches to become empty, and also allows a few
> callers to lose their existing explicit free()s immediately after
> they call _release(), perhaps?