The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
like a one-shot command when it reads its configuration: memory is
allocated and released only when the command exits.
This is kind of okay for git-cherry-pick, which *is* a one-shot
command. All the work to make the sequencer its work horse was
done to allow using the functionality as a library function, though,
including proper clean-up after use.
To remedy that, we now take custody of the option values in question,
requiring those values to be malloc()ed or strdup()ed (an alternative
approach, to add a list of pointers to malloc()ed data and to ask the
sequencer to release all of them in the end, was proposed earlier but
rejected during review).
Note: this means that we now have to take care to strdup() the values
passed via the command-line.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
---
builtin/revert.c | 4 ++++
sequencer.c | 26 ++++++++++++++++++++++----
sequencer.h | 6 +++---
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559..ba5a88c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -174,6 +174,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (argc > 1)
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);
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/sequencer.c b/sequencer.c
index 8d272fb..04c55f2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -117,6 +117,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
static void remove_sequencer_state(const struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
+ int i;
+
+ free(opts->gpg_sign);
+ free(opts->strategy);
+ for (i = 0; i < opts->xopts_nr; i++)
+ free(opts->xopts[i]);
+ free(opts->xopts);
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
@@ -280,7 +287,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
struct merge_options o;
struct tree *result, *next_tree, *base_tree, *head_tree;
int clean;
- const char **xopt;
+ char **xopt;
static struct lock_file index_lock;
hold_locked_index(&index_lock, 1);
@@ -583,7 +590,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ res |= try_merge_command(opts->strategy,
+ opts->xopts_nr, (const char **)opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
@@ -783,6 +791,16 @@ static int read_populate_todo(struct commit_list **todo_list,
return 0;
}
+static int git_config_string_dup(char **dest,
+ const char *var, const char *value)
+{
+ if (!value)
+ return config_error_nonbool(var);
+ free(*dest);
+ *dest = xstrdup(value);
+ return 0;
+}
+
static int populate_opts_cb(const char *key, const char *value, void *data)
{
struct replay_opts *opts = data;
@@ -803,9 +821,9 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
else if (!strcmp(key, "options.strategy"))
- git_config_string(&opts->strategy, key, value);
+ git_config_string_dup(&opts->strategy, key, value);
else if (!strcmp(key, "options.gpg-sign"))
- git_config_string(&opts->gpg_sign, key, value);
+ git_config_string_dup(&opts->gpg_sign, key, value);
else if (!strcmp(key, "options.strategy-option")) {
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
opts->xopts[opts->xopts_nr++] = xstrdup(value);
diff --git a/sequencer.h b/sequencer.h
index dd4d33a..8453669 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,11 +34,11 @@ struct replay_opts {
int mainline;
- const char *gpg_sign;
+ char *gpg_sign;
/* Merge strategy */
- const char *strategy;
- const char **xopts;
+ char *strategy;
+ char **xopts;
size_t xopts_nr, xopts_alloc;
/* Only used by REPLAY_NONE */
--
2.10.1.583.g721a9e0