Hi Patrick
On 04/06/2024 13:38, Patrick Steinhardt wrote:
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.
This looks sensible
Thanks
Phillip
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
builtin/rebase.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 11f276012c..26068cf542 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1063,6 +1063,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
{
struct rebase_options options;
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;
@@ -1177,7 +1178,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"),
@@ -1488,13 +1489,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");
