Skip to content

Commit

Permalink
transport: fix leaking arguments when fetching from bundle
Browse files Browse the repository at this point in the history
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@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
pks-t authored and gitster committed Aug 22, 2024
1 parent c92abe7 commit 7720460
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 3 deletions.
2 changes: 2 additions & 0 deletions builtin/bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 1 addition & 3 deletions bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 7720460

Please sign in to comment.