reduce_heads: fix memory leaks

We currently have seven callers of `reduce_heads(foo)`. Six of them do
not use the original list `foo` again, and actually, all six of those
end up leaking it.

Introduce and use `reduce_heads_replace(&foo)` as a leak-free version of
`foo = reduce_heads(foo)` to fix several of these. Fix the remaining
leaks using `free_commit_list()`.

While we're here, document `reduce_heads()` and mark it as `extern`.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea7fe3475564f337bf660d8155909445..11c47401854fa4bfd8b75429f053882ff1c61551 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1728,7 +1728,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
                                allow_fast_forward = 0;
                }
                if (allow_fast_forward)
-                       parents = reduce_heads(parents);
+                       reduce_heads_replace(&parents);
        } else {
                if (!reflog_msg)
                        reflog_msg = (whence == FROM_CHERRY_PICK)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index e99b5ddbf9a51be17bc0d976c942c29afe6563cd..27a2361e91dcd94cba4556b04b028286835f8823 100644 (file)
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -571,7 +571,7 @@ static void find_merge_parents(struct merge_parents *result,
        head_commit = lookup_commit(head);
        if (head_commit)
                commit_list_insert(head_commit, &parents);
-       parents = reduce_heads(parents);
+       reduce_heads_replace(&parents);
 
        while (parents) {
                struct commit *cmit = pop_commit(&parents);

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e17835fabb6702bf665264740d29dab0c04b094f..24f6c7193546b91a8644a4417bfc6d6d94da6a62 100644 (file)
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -57,7 +57,7 @@ static int handle_independent(int count, const char **args)
        for (i = count - 1; i >= 0; i--)
                commit_list_insert(get_commit_reference(args[i]), &revs);
 
-       revs = reduce_heads(revs);
+       reduce_heads_replace(&revs);
 
        if (!revs)
                return 1;
@@ -78,7 +78,9 @@ static int handle_octopus(int count, const char **args, int show_all)
        for (i = count - 1; i >= 0; i--)
                commit_list_insert(get_commit_reference(args[i]), &revs);
 
-       result = reduce_heads(get_octopus_merge_bases(revs));
+       result = get_octopus_merge_bases(revs);
+       free_commit_list(revs);
+       reduce_heads_replace(&result);
 
        if (!result)
                return 1;

diff --git a/builtin/merge.c b/builtin/merge.c
index ab5ffe85e8f5d7a359004f2bbb52a5778d0b91b4..fbbf2a9e5eb9be47d8574ca0bab175c5a6c0d15d 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -999,6 +999,7 @@ static struct commit_list *reduce_parents(struct commit *head_commit,
 
        /* Find what parents to record by checking independent ones. */
        parents = reduce_heads(remoteheads);
+       free_commit_list(remoteheads);
 
        remoteheads = NULL;
        remotes = &remoteheads;

diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a224c18238cf9a75436e8ea192765e7fe..4edab228ebe6c1d3df580689c7cf94eb989fe4bc 100644 (file)
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -745,12 +745,15 @@ static int get_octopus_merge_base(struct object_id *merge_base,
        if (!is_null_oid(fork_point))
                commit_list_insert(lookup_commit_reference(fork_point), &revs);
 
-       result = reduce_heads(get_octopus_merge_bases(revs));
+       result = get_octopus_merge_bases(revs);
        free_commit_list(revs);
+       reduce_heads_replace(&result);
+
        if (!result)
                return 1;
 
        oidcpy(merge_base, &result->item->object.oid);
+       free_commit_list(result);
        return 0;
 }
 

diff --git a/commit.c b/commit.c
index 1e0e633790bb834ad05ed9619e4afa0bac33ce19..cab8d4455bdbd6e4b87031613300be1112a19df5 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -1090,6 +1090,13 @@ struct commit_list *reduce_heads(struct commit_list *heads)
        return result;
 }
 
+void reduce_heads_replace(struct commit_list **heads)
+{
+       struct commit_list *result = reduce_heads(*heads);
+       free_commit_list(*heads);
+       *heads = result;
+}
+
 static const char gpg_sig_header[] = "gpgsig";
 static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1;
 

diff --git a/commit.h b/commit.h
index 6d769590f285e534d20bfd8108e8e980253f8815..99a3fea68d3f63064351ccabeaf7f11cdf8e0d04 100644 (file)
--- a/commit.h
+++ b/commit.h
@@ -313,7 +313,23 @@ extern int interactive_add(int argc, const char **argv, const char *prefix, int
 extern int run_add_interactive(const char *revision, const char *patch_mode,
                               const struct pathspec *pathspec);
 
-struct commit_list *reduce_heads(struct commit_list *heads);
+/*
+ * Takes a list of commits and returns a new list where those
+ * have been removed that can be reached from other commits in
+ * the list. It is useful for, e.g., reducing the commits
+ * randomly thrown at the git-merge command and removing
+ * redundant commits that the user shouldn't have given to it.
+ *
+ * This function destroys the STALE bit of the commit objects'
+ * flags.
+ */
+extern struct commit_list *reduce_heads(struct commit_list *heads);
+
+/*
+ * Like `reduce_heads()`, except it replaces the list. Use this
+ * instead of `foo = reduce_heads(foo);` to avoid memory leaks.
+ */
+extern void reduce_heads_replace(struct commit_list **heads);
 
 struct commit_extra_header {
        struct commit_extra_header *next;