Skip to content

Commit a5792e9

Browse files
avargitster
authored andcommitted
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. 452202c (sequencer: stop releasing the strbuf in write_message(), 2016-10-21) 2. f241ff0 (prepare the builtins for a libified merge_recursive(), 2016-07-26) 3. 6e98de7 (sequencer (rebase -i): add support for the 'fixup' and 'squash' commands, 2017-01-02) 4. https://lore.kernel.org/git/[email protected]/ Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 94ad545 commit a5792e9

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

sequencer.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2277,8 +2277,10 @@ static int do_pick_commit(struct repository *r,
22772277
reword = 1;
22782278
else if (is_fixup(command)) {
22792279
if (update_squash_messages(r, command, commit,
2280-
opts, item->flags))
2281-
return -1;
2280+
opts, item->flags)) {
2281+
res = -1;
2282+
goto leave;
2283+
}
22822284
flags |= AMEND_MSG;
22832285
if (!final_fixup)
22842286
msg_file = rebase_path_squash_msg();
@@ -2288,9 +2290,11 @@ static int do_pick_commit(struct repository *r,
22882290
} else {
22892291
const char *dest = git_path_squash_msg(r);
22902292
unlink(dest);
2291-
if (copy_file(dest, rebase_path_squash_msg(), 0666))
2292-
return error(_("could not rename '%s' to '%s'"),
2293-
rebase_path_squash_msg(), dest);
2293+
if (copy_file(dest, rebase_path_squash_msg(), 0666)) {
2294+
res = error(_("could not rename '%s' to '%s'"),
2295+
rebase_path_squash_msg(), dest);
2296+
goto leave;
2297+
}
22942298
unlink(git_path_merge_msg(r));
22952299
msg_file = dest;
22962300
flags |= EDIT_MSG;
@@ -2328,7 +2332,6 @@ static int do_pick_commit(struct repository *r,
23282332
free_commit_list(common);
23292333
free_commit_list(remotes);
23302334
}
2331-
strbuf_release(&msgbuf);
23322335

23332336
/*
23342337
* 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,
24022405
leave:
24032406
free_message(commit, &msg);
24042407
free(author);
2408+
strbuf_release(&msgbuf);
24052409
update_abort_safety_file();
24062410

24072411
return res;

0 commit comments

Comments
 (0)