Thread (1 message) 1 message, 1 author, 2022-01-22

Re: [PATCH 2/2] merge: fix memory leaks in cmd_merge()

From: Junio C Hamano <hidden>
Date: 2022-01-22 00:05:23

"Elijah Newren via GitGitGadget" [off-list ref] writes:
 done:
+	if (!automerge_was_ok) {
+		free_commit_list(common);
+		free_commit_list(remoteheads);
+	}
I wondered what happens upon a successful automerge.

We call finish_automerge() and come here, and I do see a call to
free_commit_list(common) in finish_automerge(), but the way
remoteheads is used looked a bit iffy.

In finish_automerge(), we use a temporary variable parents to point
remoteheads at it, and conditionally prepend the current HEAD at the
beginning of the parents list.  This is passed to commit_tree(),
which does pop_commit() to consume all commits on the list.

So after commit_tree() returns, all commit_list instances on
remoteheads list, and possibly the one finish_automerge() prepended
for the current HEAD, are all consumed, and there is no need to
call, and it would be wrong to call, free_commit_list(), at this
point.

So, I agree that this conditional freeing is correct.  It was just
it was a bit hard to see.

Thanks.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help