Patrick Steinhardt <ps@xxxxxx> writes:
> In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass
> to `unbundle()`, but never free it. And in theory we wouldn't have to
> because `unbundle()` already knows to free the vector for us. But it
> fails to do so when it exits early due to `verify_bundle()` failing.
>
> The calling convention that the arguments are freed by the callee and
> not the caller feels somewhat weird. Refactor the code such that it is
> instead the responsibility of the caller to free the vector, adapting
> the only two callsites where we pass extra arguments. This also fixes
> the memory 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@xxxxxx>
> ---
> builtin/bundle.c | 2 ++
> bundle.c | 4 +---
> transport.c | 2 ++
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/bundle.c b/builtin/bundle.c
> index d5d41a8f672..df97f399019 100644
> --- a/builtin/bundle.c
> +++ b/builtin/bundle.c
> @@ -220,7 +220,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
> &extra_index_pack_args, 0) ||
> list_bundle_refs(&header, argc, argv);
> bundle_header_release(&header);
> +
> cleanup:
> + strvec_clear(&extra_index_pack_args);
> free(bundle_file);
> return ret;
> }
> diff --git a/bundle.c b/bundle.c
> index ce164c37bc8..0f6c7a71ef1 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -639,10 +639,8 @@ int unbundle(struct repository *r, struct bundle_header *header,
> if (flags & VERIFY_BUNDLE_FSCK)
> strvec_push(&ip.args, "--fsck-objects");
>
> - if (extra_index_pack_args) {
> + if (extra_index_pack_args)
> strvec_pushv(&ip.args, extra_index_pack_args->v);
> - strvec_clear(extra_index_pack_args);
> - }
>
> ip.in = bundle_fd;
> ip.no_stdout = 1;
> 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;
> }
Looks much better. Will queue. Thanks.
