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
we added another place where we'd "return" before reaching the
strbuf_release().
Let's move it to a "cleanup" label, and use an appropriate "goto". It
may or may not be safe to combine the existing "leave" and "cleanup"
labels, but this change doesn't attempt to answer that question. Let's
instead avoid calling update_abort_safety_file() in these cases, as we
didn't do so before.
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)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
sequencer.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 47367e66842..db8d789fa76 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2280,8 +2280,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 cleanup;
+ }
flags |= AMEND_MSG;
if (!final_fixup)
msg_file = rebase_path_squash_msg();
@@ -2291,9 +2293,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 cleanup;
+ }
unlink(git_path_merge_msg(r));
msg_file = dest;
flags |= EDIT_MSG;
@@ -2331,7 +2335,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
@@ -2403,9 +2406,11 @@ static int do_pick_commit(struct repository *r,
}
leave:
+ update_abort_safety_file();
+cleanup:
free_message(commit, &msg);
free(author);
- update_abort_safety_file();
+ strbuf_release(&msgbuf);
return res;
}
--
2.39.0.1153.g589e4efe9dc
