builtin/revert: fix leaking `gpg_sign` and `strategy` config

We leak the config values when `gpg_sign` or `strategy` options are
being overridden via the command line. To fix this we need to free the
old value, which requires us to figure out whether the value was changed
via an option in the first place. The easy way to do this, which is to
initialize local variables with `NULL`, doesn't work because we cannot
tell the case where the user has passed e.g. `--no-gpg-sign`. Instead,
we use a sentinel value for both values that we can compare against to
check whether the user has passed the option.

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

diff --git a/builtin/revert.c b/builtin/revert.c
index 55ba1092c52f420191bdafb20c93708eaee1f69b..b7917dddd3e15e1d845c002bd31affdfd61d3cae 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,6 +110,9 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
        const char * const * usage_str = revert_or_cherry_pick_usage(opts);
        const char *me = action_name(opts);
        const char *cleanup_arg = NULL;
+       const char sentinel_value;
+       const char *strategy = &sentinel_value;
+       const char *gpg_sign = &sentinel_value;
        enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
        int cmd = 0;
        struct option base_options[] = {
@@ -125,10 +128,10 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
                OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
                             N_("select mainline parent"), option_parse_m),
                OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
-               OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
+               OPT_STRING(0, "strategy", &strategy, N_("strategy"), N_("merge strategy")),
                OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"),
                        N_("option for merge strategy")),
-               { OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
+               { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
                  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
                OPT_END()
        };
@@ -240,8 +243,14 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
                usage_with_options(usage_str, options);
 
        /* These option values will be free()d */
-       opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
-       opts->strategy = xstrdup_or_null(opts->strategy);
+       if (gpg_sign != &sentinel_value) {
+               free(opts->gpg_sign);
+               opts->gpg_sign = xstrdup_or_null(gpg_sign);
+       }
+       if (strategy != &sentinel_value) {
+               free(opts->strategy);
+               opts->strategy = xstrdup_or_null(strategy);
+       }
        if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
                opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
        free(options);

diff --git a/t/t3514-cherry-pick-revert-gpg.sh b/t/t3514-cherry-pick-revert-gpg.sh
index 5b2e250eaa509458d89d13f4a3728276525fa746..133dc072178a161a33b05ca02f8872b5286a943a 100755 (executable)
--- a/t/t3514-cherry-pick-revert-gpg.sh
+++ b/t/t3514-cherry-pick-revert-gpg.sh
@@ -5,6 +5,7 @@
 
 test_description='test {cherry-pick,revert} --[no-]gpg-sign'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"