{"thread":{"id":"63578","subject":"[PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","startedAt":"2025-06-04T03:10:10Z","lastAt":"2025-06-06T12:23:42Z","messageCount":7,"participants":["Lidong Yan via GitGitGadget","Patrick Steinhardt","lidongyan","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"519629","messageId":"pull.1988.git.git.1749006607791.gitgitgadget@gmail.com","threadId":"63578","inReplyTo":null,"subject":"[PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-06-04T03:10:07Z","receivedAt":"2025-06-04T03:10:10Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\npretty.c:repo_logmsg_reencode() allocated memory should be freed with\nrepo_unuse_commit_buffer(). Callers sometimes forgot free it at exit\npoint. Add `repo_unuse_commit_buffer()` in insert_records_from_trailers\nat builtin/shortlog.c and create_commit at builtin/replay.c\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n    repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()\n    \n    pretty.c:repo_logmsg_reencode() allocated memory should be freed with\n    repo_unuse_commit_buffer(). Callers sometimes forgot free it at exit\n    point. Add repo_unuse_commit_buffer() in insert_records_from_trailers at\n    builtin/shortlog.c and create_commit at builtin/replay.c.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1988%2Fbrandb97%2Ffix-reencode-leak-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1988/brandb97/fix-reencode-leak-v1\nPull-Request: https://github.com/git/git/pull/1988\n\n builtin/replay.c   | 1 +\n builtin/shortlog.c | 4 +++-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 225cef08807..6172c8aacc9 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n \tobj = parse_object(repo, &ret);\n \n out:\n+\trepo_unuse_commit_buffer(the_repository, based_on, message);\n \tfree_commit_extra_headers(extra);\n \tfree_commit_list(parents);\n \tstrbuf_release(&msg);\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 30075b67be8..dfc7e85ae96 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -186,8 +186,10 @@ static void insert_records_from_trailers(struct shortlog *log,\n \tcommit_buffer = repo_logmsg_reencode(the_repository, commit, NULL,\n \t\t\t\t\t     ctx->output_encoding);\n \tbody = strstr(commit_buffer, \"\\n\\n\");\n-\tif (!body)\n+\tif (!body) {\n+\t\trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n \t\treturn;\n+\t}\n \n \ttrailer_iterator_init(&iter, body);\n \twhile (trailer_iterator_advance(&iter)) {\n\nbase-commit: 7014b55638da979331baf8dc31c4e1d697cf2d67\n-- \ngitgitgadget\n"},{"id":"519640","messageId":"aD_8NxMi6Dk7CmSl@pks.im","threadId":"63578","inReplyTo":"pull.1988.git.git.1749006607791.gitgitgadget@gmail.com","subject":"Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-06-04T07:56:39Z","receivedAt":"2025-06-04T07:56:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:\n> diff --git a/builtin/replay.c b/builtin/replay.c\n> index 225cef08807..6172c8aacc9 100644\n> --- a/builtin/replay.c\n> +++ b/builtin/replay.c\n> @@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n>  \tobj = parse_object(repo, &ret);\n>  \n>  out:\n> +\trepo_unuse_commit_buffer(the_repository, based_on, message);\n>  \tfree_commit_extra_headers(extra);\n>  \tfree_commit_list(parents);\n>  \tstrbuf_release(&msg);\n\nMakes sense. This one _looks_ like a leak that I'd expect to hit in our\ntest suite as it's not part of an error path.\n\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index 30075b67be8..dfc7e85ae96 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -186,8 +186,10 @@ static void insert_records_from_trailers(struct shortlog *log,\n>  \tcommit_buffer = repo_logmsg_reencode(the_repository, commit, NULL,\n>  \t\t\t\t\t     ctx->output_encoding);\n>  \tbody = strstr(commit_buffer, \"\\n\\n\");\n> -\tif (!body)\n> +\tif (!body) {\n> +\t\trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n>  \t\treturn;\n> +\t}\n>  \n>  \ttrailer_iterator_init(&iter, body);\n>  \twhile (trailer_iterator_advance(&iter)) {\n\nShould this one maybe be converted into a `goto out` so that we can\nrelease resources in a single location, only? Something like the below\npatch.\n\nPatrick\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 30075b67be8..dd08bc40161 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -177,7 +177,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \tstruct strbuf ident = STRBUF_INIT;\n \n \tif (!log->trailers.nr)\n-\t\treturn;\n+\t\tgoto out;\n \n \t/*\n \t * Using repo_format_commit_message(\"%B\") would be simpler here, but\n@@ -187,7 +187,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\t\t\t\t     ctx->output_encoding);\n \tbody = strstr(commit_buffer, \"\\n\\n\");\n \tif (!body)\n-\t\treturn;\n+\t\tgoto out;\n \n \ttrailer_iterator_init(&iter, body);\n \twhile (trailer_iterator_advance(&iter)) {\n@@ -206,6 +206,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t}\n \ttrailer_iterator_release(&iter);\n \n+out:\n \tstrbuf_release(&ident);\n \trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n }\n"},{"id":"519652","messageId":"8AC87451-1FC6-4359-9A16-928B77185779@smail.nju.edu.cn","threadId":"63578","inReplyTo":"aD_8NxMi6Dk7CmSl@pks.im","subject":"Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"lidongyan","fromEmail":"502024330056@smail.nju.edu.cn","sentAt":"2025-06-04T10:50:46Z","receivedAt":"2025-06-04T11:00:03Z","isPatch":true,"sender":{"key":"502024330056@smail.nju.edu.cn","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"2025年6月4日 15:56，Patrick Steinhardt <ps@pks.im> 写道：\n> \n> On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:\n>> diff --git a/builtin/replay.c b/builtin/replay.c\n>> index 225cef08807..6172c8aacc9 100644\n>> --- a/builtin/replay.c\n>> +++ b/builtin/replay.c\n>> @@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n>> obj = parse_object(repo, &ret);\n>> \n>> out:\n>> + repo_unuse_commit_buffer(the_repository, based_on, message);\n>> free_commit_extra_headers(extra);\n>> free_commit_list(parents);\n>> strbuf_release(&msg);\n> \n> Makes sense. This one _looks_ like a leak that I'd expect to hit in our\n> test suite as it's not part of an error path.\n> \n>> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n>> index 30075b67be8..dfc7e85ae96 100644\n>> --- a/builtin/shortlog.c\n>> +++ b/builtin/shortlog.c\n>> @@ -186,8 +186,10 @@ static void insert_records_from_trailers(struct shortlog *log,\n>> commit_buffer = repo_logmsg_reencode(the_repository, commit, NULL,\n>>     ctx->output_encoding);\n>> body = strstr(commit_buffer, \"\\n\\n\");\n>> - if (!body)\n>> + if (!body) {\n>> + repo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n>> return;\n>> + }\n>> \n>> trailer_iterator_init(&iter, body);\n>> while (trailer_iterator_advance(&iter)) {\n> \n> Should this one maybe be converted into a `goto out` so that we can\n> release resources in a single location, only? Something like the below\n> patch.\n> \n> Patrick\n> \n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index 30075b67be8..dd08bc40161 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -177,7 +177,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n> struct strbuf ident = STRBUF_INIT;\n> \n> if (!log->trailers.nr)\n> - return;\n> + goto out;\n> \n> /*\n> * Using repo_format_commit_message(\"%B\") would be simpler here, but\n> @@ -187,7 +187,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n>     ctx->output_encoding);\n> body = strstr(commit_buffer, \"\\n\\n\");\n> if (!body)\n> - return;\n> + goto out;\n> \n> trailer_iterator_init(&iter, body);\n> while (trailer_iterator_advance(&iter)) {\n> @@ -206,6 +206,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n> }\n> trailer_iterator_release(&iter);\n> \n> +out:\n> strbuf_release(&ident);\n> repo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n> }\n> \n\nReplace return with goto out do looks better. Ident initialized\nto STRBUF_INIT means ident->alloc = 0, so release on it is also safe.\n\n"},{"id":"519711","messageId":"pull.1988.v2.git.git.1749104846531.gitgitgadget@gmail.com","threadId":"63578","inReplyTo":"pull.1988.git.git.1749006607791.gitgitgadget@gmail.com","subject":"[PATCH v2] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-06-05T06:27:26Z","receivedAt":"2025-06-05T06:27:29Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\npretty.c:repo_logmsg_reencode() allocated memory should be freed with\nrepo_unuse_commit_buffer(). Callers sometimes forgot free it at exit\npoint. Add `repo_unuse_commit_buffer()` in insert_records_from_trailers\nat builtin/shortlog.c and create_commit at builtin/replay.c\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n    repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()\n    \n    pretty.c:repo_logmsg_reencode() allocated memory should be freed with\n    repo_unuse_commit_buffer(). Callers sometimes forgot free it at exit\n    point. Add repo_unuse_commit_buffer() in insert_records_from_trailers at\n    builtin/shortlog.c and create_commit at builtin/replay.c.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1988%2Fbrandb97%2Ffix-reencode-leak-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1988/brandb97/fix-reencode-leak-v2\nPull-Request: https://github.com/git/git/pull/1988\n\nRange-diff vs v1:\n\n 1:  a414074f167 ! 1:  f5165d6a102 repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()\n     @@ builtin/replay.c: static struct commit *create_commit(struct repository *repo,\n      \n       ## builtin/shortlog.c ##\n      @@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *log,\n     - \tcommit_buffer = repo_logmsg_reencode(the_repository, commit, NULL,\n       \t\t\t\t\t     ctx->output_encoding);\n       \tbody = strstr(commit_buffer, \"\\n\\n\");\n     --\tif (!body)\n     -+\tif (!body) {\n     -+\t\trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n     - \t\treturn;\n     -+\t}\n     + \tif (!body)\n     +-\t\treturn;\n     ++\t\tgoto out;\n       \n       \ttrailer_iterator_init(&iter, body);\n       \twhile (trailer_iterator_advance(&iter)) {\n     +@@ builtin/shortlog.c: static void insert_records_from_trailers(struct shortlog *log,\n     + \t}\n     + \ttrailer_iterator_release(&iter);\n     + \n     ++out:\n     + \tstrbuf_release(&ident);\n     + \trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n     + }\n\n\n builtin/replay.c   | 1 +\n builtin/shortlog.c | 3 ++-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 225cef08807..6172c8aacc9 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n \tobj = parse_object(repo, &ret);\n \n out:\n+\trepo_unuse_commit_buffer(the_repository, based_on, message);\n \tfree_commit_extra_headers(extra);\n \tfree_commit_list(parents);\n \tstrbuf_release(&msg);\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 30075b67be8..fe15e114973 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -187,7 +187,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t\t\t\t\t     ctx->output_encoding);\n \tbody = strstr(commit_buffer, \"\\n\\n\");\n \tif (!body)\n-\t\treturn;\n+\t\tgoto out;\n \n \ttrailer_iterator_init(&iter, body);\n \twhile (trailer_iterator_advance(&iter)) {\n@@ -206,6 +206,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n \t}\n \ttrailer_iterator_release(&iter);\n \n+out:\n \tstrbuf_release(&ident);\n \trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n }\n\nbase-commit: 7014b55638da979331baf8dc31c4e1d697cf2d67\n-- \ngitgitgadget\n"},{"id":"519730","messageId":"20250605072308.GA2066712@coredump.intra.peff.net","threadId":"63578","inReplyTo":"aD_8NxMi6Dk7CmSl@pks.im","subject":"Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T07:23:08Z","receivedAt":"2025-06-05T07:23:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 04, 2025 at 09:56:39AM +0200, Patrick Steinhardt wrote:\n\n> On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:\n> > diff --git a/builtin/replay.c b/builtin/replay.c\n> > index 225cef08807..6172c8aacc9 100644\n> > --- a/builtin/replay.c\n> > +++ b/builtin/replay.c\n> > @@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n> >  \tobj = parse_object(repo, &ret);\n> >  \n> >  out:\n> > +\trepo_unuse_commit_buffer(the_repository, based_on, message);\n> >  \tfree_commit_extra_headers(extra);\n> >  \tfree_commit_list(parents);\n> >  \tstrbuf_release(&msg);\n> \n> Makes sense. This one _looks_ like a leak that I'd expect to hit in our\n> test suite as it's not part of an error path.\n\nWe'll usually never flag a leak for commit buffers, because they are\nstored in (and owned by) a commit-slab. So the memory is not leaked\nexactly, but we may hold on to it longer than we need to. This mostly\nonly becomes obvious when we do it for every commit in a code path that\ntouches a lot of commits (e.g., \"git log\" or something).\n\nThe exception is if we actually had re-encode, which requires a mismatch\nbetween the commit and output encodings (which both default to UTF-8).\nAnd then it really is a leak.\n\nIf we add a hack like this:\n\ndiff --git a/utf8.c b/utf8.c\nindex 35a0251939..d7b7d372c5 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -3,6 +3,7 @@\n #include \"git-compat-util.h\"\n #include \"strbuf.h\"\n #include \"utf8.h\"\n+#include \"parse.h\"\n \n /* This code is originally from https://www.cl.cam.ac.uk/~mgk25/ucs/ */\n \n@@ -442,6 +443,12 @@ int is_encoding_utf8(const char *name)\n int same_encoding(const char *src, const char *dst)\n {\n \tstatic const char utf8[] = \"UTF-8\";\n+\tstatic int always_reencode = -1;\n+\n+\tif (always_reencode < 0)\n+\t\talways_reencode = git_env_bool(\"GIT_TEST_ALWAYS_REENCODE\", 0);\n+\tif (always_reencode)\n+\t\treturn 0;\n \n \tif (!src)\n \t\tsrc = utf8;\n\nthen running:\n\n  GIT_TEST_ALWAYS_REENCODE=1 make SANITIZE=leak test\n\nturns up this leak via t3650-replay-basics.sh (as well as in t6429).\n\nIt's probably a bit too specialized to carry around as a permanent test\nmode, though. I thought it might find other cases, but it doesn't. The\nother one in this patch only triggers when the commit message has no\nheader separator, which is not very likely.\n\n> > -\tif (!body)\n> > +\tif (!body) {\n> > +\t\trepo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n> >  \t\treturn;\n> > +\t}\n> >  \n> >  \ttrailer_iterator_init(&iter, body);\n> >  \twhile (trailer_iterator_advance(&iter)) {\n> \n> Should this one maybe be converted into a `goto out` so that we can\n> release resources in a single location, only? Something like the below\n> patch.\n\nYeah, I think that is nicer, though...\n\n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index 30075b67be8..dd08bc40161 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -177,7 +177,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n>  \tstruct strbuf ident = STRBUF_INIT;\n>  \n>  \tif (!log->trailers.nr)\n> -\t\treturn;\n> +\t\tgoto out;\n\nIf you convert this hunk, then we'd look at the uninitialized\ncommit_buffer variable after we jump to the out label. I think the v2\njust posted is OK, though (it touches only the one conditional that\nneeds the goto).\n\n-Peff\n"},{"id":"519733","messageId":"20250605075301.GD2066712@coredump.intra.peff.net","threadId":"63578","inReplyTo":"pull.1988.v2.git.git.1749104846531.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T07:53:01Z","receivedAt":"2025-06-05T07:53:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2025 at 06:27:26AM +0000, Lidong Yan via GitGitGadget wrote:\n\n> From: Lidong Yan <502024330056@smail.nju.edu.cn>\n> \n> pretty.c:repo_logmsg_reencode() allocated memory should be freed with\n> repo_unuse_commit_buffer(). Callers sometimes forgot free it at exit\n> point. Add `repo_unuse_commit_buffer()` in insert_records_from_trailers\n> at builtin/shortlog.c and create_commit at builtin/replay.c\n\nThe patch here looks fine, and I can confirm with the hacky test-patch\nI showed elsewhere in the thread that the case in replay.c is fixed.\n\nI don't think the shortlog one is triggered by the test suite, and it's\nprobably not worth adding a specific test for a commit with no header\nseparator.\n\n-Peff\n"},{"id":"519846","messageId":"1487EB97-8D97-4535-ACF2-96AE8C3F5DB2@smail.nju.edu.cn","threadId":"63578","inReplyTo":"20250605072308.GA2066712@coredump.intra.peff.net","subject":"Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()","fromName":"lidongyan","fromEmail":"502024330056@smail.nju.edu.cn","sentAt":"2025-06-06T12:22:46Z","receivedAt":"2025-06-06T12:23:42Z","isPatch":true,"sender":{"key":"502024330056@smail.nju.edu.cn","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"2025年6月5日 15:23，Jeff King <peff@peff.net> 写道：\n> \n> On Wed, Jun 04, 2025 at 09:56:39AM +0200, Patrick Steinhardt wrote:\n> \n>> On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:\n>>> diff --git a/builtin/replay.c b/builtin/replay.c\n>>> index 225cef08807..6172c8aacc9 100644\n>>> --- a/builtin/replay.c\n>>> +++ b/builtin/replay.c\n>>> @@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo,\n>>> obj = parse_object(repo, &ret);\n>>> \n>>> out:\n>>> + repo_unuse_commit_buffer(the_repository, based_on, message);\n>>> free_commit_extra_headers(extra);\n>>> free_commit_list(parents);\n>>> strbuf_release(&msg);\n>> \n>> Makes sense. This one _looks_ like a leak that I'd expect to hit in our\n>> test suite as it's not part of an error path.\n> \n> We'll usually never flag a leak for commit buffers, because they are\n> stored in (and owned by) a commit-slab. So the memory is not leaked\n> exactly, but we may hold on to it longer than we need to. This mostly\n> only becomes obvious when we do it for every commit in a code path that\n> touches a lot of commits (e.g., \"git log\" or something).\n\nI understand. The static analysis tool which I used to test git find\nrepo_logmsg_reencode() might allocates memory through xstrdup()\nor reencode_string(), then it report a leak. And I find that xstrdup() is\nactually dead code. So only reencode_string() may cause leaks. \n  \n> The exception is if we actually had re-encode, which requires a mismatch\n> between the commit and output encodings (which both default to UTF-8).\n> And then it really is a leak.\n> \n\nAgreed.\n\n> If we add a hack like this:\n> \n> diff --git a/utf8.c b/utf8.c\n> index 35a0251939..d7b7d372c5 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -3,6 +3,7 @@\n> #include \"git-compat-util.h\"\n> #include \"strbuf.h\"\n> #include \"utf8.h\"\n> +#include \"parse.h\"\n> \n> /* This code is originally from https://www.cl.cam.ac.uk/~mgk25/ucs/ */\n> \n> @@ -442,6 +443,12 @@ int is_encoding_utf8(const char *name)\n> int same_encoding(const char *src, const char *dst)\n> {\n> static const char utf8[] = \"UTF-8\";\n> + static int always_reencode = -1;\n> +\n> + if (always_reencode < 0)\n> + always_reencode = git_env_bool(\"GIT_TEST_ALWAYS_REENCODE\", 0);\n> + if (always_reencode)\n> + return 0;\n> \n> if (!src)\n> src = utf8;\n> \n> then running:\n> \n>  GIT_TEST_ALWAYS_REENCODE=1 make SANITIZE=leak test\n> \n> turns up this leak via t3650-replay-basics.sh (as well as in t6429).\n> \n> It's probably a bit too specialized to carry around as a permanent test\n> mode, though. I thought it might find other cases, but it doesn't. The\n> other one in this patch only triggers when the commit message has no\n> header separator, which is not very likely.\n> \n>>> - if (!body)\n>>> + if (!body) {\n>>> + repo_unuse_commit_buffer(the_repository, commit, commit_buffer);\n>>> return;\n>>> + }\n>>> \n>>> trailer_iterator_init(&iter, body);\n>>> while (trailer_iterator_advance(&iter)) {\n>> \n>> Should this one maybe be converted into a `goto out` so that we can\n>> release resources in a single location, only? Something like the below\n>> patch.\n> \n> Yeah, I think that is nicer, though...\n> \n>> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n>> index 30075b67be8..dd08bc40161 100644\n>> --- a/builtin/shortlog.c\n>> +++ b/builtin/shortlog.c\n>> @@ -177,7 +177,7 @@ static void insert_records_from_trailers(struct shortlog *log,\n>> struct strbuf ident = STRBUF_INIT;\n>> \n>> if (!log->trailers.nr)\n>> - return;\n>> + goto out;\n> \n> If you convert this hunk, then we'd look at the uninitialized\n> commit_buffer variable after we jump to the out label. I think the v2\n> just posted is OK, though (it touches only the one conditional that\n> needs the goto).\n> \n> -Peff\n\nI actually learn from your hack that GIT_TEST_ is something like GIT_TRACE,\nboth aids to test and debug.\n\nThanks,\nLidong\n\n"}]}