Re: [PATCH] refs/files: prevent memory leak by freeing packed_ref_store

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

On Sat, Aug 03, 2024 at 10:37:51AM +0000, Sven Strickroth via GitGitGadget wrote:
> From: Sven Strickroth <email@cs-ware.de>
> 
> This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).

The format of references should match `git log --format=reference`,
which would be:

    64a6dd8ffc (refs: implement removal of ref storages, 2024-06-06)

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index aa52d9be7c7..11551de8f84 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -157,6 +157,7 @@ static void files_ref_store_release(struct ref_store *ref_store)
>  	free_ref_cache(refs->loose);
>  	free(refs->gitcommondir);
>  	ref_store_release(refs->packed_ref_store);
> +	free(refs->packed_ref_store);
>  }

Makes sense. `packed_ref_store_init()` returns a newly-allocated ref
store, and `ref_store_release()` only releases the store contents.
Consequently, we have to manually free the store here.

That does highlight that `packed_ref_store_init()` is misnamed and
really should be called `packed_ref_store_new()`, as it also allocates
the structure itself. But that's a #leftoverbit for another day, I'd
say.

Out of curiosity, did you hit this memory leak in some of our tests, or
did you just happen to stumble over it by chance?

Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]