git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Andreas Jacobsen <andreas@andreasjacobsen.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] merge-tree: handle directory/empty conflict correctly
Date: Mon, 6 May 2013 16:20:54 +0100	[thread overview]
Message-ID: <20130506152054.GE25912@serenity.lan> (raw)
In-Reply-To: <20130506144958.GD25912@serenity.lan>

git-merge-tree causes a null pointer dereference when a directory
entry exists in only one or two of the three trees being compared with
no corresponding entry in the other tree(s).

When this happens, we want to handle the entry as a directory and not
attempt to mark it as a file merge.  Do this by setting the entries bit
in the directory mask when the entry is missing or when it is a
directory, only performing the file comparison when we know that a file
entry exists.

Reported-by: Andreas Jacobsen <andreas@andreasjacobsen.com>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Andreas, can you try this patch and see if it fixes your test case?

 builtin/merge-tree.c  |  6 +++++-
 t/t4300-merge-tree.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ec49917..61cbde4 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -251,7 +251,11 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3])
 
 	for (i = 0; i < 3; i++) {
 		mask |= (1 << i);
-		if (n[i].mode && S_ISDIR(n[i].mode))
+		/*
+		 * Treat missing entries as directories so that we return
+		 * after unresolved_directory has handled this.
+		 */
+		if (!n[i].mode || S_ISDIR(n[i].mode))
 			dirmask |= (1 << i);
 	}
 
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 2defb42..9015e47 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -259,6 +259,57 @@ EXPECTED
 	test_cmp expected actual
 '
 
+test_expect_success 'tree add A, B (same)' '
+	cat >expect <<-\EOF &&
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "file" add-tree-A &&
+	git merge-tree initial add-tree-A add-tree-A >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'tree add A, B (different)' '
+	cat >expect <<-\EOF &&
+	added in both
+	  our    100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	  their  100644 ba629238ca89489f2b350e196ca445e09d8bb834 sub/file
+	@@ -1 +1,5 @@
+	+<<<<<<< .our
+	 AAA
+	+=======
+	+BBB
+	+>>>>>>> .their
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "AAA" add-tree-a-b-A &&
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "BBB" add-tree-a-b-B &&
+	git merge-tree initial add-tree-a-b-A add-tree-a-b-B >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'tree unchanged A, removed B' '
+	cat >expect <<-\EOF &&
+	removed in remote
+	  base   100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	  our    100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/file
+	@@ -1 +0,0 @@
+	-AAA
+	EOF
+	git reset --hard initial &&
+	mkdir sub &&
+	test_commit "add sub/file" "sub/file" "AAA" tree-remove-b-initial &&
+	git rm sub/file &&
+	test_tick &&
+	git commit -m "remove sub/file" &&
+	git tag tree-remove-b-B &&
+	git merge-tree tree-remove-b-initial tree-remove-b-initial tree-remove-b-B >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'turn file to tree' '
 	git reset --hard initial &&
 	rm initial-file &&
-- 
1.8.3.rc0.149.g98a72f2.dirty

  reply	other threads:[~2013-05-06 15:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-06 13:02 Segfault in git merge-tree (1.8.2.2) Andreas Jacobsen
2013-05-06 13:39 ` John Keeping
2013-05-06 14:13   ` Andreas Jacobsen
2013-05-06 14:29     ` John Keeping
2013-05-06 14:49       ` John Keeping
2013-05-06 15:20         ` John Keeping [this message]
2013-05-07  4:54           ` [PATCH] merge-tree: handle directory/empty conflict correctly Andreas Jacobsen
2013-05-07  5:17             ` Junio C Hamano

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=20130506152054.GE25912@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=andreas@andreasjacobsen.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).