[PATCH v2] revert: plug memory leak in "cherry-pick root commit" codepath

[Jonathan Nieder <jrnieder@gmail.com>]

Junio C Hamano wrote:

> Thanks for noticing, but shouldn't we be just using
>
> 	lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN)
>
> or something, instead of hand-crafting a fake tree object?

Yes.  Ever since v1.5.5-rc0~180^2~1 (hard-code the empty tree object,
2008-02-13), there is no need to call pretend_sha1_file() again to
get a fake empty tree object, so this lookup_tree() should work
fine.

-- >8 --
Subject: revert: plug memory leak in "cherry-pick root commit" codepath

For each parentless commit it is asked to reuse, "git cherry-pick" and
"git revert" hand-craft a fake parent tree object on the heap to pass
to merge_trees().  Leaking such a small one-time allocation would not
be a big deal, but now that cherry-pick/revert can take multiple
commit arguments, it can start to add up.

The fix is simple: don't create a new fake empty tree at all, but rely
on the built-in one that has existed since 346245a1 (hard-code the
empty tree object, 2008-02-13).

While at it, add a test to make sure cherry-picking multiple
parentless commits continues to work.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Improved-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c            |    7 +------
 t/t3503-cherry-pick-root.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 853e9e40..a26a7c93 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -273,12 +273,7 @@ static void write_message(struct strbuf *msgbuf, const char *filename)
 
 static struct tree *empty_tree(void)
 {
-	struct tree *tree = xcalloc(1, sizeof(struct tree));
-
-	tree->object.parsed = 1;
-	tree->object.type = OBJ_TREE;
-	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
-	return tree;
+	return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
 }
 
 static NORETURN void die_dirty_index(const char *me)
diff --git a/t/t3503-cherry-pick-root.sh b/t/t3503-cherry-pick-root.sh
index b0faa299..472e5b80 100755
--- a/t/t3503-cherry-pick-root.sh
+++ b/t/t3503-cherry-pick-root.sh
@@ -16,15 +16,40 @@ test_expect_success setup '
 	echo second > file2 &&
 	git add file2 &&
 	test_tick &&
-	git commit -m "second"
+	git commit -m "second" &&
+
+	git symbolic-ref HEAD refs/heads/third &&
+	rm .git/index file2 &&
+	echo third > file3 &&
+	git add file3 &&
+	test_tick &&
+	git commit -m "third"
 
 '
 
 test_expect_success 'cherry-pick a root commit' '
 
+	git checkout second^0 &&
 	git cherry-pick master &&
 	test first = $(cat file1)
 
 '
 
+test_expect_success 'cherry-pick two root commits' '
+
+	echo first >expect.file1 &&
+	echo second >expect.file2 &&
+	echo third >expect.file3 &&
+
+	git checkout second^0 &&
+	git cherry-pick master third &&
+
+	test_cmp expect.file1 file1 &&
+	test_cmp expect.file2 file2 &&
+	test_cmp expect.file3 file3 &&
+	git rev-parse --verify HEAD^^ &&
+	test_must_fail git rev-parse --verify HEAD^^^
+
+'
+
 test_done
-- 
1.7.6