From: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
In sequencer.c, caller only pass TODO_SQUASH or TODO_FIXUP to
update_squash_messages(), any other command passed in should be
considered as BUG. Thus I think `return error('unknown command')`
should be replaced as `BUG('unknown command')`.
Signed-off-by: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
---
sequencer: replace error() with BUG() in update_squash_messages()
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1964%2Fbrandb97%2Ffix-sequencer-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1964/brandb97/fix-sequencer-leak-v2
Pull-Request: https://github.com/git/git/pull/1964
Range-diff vs v1:
1: a4bf7dd1cf6 ! 1: aa5ff030b37 sequencer: fix memory leak if `update_squash_messages()` failed
@@ Metadata
Author: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
## Commit message ##
- sequencer: fix memory leak if `update_squash_messages()` failed
+ sequencer: replace error() with BUG() in update_squash_messages()
- In sequencer.c:update_squash_messages, `repo_logmsg_reencode` returns
- either an allocated reencode string or commit buffer if no encode is
- needed. To free `repo_logmsg_reencode` result, `repo_unuse_commit_buffer`
- should be used. However, when encountering the error("unknown command..."),
- the absence of `repo_unuse_commit_buffer` results in a memory leak. I
- think we should add a `repo_unuse_commit_buffer` before return.
+ In sequencer.c, caller only pass TODO_SQUASH or TODO_FIXUP to
+ update_squash_messages(), any other command passed in should be
+ considered as BUG. Thus I think `return error('unknown command')`
+ should be replaced as `BUG('unknown command')`.
Signed-off-by: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
## sequencer.c ##
@@ sequencer.c: static int update_squash_messages(struct repository *r,
- strbuf_addstr(&buf, "\n\n");
strbuf_add_commented_lines(&buf, body, strlen(body),
comment_line_str);
-- } else
-+ } else {
-+ repo_unuse_commit_buffer(r, commit, message);
- return error(_("unknown command: %d"), command);
-+ }
+ } else
+- return error(_("unknown command: %d"), command);
++ BUG(_("unknown command: %d"), command);
repo_unuse_commit_buffer(r, commit, message);
if (!res)
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index b5c4043757e..3cd0dd3434e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2139,7 +2139,7 @@ static int update_squash_messages(struct repository *r,
strbuf_add_commented_lines(&buf, body, strlen(body),
comment_line_str);
} else
- return error(_("unknown command: %d"), command);
+ BUG(_("unknown command: %d"), command);
repo_unuse_commit_buffer(r, commit, message);
if (!res)
base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
--
gitgitgadget
