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>
Johannes Schindelin committed
Oct 21, 2016 at 14:24 UTC
03a4e260e2bf4eebd7ea6658d24507fbf80f3ecd
3 files changed
+29
-7
builtin/revert.c
+4
@@ -174,6 +174,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
174
175
if (argc > 1)
176
usage_with_options(usage_str, options);
177
+
178
+ /* These option values will be free()d */
179
+ opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
180
+ opts->strategy = xstrdup_or_null(opts->strategy);
181
}
182
183
int cmd_revert(int argc, const char **argv, const char *prefix)
sequencer.c
+22
-4
@@ -117,6 +117,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
117
static void remove_sequencer_state(const struct replay_opts *opts)
118
{
119
struct strbuf dir = STRBUF_INIT;
120
+ int i;
121
+
122
+ free(opts->gpg_sign);
123
+ free(opts->strategy);
124
+ for (i = 0; i < opts->xopts_nr; i++)
125
+ free(opts->xopts[i]);
126
+ free(opts->xopts);
127
128
strbuf_addf(&dir, "%s", get_dir(opts));
129
remove_dir_recursively(&dir, 0);
@@ -280,7 +287,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
287
struct merge_options o;
288
struct tree *result, *next_tree, *base_tree, *head_tree;
289
int clean;
283
- const char **xopt;
290
+ char **xopt;
291
static struct lock_file index_lock;
292
293
hold_locked_index(&index_lock, 1);
@@ -583,7 +590,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
590
591
commit_list_insert(base, &common);
592
commit_list_insert(next, &remotes);
586
- res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
593
+ res |= try_merge_command(opts->strategy,
594
+ opts->xopts_nr, (const char **)opts->xopts,
595
common, sha1_to_hex(head), remotes);
596
free_commit_list(common);
597
free_commit_list(remotes);
@@ -783,6 +791,16 @@ static int read_populate_todo(struct commit_list **todo_list,
791
return 0;
792
}
793
794
+static int git_config_string_dup(char **dest,
795
+ const char *var, const char *value)
796
+{
797
+ if (!value)
798
+ return config_error_nonbool(var);
799
+ free(*dest);
800
+ *dest = xstrdup(value);
801
+ return 0;
802
+}
803
+
804
static int populate_opts_cb(const char *key, const char *value, void *data)
805
{
806
struct replay_opts *opts = data;
@@ -803,9 +821,9 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
821
else if (!strcmp(key, "options.mainline"))
822
opts->mainline = git_config_int(key, value);
823
else if (!strcmp(key, "options.strategy"))
806
- git_config_string(&opts->strategy, key, value);
824
+ git_config_string_dup(&opts->strategy, key, value);
825
else if (!strcmp(key, "options.gpg-sign"))
808
- git_config_string(&opts->gpg_sign, key, value);
826
+ git_config_string_dup(&opts->gpg_sign, key, value);
827
else if (!strcmp(key, "options.strategy-option")) {
828
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
829
opts->xopts[opts->xopts_nr++] = xstrdup(value);
sequencer.h
+3
-3
@@ -34,11 +34,11 @@ struct replay_opts {
34
35
int mainline;
36
37
- const char *gpg_sign;
37
+ char *gpg_sign;
38
39
/* Merge strategy */
40
- const char *strategy;
41
- const char **xopts;
40
+ char *strategy;
41
+ char **xopts;
42
size_t xopts_nr, xopts_alloc;
43
44
/* Only used by REPLAY_NONE */