merge: clarify collect_parents() logic
authorJunio C Hamano <gitster@pobox.com>
Sat, 25 Apr 2015 17:25:43 +0000 (10:25 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 29 Apr 2015 20:17:53 +0000 (13:17 -0700)
Clarify this small function in three ways.

- The function initially collects all commits to be merged into a
commit_list "remoteheads"; the "remotes" pointer always points at
the tail of this list (either the remoteheads variable itself, or
the ->next slot of the element at the end of the list) to help
elongate the list by repeated calls to commit_list_insert().
Because the new element appended by commit_list_insert() will
always have its ->next slot NULLed out, there is no need for us
to assign NULL to *remotes to terminate the list at the end.

- The variable "head_subsumed" always confused me every time I read
this code. What is happening here is that we inspect what the
caller told us to merge (including the current HEAD) and come up
with the list of parents to be recorded for the resulting merge
commit, omitting commits that are ancestor of other commits.
This filtering may remove the current HEAD from the resulting
parent list---and we signal that fact with this variable, so that
we can later record it as the first parent when "--no-ff" is in
effect.

- The "parents" list is created for this function by reduce_heads()
and was not deallocated after its use, even though the loop
control was written in such a way to allow us to do so by taking
the "next" element in a separate variable so that it can be used
in the next-step part of the loop control.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/merge.c
index b2d03323cbb8ad382c87f73a134e201f7d8d527f..d2e36e2051d11a97f5d93ee4833ef60a14544c76 100644 (file)
@@ -1061,11 +1061,19 @@ static struct commit_list *collect_parents(struct commit *head_commit,
                                         "not something we can merge");
                remotes = &commit_list_insert(commit, remotes)->next;
        }
-       *remotes = NULL;
 
+       /*
+        * Is the current HEAD reachable from another commit being
+        * merged?  If so we do not want to record it as a parent of
+        * the resulting merge, unless --no-ff is given.  We will flip
+        * this variable to 0 when we find HEAD among the independent
+        * tips being merged.
+        */
+       *head_subsumed = 1;
+
+       /* Find what parents to record by checking independent ones. */
        parents = reduce_heads(remoteheads);
 
-       *head_subsumed = 1; /* we will flip this to 0 when we find it */
        for (remoteheads = NULL, remotes = &remoteheads;
             parents;
             parents = next) {
@@ -1075,6 +1083,7 @@ static struct commit_list *collect_parents(struct commit *head_commit,
                        *head_subsumed = 0;
                else
                        remotes = &commit_list_insert(commit, remotes)->next;
+               free(parents);
        }
        return remoteheads;
 }