ls-remote & transport API: release "struct transport_ls_refs_options"
Fix a memory leak in codepaths that use the "struct transport_ls_refs_options" API. Since the introduction of the struct in 39835409d10 (connect, transport: encapsulate arg in struct, 2021-02-05) the caller has been responsible for freeing it. That commit in turn migrated code originally added in 402c47d9391 (clone: send ref-prefixes when using protocol v2, 2018-07-20) and b4be74105fe (ls-remote: pass ref prefixes when requesting a remote's refs, 2018-03-15). Only some of those codepaths were releasing the allocated resources of the struct, now all of them will. Mark the "t/t5511-refspec.sh" test as passing when git is compiled with SANITIZE=leak. They'll now be listed as running under the "GIT_TEST_PASSING_SANITIZE_LEAK=true" test mode (the "linux-leaks" CI target). Previously 24/47 tests would fail. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ævar Arnfjörð Bjarmason committed
Feb 5, 2022 at 01:08 UTC
f36d4f8316ac567bd3bd0de3c051f2cd8ae2444b
7 files changed
+26
-15
builtin/clone.c
+6
-7
@@ -1233,7 +1233,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1233
}
1234
else {
1235
const char *branch;
1236
- char *ref;
1236
+ const char *ref;
1237
+ char *ref_free = NULL;
1238
1239
if (option_branch)
1240
die(_("Remote branch %s not found in upstream %s"),
@@ -1250,17 +1251,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1251
skip_prefix(transport_ls_refs_options.unborn_head_target,
1252
"refs/heads/", &branch)) {
1253
ref = transport_ls_refs_options.unborn_head_target;
1253
- transport_ls_refs_options.unborn_head_target = NULL;
1254
create_symref("HEAD", ref, reflog_msg.buf);
1255
} else {
1256
branch = git_default_branch_name(0);
1257
- ref = xstrfmt("refs/heads/%s", branch);
1257
+ ref_free = xstrfmt("refs/heads/%s", branch);
1258
+ ref = ref_free;
1259
}
1260
1261
if (!option_bare)
1262
install_branch_config(0, branch, remote_name, ref);
1262
-
1263
- free(ref);
1263
+ free(ref_free);
1264
}
1265
1266
write_refspec_config(src_ref_prefix, our_head_points_at,
@@ -1312,7 +1312,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1312
UNLEAK(repo);
1313
junk_mode = JUNK_LEAVE_ALL;
1314
1315
- strvec_clear(&transport_ls_refs_options.ref_prefixes);
1316
- free(transport_ls_refs_options.unborn_head_target);
1315
+ transport_ls_refs_options_release(&transport_ls_refs_options);
1316
return err;
1317
}
builtin/fetch.c
+1
-1
@@ -1593,7 +1593,7 @@ static int do_fetch(struct transport *transport,
1593
} else
1594
remote_refs = NULL;
1595
1596
- strvec_clear(&transport_ls_refs_options.ref_prefixes);
1596
+ transport_ls_refs_options_release(&transport_ls_refs_options);
1597
1598
ref_map = get_ref_map(transport->remote, remote_refs, rs,
1599
tags, &autotags);
builtin/ls-remote.c
+2
-1
@@ -155,6 +155,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
155
156
ref_array_clear(&ref_array);
157
if (transport_disconnect(transport))
158
- return 1;
158
+ status = 1;
159
+ transport_ls_refs_options_release(&transport_options);
160
return status;
161
}
connect.c
+2
-2
@@ -379,7 +379,7 @@ struct ref **get_remote_heads(struct packet_reader *reader,
379
380
/* Returns 1 when a valid ref has been added to `list`, 0 otherwise */
381
static int process_ref_v2(struct packet_reader *reader, struct ref ***list,
382
- char **unborn_head_target)
382
+ const char **unborn_head_target)
383
{
384
int ret = 1;
385
int i = 0;
@@ -483,7 +483,7 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
483
const char *hash_name;
484
struct strvec *ref_prefixes = transport_options ?
485
&transport_options->ref_prefixes : NULL;
486
- char **unborn_head_target = transport_options ?
486
+ const char **unborn_head_target = transport_options ?
487
&transport_options->unborn_head_target : NULL;
488
*list = NULL;
489
t/t5511-refspec.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='refspec parsing'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_refspec () {
transport.c
+7
-1
@@ -1292,7 +1292,7 @@ int transport_push(struct repository *r,
1292
&transport_options);
1293
trace2_region_leave("transport_push", "get_refs_list", r);
1294
1295
- strvec_clear(&transport_options.ref_prefixes);
1295
+ transport_ls_refs_options_release(&transport_options);
1296
1297
if (flags & TRANSPORT_PUSH_ALL)
1298
match_flags |= MATCH_REFS_ALL;
@@ -1420,6 +1420,12 @@ const struct ref *transport_get_remote_refs(struct transport *transport,
1420
return transport->remote_refs;
1421
}
1422
1423
+void transport_ls_refs_options_release(struct transport_ls_refs_options *opts)
1424
+{
1425
+ strvec_clear(&opts->ref_prefixes);
1426
+ free((char *)opts->unborn_head_target);
1427
+}
1428
+
1429
int transport_fetch_refs(struct transport *transport, struct ref *refs)
1430
{
1431
int rc;
transport.h
+7
-3
@@ -257,15 +257,19 @@ struct transport_ls_refs_options {
257
/*
258
* If unborn_head_target is not NULL, and the remote reports HEAD as
259
* pointing to an unborn branch, transport_get_remote_refs() stores the
260
- * unborn branch in unborn_head_target. It should be freed by the
261
- * caller.
260
+ * unborn branch in unborn_head_target.
261
*/
263
- char *unborn_head_target;
262
+ const char *unborn_head_target;
263
};
264
#define TRANSPORT_LS_REFS_OPTIONS_INIT { \
265
.ref_prefixes = STRVEC_INIT, \
266
}
267
268
+/**
269
+ * Release the "struct transport_ls_refs_options".
270
+ */
271
+void transport_ls_refs_options_release(struct transport_ls_refs_options *opts);
272
+
273
/*
274
* Retrieve refs from a remote.
275
*/