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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2022, 00:05 UTC
Message-ID: <xmqqczkkeitd.fsf@gitster.g>
In-Reply-To: <c6522d6449f9323edbbd9329b2859368492aabd3.1642664835.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

>  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.