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@xxxxxx>
---
builtin/revert.c | 17 +++++++++++++----
t/t3514-cherry-pick-revert-gpg.sh | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 55ba1092c5..b7917dddd3 100644
--- 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 5b2e250eaa..133dc07217 100755
--- 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"
--
2.46.2.852.g229c0bf0e5.dirty
