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.schinde...@gmx.de>
---
 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


Reply via email to