[PATCH v2] revert: plug memory leak in "cherry-pick root commit" codepath
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Aug 16, 2011, 18:16 UTC
- Message-ID
- <20110816181633.GB10336@elie.gateway.2wire.net>
- In-Reply-To
- <7v39h1i6rr.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 5 quoted lines
> 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