builtin/rebase: always store allocated string in `options.strategy`

The `struct rebase_options::strategy` field is a `char *`, but we do end
up assigning string constants to it in two cases:

  - When being passed a `--strategy=` option via the command line.

  - When being passed a strategy option via `--strategy-option=`, but
    not a strategy.

This will cause warnings once we enable `-Wwrite-strings`.

Ideally, we'd just convert the field to be a `const char *`. But we also
assign to this field via the GIT_TEST_MERGE_ALGORITHM envvar, which we
have to strdup(3P) into it.

Instead, refactor the code to make sure that we only ever assign
allocated strings to this field.

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

diff --git a/builtin/rebase.c b/builtin/rebase.c
index adc990b55e62dcc361aeea6222976bc8b8ab881b..4506bae768a4c91787574a5cceaddbc3fe812b63 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1061,6 +1061,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
        struct rebase_options options = REBASE_OPTIONS_INIT;
        const char *branch_name;
+       const char *strategy_opt = NULL;
        int ret, flags, total_argc, in_progress = 0;
        int keep_base = 0;
        int ok_to_skip_pre_rebase = 0;
@@ -1175,7 +1176,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                        PARSE_OPT_OPTARG, parse_opt_rebase_merges),
                OPT_BOOL(0, "fork-point", &options.fork_point,
                         N_("use 'merge-base --fork-point' to refine upstream")),
-               OPT_STRING('s', "strategy", &options.strategy,
+               OPT_STRING('s', "strategy", &strategy_opt,
                           N_("strategy"), N_("use the given merge strategy")),
                OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts,
                                N_("option"),
@@ -1484,13 +1485,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                }
        }
 
-       if (options.strategy_opts.nr && !options.strategy)
-               options.strategy = "ort";
-
-       if (options.strategy) {
-               options.strategy = xstrdup(options.strategy);
+       if (strategy_opt)
+               options.strategy = xstrdup(strategy_opt);
+       else if (options.strategy_opts.nr && !options.strategy)
+               options.strategy = xstrdup("ort");
+       if (options.strategy)
                imply_merge(&options, "--strategy");
-       }
 
        if (options.root && !options.onto_name)
                imply_merge(&options, "--root without --onto");