Re: [PATCH 19/20] transport: fix leaking arguments when fetching from bundle

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 21, 2024, 18:07 UTC
Message-ID: <xmqqseuxyck4.fsf@gitster.g>
In-Reply-To: <b72f7d1ee394291a018061c91222ea17a99c2e53.1724159575.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 23 quoted lines

> In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass
> to `unbundle()`, but never free it. Fix this leak.
>
> This memory leak gets hit in t5510, but fixing it isn't sufficient to
> make the whole test suite pass.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  transport.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/transport.c b/transport.c
> index f0672fdc505..da639d3bff0 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,
>  		       &extra_index_pack_args,
>  		       fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);
>  	transport->hash_algo = data->header.hash_algo;
> +
> +	strvec_clear(&extra_index_pack_args);
>  	return ret;
>  }

Hmph.  unbundle() has this:

	if (extra_index_pack_args) {
		strvec_pushv(&ip.args, extra_index_pack_args->v);
		strvec_clear(extra_index_pack_args);
	}

so while I think this patch would not hurt at all, do we need this patch?

The other caller of unbundle() that passes the extra_index_pack_args is cmd_bundle_unbundle() and it does not do anything after it is done.