transport: fix leaking arguments when fetching from bundle

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>

Patrick Steinhardt committed Aug 22, 2024 at 11:18 UTC 7720460ccf8d5a8fcc5cf37bad97b26d799a5644
3 files changed +5 -3
builtin/bundle.c
+2
@@ -220,7 +220,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
220 &extra_index_pack_args, 0) ||
221 list_bundle_refs(&header, argc, argv);
222 bundle_header_release(&header);
223 +
224 cleanup:
225 + strvec_clear(&extra_index_pack_args);
226 free(bundle_file);
227 return ret;
228 }
bundle.c
+1 -3
@@ -639,10 +639,8 @@ int unbundle(struct repository *r, struct bundle_header *header,
639 if (flags & VERIFY_BUNDLE_FSCK)
640 strvec_push(&ip.args, "--fsck-objects");
641
642 - if (extra_index_pack_args) {
642 + if (extra_index_pack_args)
643 strvec_pushv(&ip.args, extra_index_pack_args->v);
644 - strvec_clear(extra_index_pack_args);
645 - }
644
645 ip.in = bundle_fd;
646 ip.no_stdout = 1;
transport.c
+2
@@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,
189 &extra_index_pack_args,
190 fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);
191 transport->hash_algo = data->header.hash_algo;
192 +
193 + strvec_clear(&extra_index_pack_args);
194 return ret;
195 }
196