Skip to content

Commit

Permalink
revert: plug memory leak in "cherry-pick root commit" codepath
Browse files Browse the repository at this point in the history
The empty tree passed as common ancestor to merge_trees() when
cherry-picking a parentless commit is allocated on the heap and never
freed.  Leaking such a small one-time allocation is not a very big
problem, but now that "git cherry-pick" can cherry-pick multiple
commits it can start to add up.

Avoid the leak by storing the fake tree exactly once in the BSS
section (i.e., use a static).  While at it, let's add a test to make
sure cherry-picking multiple parentless commits continues to work.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jonathan Nieder authored and Junio C Hamano committed Aug 16, 2011
1 parent 86c7bb4 commit 127f045
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
7 changes: 1 addition & 6 deletions builtin/revert.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 26 additions & 1 deletion t/t3503-cherry-pick-root.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 127f045

Please sign in to comment.