[PATCH v2] sequencer: replace error() with BUG() in update_squash_messages()

["Lidong Yan via GitGitGadget" <gitgitgadget@gmail.com>]

From: Lidong Yan <502024330056@smail.nju.edu.cn>

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@smail.nju.edu.cn>
---
    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@smail.nju.edu.cn>
      
       ## 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@smail.nju.edu.cn>
      
       ## 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