In [1] the strbuf_release(&msgbuf) was moved into this do_pick_commit(), but didn't take into account the case of [2], where we'd return before the strbuf_release(&msgbuf). Then when the "fixup" support was added in [3] this leak got worse, as in this error case we added another place where we'd "return" before reaching the strbuf_release(). This changes the behavior so that we'll call update_abort_safety_file() in these cases where we'd previously "return", but as noted in [4] "update_abort_safety_file() is a no-op when rebasing and you're changing code that is only run when rebasing.". Here "no-op" refers to the early return in update_abort_safety_file() if git_path_seq_dir() doesn't exist. 1. 452202c74b8 (sequencer: stop releasing the strbuf in write_message(), 2016-10-21) 2. f241ff0d0a9 (prepare the builtins for a libified merge_recursive(), 2016-07-26) 3. 6e98de72c03 (sequencer (rebase -i): add support for the 'fixup' and 'squash' commands, 2017-01-02) 4. https://lore.kernel.org/git/bcace50b-a4c3-c468-94a3-4fe0c62b3671@xxxxxxxxxxxxx/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> --- sequencer.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sequencer.c b/sequencer.c index ea568d99498..2c5ed5bc5ea 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2277,8 +2277,10 @@ static int do_pick_commit(struct repository *r, reword = 1; else if (is_fixup(command)) { if (update_squash_messages(r, command, commit, - opts, item->flags)) - return -1; + opts, item->flags)) { + res = -1; + goto leave; + } flags |= AMEND_MSG; if (!final_fixup) msg_file = rebase_path_squash_msg(); @@ -2288,9 +2290,11 @@ static int do_pick_commit(struct repository *r, } else { const char *dest = git_path_squash_msg(r); unlink(dest); - if (copy_file(dest, rebase_path_squash_msg(), 0666)) - return error(_("could not rename '%s' to '%s'"), - rebase_path_squash_msg(), dest); + if (copy_file(dest, rebase_path_squash_msg(), 0666)) { + res = error(_("could not rename '%s' to '%s'"), + rebase_path_squash_msg(), dest); + goto leave; + } unlink(git_path_merge_msg(r)); msg_file = dest; flags |= EDIT_MSG; @@ -2328,7 +2332,6 @@ static int do_pick_commit(struct repository *r, free_commit_list(common); free_commit_list(remotes); } - strbuf_release(&msgbuf); /* * If the merge was clean or if it failed due to conflict, we write @@ -2402,6 +2405,7 @@ static int do_pick_commit(struct repository *r, leave: free_message(commit, &msg); free(author); + strbuf_release(&msgbuf); update_abort_safety_file(); return res; -- 2.39.0.1225.g30a3d88132d
