On 14/05/2025 14:53, Lidong Yan via GitGitGadget wrote:
From: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
In sequencer.c:todo_list_rearrange_squash, if it fails, memory
allocated in `next`, `tail`, `subjects` and `subject2item` will leak.
Jump to cleanup label before return could fix this leak problem.
You could mention that you're adding the cleanup label.
I suspect reaching this condition is a bug as well because we should
only rearrange the todo list when the rebase starts. However I'm not
100% sure that it is impossible to trigger this condition so lets free
it as you suggest. The code changes look good.
Out of interest how are you finding these leaks?
Thanks
Phillip
Signed-off-by: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
---
sequencer: fix memory leak if todo_list_rearrange_squash() failed
In sequencer.c:todo_list_rearrange_squash, if it fails, memory allocated
in next, tail, subjects and subject2item will leak. Jump to cleanup
label before return could fix this leak problem.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1965%2Fbrandb97%2Ffix-sequencer-todo-leak-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1965/brandb97/fix-sequencer-todo-leak-v1
Pull-Request: https://github.com/git/git/pull/1965
sequencer.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b5c4043757e..5fb7b68a7ab 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6596,6 +6596,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
char **subjects;
struct commit_todo_item commit_todo;
struct todo_item *items = NULL;
+ int ret = 0;
init_commit_todo_item(&commit_todo);
/*
@@ -6626,8 +6627,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
}
if (is_fixup(item->command)) {
- clear_commit_todo_item(&commit_todo);
- return error(_("the script was already rearranged."));
+ ret = error(_("the script was already rearranged."));
+ goto cleanup;
}
repo_parse_commit(the_repository, item->commit);
@@ -6729,6 +6730,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
todo_list->items = items;
}
+cleanup:
free(next);
free(tail);
for (i = 0; i < todo_list->nr; i++)
@@ -6738,7 +6740,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
clear_commit_todo_item(&commit_todo);
- return 0;
+ return ret;
}
int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
