sequencer.c: always free() the "msgbuf" in do_pick_commit()

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@dunelm.org.uk/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index cb4b1ce062cbe7a84edb5b4a587e6d9b3f97a5a0..fb23f734ade4902f6472d9cf0db1155389f02fa1 100644 (file)
--- 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 @@ fast_forward_edit:
 leave:
        free_message(commit, &msg);
        free(author);
+       strbuf_release(&msgbuf);
        update_abort_safety_file();
 
        return res;