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>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f90a4ca4b7a6fd1befeb60cafdc5519037b27165..3cf44b4683a9ba48abaaa32ef20bc127920865bd 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1275,12 +1275,12 @@ static void setup_new_branch_info_and_source_tree(
        }
 }
 
-static const char *parse_remote_branch(const char *arg,
-                                      struct object_id *rev,
-                                      int could_be_checkout_paths)
+static char *parse_remote_branch(const char *arg,
+                                struct object_id *rev,
+                                int could_be_checkout_paths)
 {
        int num_matches = 0;
-       const char *remote = unique_tracking_name(arg, rev, &num_matches);
+       char *remote = unique_tracking_name(arg, rev, &num_matches);
 
        if (remote && could_be_checkout_paths) {
                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,
        const char **new_branch = &opts->new_branch;
        int argcount = 0;
        const char *arg;
+       char *remote = NULL;
        int dash_dash_pos;
        int has_dash_dash = 0;
        int i;
@@ -1416,8 +1417,8 @@ static int parse_branchname_arg(int argc, const char **argv,
                        recover_with_dwim = 0;
 
                if (recover_with_dwim) {
-                       const char *remote = parse_remote_branch(arg, rev,
-                                                                could_be_checkout_paths);
+                       remote = parse_remote_branch(arg, rev,
+                                                    could_be_checkout_paths);
                        if (remote) {
                                *new_branch = arg;
                                arg = remote;
@@ -1459,6 +1460,7 @@ static int parse_branchname_arg(int argc, const char **argv,
                argc--;
        }
 
+       free(remote);
        return argcount;
 }
 

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7e0868df722d3a367c78c9aa91f90ccfc4ae3bbb..937da6c0eeff41d7ead9e116ea81f31f3d0cede0 100644 (file)
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -736,16 +736,14 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)
        return 1;
 }
 
-static const char *dwim_branch(const char *path, const char **new_branch)
+static char *dwim_branch(const char *path, char **new_branch)
 {
        int n;
        int branch_exists;
        const char *s = worktree_basename(path, &n);
-       const char *branchname = xstrndup(s, n);
+       char *branchname = xstrndup(s, n);
        struct strbuf ref = STRBUF_INIT;
 
-       UNLEAK(branchname);
-
        branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
                        refs_ref_exists(get_main_ref_store(the_repository),
                                        ref.buf);
@@ -756,8 +754,7 @@ static const char *dwim_branch(const char *path, const char **new_branch)
        *new_branch = branchname;
        if (guess_remote) {
                struct object_id oid;
-               const char *remote =
-                       unique_tracking_name(*new_branch, &oid, NULL);
+               char *remote = unique_tracking_name(*new_branch, &oid, NULL);
                return remote;
        }
        return NULL;
@@ -769,6 +766,8 @@ static int add(int ac, const char **av, const char *prefix)
        const char *new_branch_force = NULL;
        char *path;
        const char *branch;
+       char *branch_to_free = NULL;
+       char *new_branch_to_free = NULL;
        const char *new_branch = NULL;
        const char *opt_track = NULL;
        const char *lock_reason = NULL;
@@ -859,16 +858,17 @@ static int add(int ac, const char **av, const char *prefix)
                opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
        } else if (ac < 2) {
                /* DWIM: Guess branch name from path. */
-               const char *s = dwim_branch(path, &new_branch);
+               char *s = dwim_branch(path, &new_branch_to_free);
                if (s)
-                       branch = s;
+                       branch = branch_to_free = s;
+               new_branch = new_branch_to_free;
 
                /* DWIM: Infer --orphan when repo has no refs. */
                opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
        } else if (ac == 2) {
                struct object_id oid;
                struct commit *commit;
-               const char *remote;
+               char *remote;
 
                commit = lookup_commit_reference_by_name(branch);
                if (!commit) {
@@ -923,6 +923,8 @@ static int add(int ac, const char **av, const char *prefix)
 
        ret = add_worktree(path, branch, &opts);
        free(path);
+       free(branch_to_free);
+       free(new_branch_to_free);
        return ret;
 }
 

diff --git a/checkout.c b/checkout.c
index 4256e71a7c41cd4fdfbcd832727c21548b52b740..cfaea4bd10b215293cf0f1033cd7be5ba392c577 100644 (file)
--- a/checkout.c
+++ b/checkout.c
@@ -45,8 +45,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
        return 0;
 }
 
-const char *unique_tracking_name(const char *name, struct object_id *oid,
-                                int *dwim_remotes_matched)
+char *unique_tracking_name(const char *name, struct object_id *oid,
+                          int *dwim_remotes_matched)
 {
        struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
        const char *default_remote = NULL;

diff --git a/checkout.h b/checkout.h
index 3c514a5ab4f923db5e412fe5552960d8382ea9f5..ba15a13fb35be65e8c4b90fb87751f294105176e 100644 (file)
--- a/checkout.h
+++ b/checkout.h
@@ -8,8 +8,8 @@
  * tracking branch.  Return the name of the remote if such a branch
  * exists, NULL otherwise.
  */
-const char *unique_tracking_name(const char *name,
-                                struct object_id *oid,
-                                int *dwim_remotes_matched);
+char *unique_tracking_name(const char *name,
+                          struct object_id *oid,
+                          int *dwim_remotes_matched);
 
 #endif /* CHECKOUT_H */

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index a3b1449ef1166756e5bd029a2f94ac6e1c50b03b..2caada3d834ffc139c0af2e3f1b0b52f9eee9718 100755 (executable)
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -4,6 +4,7 @@ test_description='checkout <branch>
 
 Ensures that checkout on an unborn branch does what the user expects'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Is the current branch "refs/heads/$1"?

diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh
index c91c4db9361133e4e790f2c0201c5f41c14eaff1..77b2346291b39cfe16c614e8018a20195e7f1118 100755 (executable)
--- a/t/t2060-switch.sh
+++ b/t/t2060-switch.sh
@@ -5,6 +5,7 @@ test_description='switch basic functionality'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup' '

diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh
index ba069dccbdf56197b0e93831fdb1316853d54ef4..94ea88e384e6219c7d47c6c9fb8409640623680d 100755 (executable)
--- a/t/t3426-rebase-submodule.sh
+++ b/t/t3426-rebase-submodule.sh
@@ -2,6 +2,7 @@
 
 test_description='rebase can handle submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 . "$TEST_DIRECTORY"/lib-rebase.sh

diff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh
index f22d1ddead1ac94de714f61fc10b87cc4bae3aec..9387a22a9e7100335208713fcb069b31c69f7e80 100755 (executable)
--- a/t/t3512-cherry-pick-submodule.sh
+++ b/t/t3512-cherry-pick-submodule.sh
@@ -5,6 +5,7 @@ test_description='cherry-pick can handle submodules'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 

diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
index 8bfe3ed2467fa17d9f1910cd76c721c73776d4ab..e178968b408f4b9068bcc07055ffacf781154181 100755 (executable)
--- a/t/t3513-revert-submodule.sh
+++ b/t/t3513-revert-submodule.sh
@@ -2,6 +2,7 @@
 
 test_description='revert can handle submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 98259e2adaa9dba9fa6bbcb79e50948e69a6b9e6..31ac31d4bcd3bbef54f10234cb5c437d3f97fc35 100755 (executable)
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -8,6 +8,7 @@ test_description='Test of the various options to git rm.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Setup some files to be removed, some with funny characters

diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh
index 0f7348ec21b8821cb6b7594919bbc637ec0a7236..0f61f01ef43b2c889bd2850fbd41c4a42f52c6e6 100755 (executable)
--- a/t/t3906-stash-submodule.sh
+++ b/t/t3906-stash-submodule.sh
@@ -2,6 +2,7 @@
 
 test_description='stash can handle submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 

diff --git a/t/t4137-apply-submodule.sh b/t/t4137-apply-submodule.sh
index 07d52625375d4bec97db0a14d74a4fd154681b5e..ebd0d4ad17c1bfbb1dcc231a32dbcb295496c7f7 100755 (executable)
--- a/t/t4137-apply-submodule.sh
+++ b/t/t4137-apply-submodule.sh
@@ -2,6 +2,7 @@
 
 test_description='git apply handling submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 

diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
index 82013fc9037c1dd67cfda1a0206f6b73a2869677..3946e18089a2a50a2a7a6433d462a7c97546a384 100755 (executable)
--- a/t/t6041-bisect-submodule.sh
+++ b/t/t6041-bisect-submodule.sh
@@ -2,6 +2,7 @@
 
 test_description='bisect can handle submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 

diff --git a/t/t6438-submodule-directory-file-conflicts.sh b/t/t6438-submodule-directory-file-conflicts.sh
index 8df67a0ef99d26357404579ef3dbc102317fc5c6..3594190af84c1ba2b65a3c69768147bd975dc155 100755 (executable)
--- a/t/t6438-submodule-directory-file-conflicts.sh
+++ b/t/t6438-submodule-directory-file-conflicts.sh
@@ -2,6 +2,7 @@
 
 test_description='merge can handle submodules'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh