git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2 08/15] merge: clarify collect_parents() logic
Date: Wed, 29 Apr 2015 14:29:26 -0700	[thread overview]
Message-ID: <1430342973-30344-9-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1430342973-30344-1-git-send-email-gitster@pobox.com>

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 | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index b2d0332..d2e36e2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -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;
 }
-- 
2.4.0-rc3-300-g052d062

  parent reply	other threads:[~2015-04-29 21:30 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-19  1:39 [BUG] "git pull" will regress between 'master' and 'pu' Junio C Hamano
2015-04-19 13:07 ` Jeff King
2015-04-19 17:38   ` brian m. carlson
2015-04-19 18:19     ` Jeff King
2015-04-20 18:59   ` Junio C Hamano
2015-04-20 19:10     ` Jeff King
2015-04-20 19:24       ` Junio C Hamano
2015-04-26  5:25         ` [PATCH 00/14] Teach "git merge FETCH_HEAD" octopus merges Junio C Hamano
2015-04-26  5:25           ` [PATCH 01/14] merge: simplify code flow Junio C Hamano
2015-04-26  5:25           ` [PATCH 02/14] t5520: style fixes Junio C Hamano
2015-04-26  5:25           ` [PATCH 03/14] t5520: test pulling an octopus into an unborn branch Junio C Hamano
2015-04-26  5:25           ` [PATCH 04/14] merge: clarify "pulling into void" special case Junio C Hamano
2015-04-26  5:25           ` [PATCH 05/14] merge: do not check argc to determine number of remote heads Junio C Hamano
2015-04-26  5:25           ` [PATCH 06/14] merge: small leakfix and code simplification Junio C Hamano
2015-04-26  5:26           ` [PATCH 07/14] merge: clarify collect_parents() logic Junio C Hamano
2015-04-26  5:26           ` [PATCH 08/14] merge: split reduce_parents() out of collect_parents() Junio C Hamano
2015-04-26  5:26           ` [PATCH 09/14] merge: narrow scope of merge_names Junio C Hamano
2015-04-26  5:26           ` [PATCH 10/14] merge: extract prepare_merge_message() logic out Junio C Hamano
2015-04-26  5:26           ` [PATCH 11/14] merge: make collect_parents() auto-generate the merge message Junio C Hamano
2015-04-26  5:26           ` [PATCH 12/14] merge: decide if we auto-generate the message early in collect_parents() Junio C Hamano
2015-04-26  5:26           ` [PATCH 13/14] merge: handle FETCH_HEAD internally Junio C Hamano
2015-04-26  5:26           ` [PATCH 14/14] merge: deprecate 'git merge <message> HEAD <commit>' syntax Junio C Hamano
2015-04-29 21:29           ` [PATCH v2 00/15] Teach "git merge FETCH_HEAD" octopus merges Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 01/15] merge: test the top-level merge driver Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 02/15] merge: simplify code flow Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 03/15] t5520: style fixes Junio C Hamano
2015-05-01  8:35               ` Paul Tan
2015-05-03  1:57                 ` Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 04/15] t5520: test pulling an octopus into an unborn branch Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 05/15] merge: clarify "pulling into void" special case Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 06/15] merge: do not check argc to determine number of remote heads Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 07/15] merge: small leakfix and code simplification Junio C Hamano
2015-04-29 21:29             ` Junio C Hamano [this message]
2015-04-29 21:29             ` [PATCH v2 09/15] merge: split reduce_parents() out of collect_parents() Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 10/15] merge: narrow scope of merge_names Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 11/15] merge: extract prepare_merge_message() logic out Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 12/15] merge: make collect_parents() auto-generate the merge message Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 13/15] merge: decide if we auto-generate the message early in collect_parents() Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 14/15] merge: handle FETCH_HEAD internally Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 15/15] merge: deprecate 'git merge <message> HEAD <commit>' syntax Junio C Hamano
2015-04-20 19:28 ` [BUG] "git pull" will regress between 'master' and 'pu' Junio C Hamano
2015-04-21  7:23   ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1430342973-30344-9-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).