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>
Patrick Steinhardt committed
Sep 30, 2024 at 11:13 UTC
fdf972a9df19915ef7bddf447bfecbcd0d8aea17
2 files changed
+14
-4
builtin/revert.c
+13
-4
@@ -110,6 +110,9 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
110
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
111
const char *me = action_name(opts);
112
const char *cleanup_arg = NULL;
113
+ const char sentinel_value;
114
+ const char *strategy = &sentinel_value;
115
+ const char *gpg_sign = &sentinel_value;
116
enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
117
int cmd = 0;
118
struct option base_options[] = {
@@ -125,10 +128,10 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
128
OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
129
N_("select mainline parent"), option_parse_m),
130
OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
128
- OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
131
+ OPT_STRING(0, "strategy", &strategy, N_("strategy"), N_("merge strategy")),
132
OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"),
133
N_("option for merge strategy")),
131
- { OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
134
+ { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
135
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
136
OPT_END()
137
};
@@ -240,8 +243,14 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
243
usage_with_options(usage_str, options);
244
245
/* These option values will be free()d */
243
- opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
244
- opts->strategy = xstrdup_or_null(opts->strategy);
246
+ if (gpg_sign != &sentinel_value) {
247
+ free(opts->gpg_sign);
248
+ opts->gpg_sign = xstrdup_or_null(gpg_sign);
249
+ }
250
+ if (strategy != &sentinel_value) {
251
+ free(opts->strategy);
252
+ opts->strategy = xstrdup_or_null(strategy);
253
+ }
254
if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
255
opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
256
free(options);
t/t3514-cherry-pick-revert-gpg.sh
+1
@@ -5,6 +5,7 @@
5
6
test_description='test {cherry-pick,revert} --[no-]gpg-sign'
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
. "$TEST_DIRECTORY/lib-gpg.sh"
11