Message ID | c6522d6449f9323edbbd9329b2859368492aabd3.1642664835.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 6046f7a91c3bf5c76702f10a4a83e8a63afe2fb4 |
Headers | show |
Series | Fix some old memory leaks in merge-ort and builtin/merge | expand |
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> 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.
diff --git a/builtin/merge.c b/builtin/merge.c index 74e53cf20a7..bd8fff9b223 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1273,7 +1273,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0; struct commit_list *common = NULL; const char *best_strategy = NULL, *wt_strategy = NULL; - struct commit_list *remoteheads, *p; + struct commit_list *remoteheads = NULL, *p; void *branch_to_free; int orig_argc = argc; @@ -1752,6 +1752,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix) ret = suggest_conflicts(); done: + if (!automerge_was_ok) { + free_commit_list(common); + free_commit_list(remoteheads); + } strbuf_release(&buf); free(branch_to_free); return ret;