Re: [PATCH 1/2] bundle: plug resource leak

[Jeff King <peff@peff.net>]

On Tue, Mar 01, 2016 at 03:35:34PM -0800, Junio C Hamano wrote:

> The bundle header structure holds two lists of refs and object
> names, which should be released when the user is done with it.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  bundle.c    | 12 ++++++++++++
>  bundle.h    |  1 +
>  transport.c |  1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/bundle.c b/bundle.c
> index 506ac49..9c5a6f0 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -102,6 +102,18 @@ int is_bundle(const char *path, int quiet)
>  	return (fd >= 0);
>  }
>  
> +void release_bundle_header(struct bundle_header *header)
> +{
> +	int i;
> +
> +	for (i = 0; i < header->prerequisites.nr; i++)
> +		free(header->prerequisites.list[i].name);
> +	free(header->prerequisites.list);
> +	for (i = 0; i < header->references.nr; i++)
> +		free(header->references.list[i].name);
> +	free(header->references.list);
> +}

Looks good. It's probably not worth adding a release_ref_list() to
handle the repeated data structures.

I do find it hard to believe that the bundle code had to invent its own
ref storage data structure, and couldn't just use "struct ref" like all
of the other code. It doesn't look like we ever sort it or do
non-sequential access. The linked-list "struct ref" probably would have
been fine.

Not a problem you are introducing, of course, but if you are touching
this code a lot, it might be worth seeing how painful it is.

-Peff