builtin/replay: plug leaking `advance_name` variable

The `advance_name` variable can either contain a static string when
parsed via the `--advance` command line option or it may be an allocated
string when set via `determine_replay_mode()`. Because we cannot be sure
whether it is allocated or not we just didn't free it at all, resulting
in a memory leak.

Split up the variables such that we can track the static and allocated
strings separately and then free the allocated one to fix the memory
leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/replay.c b/builtin/replay.c
index 04483266367bb677d8459e700e0de8a2d133c6c5..27b40eda981165a3cb0051563d4740afa16320b4 100644 (file)
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -151,7 +151,7 @@ static void get_ref_information(struct rev_cmdline_info *cmd_info,
 
 static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
                                  const char *onto_name,
-                                 const char **advance_name,
+                                 char **advance_name,
                                  struct commit **onto,
                                  struct strset **update_refs)
 {
@@ -174,6 +174,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
                *onto = peel_committish(*advance_name);
                if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),
                             &oid, &fullname, 0) == 1) {
+                       free(*advance_name);
                        *advance_name = fullname;
                } else {
                        die(_("argument to --advance must be a reference"));
@@ -197,6 +198,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
                if (negative_refs_complete) {
                        struct hashmap_iter iter;
                        struct strmap_entry *entry;
+                       const char *last_key = NULL;
 
                        if (rinfo.negative_refexprs == 0)
                                die(_("all positive revisions given must be references"));
@@ -208,8 +210,11 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
                        /* Only one entry, but we have to loop to get it */
                        strset_for_each_entry(&rinfo.negative_refs,
                                              &iter, entry) {
-                               *advance_name = entry->key;
+                               last_key = entry->key;
                        }
+
+                       free(*advance_name);
+                       *advance_name = xstrdup_or_null(last_key);
                } else { /* positive_refs_complete */
                        if (rinfo.negative_refexprs > 1)
                                die(_("cannot implicitly determine correct base for --onto"));
@@ -271,7 +276,8 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
-       const char *advance_name = NULL;
+       const char *advance_name_opt = NULL;
+       char *advance_name = NULL;
        struct commit *onto = NULL;
        const char *onto_name = NULL;
        int contained = 0;
@@ -292,7 +298,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
                NULL
        };
        struct option replay_options[] = {
-               OPT_STRING(0, "advance", &advance_name,
+               OPT_STRING(0, "advance", &advance_name_opt,
                           N_("branch"),
                           N_("make replay advance given branch")),
                OPT_STRING(0, "onto", &onto_name,
@@ -306,14 +312,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
        argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
                             PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
 
-       if (!onto_name && !advance_name) {
+       if (!onto_name && !advance_name_opt) {
                error(_("option --onto or --advance is mandatory"));
                usage_with_options(replay_usage, replay_options);
        }
 
-       if (advance_name && contained)
+       if (advance_name_opt && contained)
                die(_("options '%s' and '%s' cannot be used together"),
                    "--advance", "--contained");
+       advance_name = xstrdup_or_null(advance_name_opt);
 
        repo_init_revisions(the_repository, &revs, prefix);
 
@@ -441,6 +448,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 cleanup:
        release_revisions(&revs);
+       free(advance_name);
 
        /* Return */
        if (ret < 0)

diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 389670262e458eadb7029251aa44b52be8ae5350..12bd3db4cb74635ff1a939c80897e9fb98a208a7 100755 (executable)
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -5,6 +5,7 @@ test_description='basic git replay tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 GIT_AUTHOR_NAME=author@name