The strings allocated in `setup_unpack_trees_porcelain()` are never
freed. Provide a function `clear_unpack_trees_porcelain()` to do so and
call it where we use `setup_unpack_trees_porcelain()`. The only
non-trivial user is `unpack_trees_start()`, where we should place the
new call in `unpack_trees_finish()`.
We keep the string pointers in an array, mixing pointers to static
memory and memory that we allocate on the heap. We also keep several
copies of the individual pointers. So we need to make sure that we do
not free what we must not free and that we do not double-free. Keep the
unique, heap-allocated pointers in a separate string list, to make the
freeing safe and future-proof.
Zero the whole array of string pointers to make sure that we do not
leave any dangling pointers.
Note that we only take responsibility for the memory allocated in
`setup_unpack_trees_porcelain()` and not any other members of the
`struct unpack_trees_options`.
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Martin Ågren <martin.ag...@gmail.com>
---
unpack-trees.h | 6 ++++++
builtin/checkout.c | 1 +
merge-recursive.c | 1 +
merge.c | 3 +++
unpack-trees.c | 20 +++++++++++++++++---
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/unpack-trees.h b/unpack-trees.h
index 41178ada94..5a84123a40 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -33,6 +33,11 @@ enum unpack_trees_error_types {
void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
const char *cmd);
+/*
+ * Frees resources allocated by setup_unpack_trees_porcelain().
+ */
+void clear_unpack_trees_porcelain(struct unpack_trees_options *opts);
+
struct unpack_trees_options {
unsigned int reset,
merge,
@@ -57,6 +62,7 @@ struct unpack_trees_options {
struct pathspec *pathspec;
merge_fn_t fn;
const char *msgs[NB_UNPACK_TREES_ERROR_TYPES];
+ struct string_list msgs_to_free;
/*
* Store error messages in an array, each case
* corresponding to a error message type
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b49b582071..5cebe170fc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -526,6 +526,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
init_tree_desc(&trees[1], tree->buffer, tree->size);
ret = unpack_trees(2, trees, &topts);
+ clear_unpack_trees_porcelain(&topts);
if (ret == -1) {
/*
* Unpack couldn't do a trivial merge; either
diff --git a/merge-recursive.c b/merge-recursive.c
index ddb0fa7369..338f63a952 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -382,6 +382,7 @@ static int unpack_trees_start(struct merge_options *o,
static void unpack_trees_finish(struct merge_options *o)
{
discard_index(&o->orig_index);
+ clear_unpack_trees_porcelain(&o->unpack_opts);
}
struct tree *write_tree_from_memory(struct merge_options *o)
diff --git a/merge.c b/merge.c
index f123658e58..b433291d0c 100644
--- a/merge.c
+++ b/merge.c
@@ -130,8 +130,11 @@ int checkout_fast_forward(const struct object_id *head,
if (unpack_trees(nr_trees, t, &opts)) {
rollback_lock_file(&lock_file);
+ clear_unpack_trees_porcelain(&opts);
return -1;
}
+ clear_unpack_trees_porcelain(&opts);
+
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
return error(_("unable to write new index file"));
return 0;
diff --git a/unpack-trees.c b/unpack-trees.c
index 79fd97074e..86046b987a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -103,6 +103,12 @@ void setup_unpack_trees_porcelain(struct
unpack_trees_options *opts,
const char **msgs = opts->msgs;
const char *msg;
+ /*
+ * As we add strings using `...appendf()`, this does not matter,
+ * but when we clear the string list, we want them to be freed.
+ */
+ opts->msgs_to_free.strdup_strings = 1;
+
if (!strcmp(cmd, "checkout"))
msg = advice_commit_before_merge
? _("Your local changes to the following files would be
overwritten by checkout:\n%%s"
@@ -119,7 +125,7 @@ void setup_unpack_trees_porcelain(struct
unpack_trees_options *opts,
"Please commit your changes or stash them before you
%s.")
: _("Your local changes to the following files would be
overwritten by %s:\n%%s");
msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =
- xstrfmt(msg, cmd, cmd);
+ string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
msgs[ERROR_NOT_UPTODATE_DIR] =
_("Updating the following directories would lose untracked
files in them:\n%s");
@@ -139,7 +145,8 @@ void setup_unpack_trees_porcelain(struct
unpack_trees_options *opts,
? _("The following untracked working tree files would be
removed by %s:\n%%s"
"Please move or remove them before you %s.")
: _("The following untracked working tree files would be
removed by %s:\n%%s");
- msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, cmd, cmd);
+ msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] =
+ string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
if (!strcmp(cmd, "checkout"))
msg = advice_commit_before_merge
@@ -156,7 +163,8 @@ void setup_unpack_trees_porcelain(struct
unpack_trees_options *opts,
? _("The following untracked working tree files would be
overwritten by %s:\n%%s"
"Please move or remove them before you %s.")
: _("The following untracked working tree files would be
overwritten by %s:\n%%s");
- msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = xstrfmt(msg, cmd, cmd);
+ msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] =
+ string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string;
/*
* Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we
@@ -179,6 +187,12 @@ void setup_unpack_trees_porcelain(struct
unpack_trees_options *opts,
opts->unpack_rejects[i].strdup_strings = 1;
}
+void clear_unpack_trees_porcelain(struct unpack_trees_options *opts)
+{
+ string_list_clear(&opts->msgs_to_free, 0);
+ memset(opts->msgs, 0, sizeof(opts->msgs));
+}
+
static int do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
unsigned int set, unsigned int clear)
{
--
2.17.0.840.g5d83f92caf
