checkout: clarify memory ownership in `unique_tracking_name()`
The function `unique_tracking_name()` returns an allocated string, but does not clearly indicate this because its return type is `const char *` instead of `char *`. This has led to various callsites where we never free its returned memory at all, which causes memory leaks. Plug those leaks and mark now-passing tests as leak free. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
May 27, 2024 at 13:46 UTC
cc395d6b47e4af59b3e87a64b34dffa79e8dc262
14 files changed
+34
-20
builtin/checkout.c
+8
-6
@@ -1275,12 +1275,12 @@ static void setup_new_branch_info_and_source_tree(
1275
}
1276
}
1277
1278
-static const char *parse_remote_branch(const char *arg,
1279
- struct object_id *rev,
1280
- int could_be_checkout_paths)
1278
+static char *parse_remote_branch(const char *arg,
1279
+ struct object_id *rev,
1280
+ int could_be_checkout_paths)
1281
{
1282
int num_matches = 0;
1283
- const char *remote = unique_tracking_name(arg, rev, &num_matches);
1283
+ char *remote = unique_tracking_name(arg, rev, &num_matches);
1284
1285
if (remote && could_be_checkout_paths) {
1286
die(_("'%s' could be both a local file and a tracking branch.\n"
@@ -1316,6 +1316,7 @@ static int parse_branchname_arg(int argc, const char **argv,
1316
const char **new_branch = &opts->new_branch;
1317
int argcount = 0;
1318
const char *arg;
1319
+ char *remote = NULL;
1320
int dash_dash_pos;
1321
int has_dash_dash = 0;
1322
int i;
@@ -1416,8 +1417,8 @@ static int parse_branchname_arg(int argc, const char **argv,
1417
recover_with_dwim = 0;
1418
1419
if (recover_with_dwim) {
1419
- const char *remote = parse_remote_branch(arg, rev,
1420
- could_be_checkout_paths);
1420
+ remote = parse_remote_branch(arg, rev,
1421
+ could_be_checkout_paths);
1422
if (remote) {
1423
*new_branch = arg;
1424
arg = remote;
@@ -1459,6 +1460,7 @@ static int parse_branchname_arg(int argc, const char **argv,
1460
argc--;
1461
}
1462
1463
+ free(remote);
1464
return argcount;
1465
}
1466
builtin/worktree.c
+11
-9
@@ -736,16 +736,14 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)
736
return 1;
737
}
738
739
-static const char *dwim_branch(const char *path, const char **new_branch)
739
+static char *dwim_branch(const char *path, char **new_branch)
740
{
741
int n;
742
int branch_exists;
743
const char *s = worktree_basename(path, &n);
744
- const char *branchname = xstrndup(s, n);
744
+ char *branchname = xstrndup(s, n);
745
struct strbuf ref = STRBUF_INIT;
746
747
- UNLEAK(branchname);
748
-
747
branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
748
refs_ref_exists(get_main_ref_store(the_repository),
749
ref.buf);
@@ -756,8 +754,7 @@ static const char *dwim_branch(const char *path, const char **new_branch)
754
*new_branch = branchname;
755
if (guess_remote) {
756
struct object_id oid;
759
- const char *remote =
760
- unique_tracking_name(*new_branch, &oid, NULL);
757
+ char *remote = unique_tracking_name(*new_branch, &oid, NULL);
758
return remote;
759
}
760
return NULL;
@@ -769,6 +766,8 @@ static int add(int ac, const char **av, const char *prefix)
766
const char *new_branch_force = NULL;
767
char *path;
768
const char *branch;
769
+ char *branch_to_free = NULL;
770
+ char *new_branch_to_free = NULL;
771
const char *new_branch = NULL;
772
const char *opt_track = NULL;
773
const char *lock_reason = NULL;
@@ -859,16 +858,17 @@ static int add(int ac, const char **av, const char *prefix)
858
opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
859
} else if (ac < 2) {
860
/* DWIM: Guess branch name from path. */
862
- const char *s = dwim_branch(path, &new_branch);
861
+ char *s = dwim_branch(path, &new_branch_to_free);
862
if (s)
864
- branch = s;
863
+ branch = branch_to_free = s;
864
+ new_branch = new_branch_to_free;
865
866
/* DWIM: Infer --orphan when repo has no refs. */
867
opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
868
} else if (ac == 2) {
869
struct object_id oid;
870
struct commit *commit;
871
- const char *remote;
871
+ char *remote;
872
873
commit = lookup_commit_reference_by_name(branch);
874
if (!commit) {
@@ -923,6 +923,8 @@ static int add(int ac, const char **av, const char *prefix)
923
924
ret = add_worktree(path, branch, &opts);
925
free(path);
926
+ free(branch_to_free);
927
+ free(new_branch_to_free);
928
return ret;
929
}
930
checkout.c
+2
-2
@@ -45,8 +45,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
45
return 0;
46
}
47
48
-const char *unique_tracking_name(const char *name, struct object_id *oid,
49
- int *dwim_remotes_matched)
48
+char *unique_tracking_name(const char *name, struct object_id *oid,
49
+ int *dwim_remotes_matched)
50
{
51
struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
52
const char *default_remote = NULL;
checkout.h
+3
-3
@@ -8,8 +8,8 @@
8
* tracking branch. Return the name of the remote if such a branch
9
* exists, NULL otherwise.
10
*/
11
-const char *unique_tracking_name(const char *name,
12
- struct object_id *oid,
13
- int *dwim_remotes_matched);
11
+char *unique_tracking_name(const char *name,
12
+ struct object_id *oid,
13
+ int *dwim_remotes_matched);
14
15
#endif /* CHECKOUT_H */
t/t2024-checkout-dwim.sh
+1
@@ -4,6 +4,7 @@ test_description='checkout <branch>
4
5
Ensures that checkout on an unborn branch does what the user expects'
6
7
+TEST_PASSES_SANITIZE_LEAK=true
8
. ./test-lib.sh
9
10
# Is the current branch "refs/heads/$1"?
t/t2060-switch.sh
+1
@@ -5,6 +5,7 @@ test_description='switch basic functionality'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
test_expect_success 'setup' '
t/t3426-rebase-submodule.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='rebase can handle submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8
. "$TEST_DIRECTORY"/lib-rebase.sh
t/t3512-cherry-pick-submodule.sh
+1
@@ -5,6 +5,7 @@ test_description='cherry-pick can handle submodules'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
. "$TEST_DIRECTORY"/lib-submodule-update.sh
11
t/t3513-revert-submodule.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='revert can handle submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8
t/t3600-rm.sh
+1
@@ -8,6 +8,7 @@ test_description='Test of the various options to git rm.'
8
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
9
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
10
11
+TEST_PASSES_SANITIZE_LEAK=true
12
. ./test-lib.sh
13
14
# Setup some files to be removed, some with funny characters
t/t3906-stash-submodule.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='stash can handle submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8
t/t4137-apply-submodule.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='git apply handling submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8
t/t6041-bisect-submodule.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='bisect can handle submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8
t/t6438-submodule-directory-file-conflicts.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='merge can handle submodules'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-submodule-update.sh
8