sequencer: plug memory leaks for the option values

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, take custody of the option values in question,
allocating and duping literal constants as needed and freeing them
at end.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559c97aa12181d675180ac8785efe9af967b..ba5a88cbfd279cc6a408045d8e205d3c68fa9f7c 100644 (file)
--- 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);
 
        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)
 }
 
 int cmd_revert(int argc, const char **argv, const char *prefix)

diff --git a/sequencer.c b/sequencer.c
index 8d272fbdcabd490414e8c2fb4d88b51e57500913..04c55f2087a1191a5dfb6d08c099acd401090edf 100644 (file)
--- 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;
 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);
 
        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;
        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);
        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);
 
                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);
                                        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;
 }
 
        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;
 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"))
        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"))
        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);
        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 dd4d33a5723737d8cdd44a138c666997802afd23..8453669765b38cca7203fc7634c511a3ee652d68 100644 (file)
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,11 +34,11 @@ struct replay_opts {
 
        int mainline;
 
 
        int mainline;
 

-       const char *gpg_sign;
+       char *gpg_sign;

 
        /* Merge strategy */
 
        /* Merge strategy */

-       const char *strategy;
-       const char **xopts;
+       char *strategy;
+       char **xopts;

        size_t xopts_nr, xopts_alloc;
 
        /* Only used by REPLAY_NONE */
        size_t xopts_nr, xopts_alloc;
 
        /* Only used by REPLAY_NONE */