Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()
From: Patrick Steinhardt <hidden>
Date: 2025-06-04 07:56:43
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/replay.c b/builtin/replay.c index 225cef08807..6172c8aacc9 100644 --- a/builtin/replay.c +++ b/builtin/replay.c@@ -84,6 +84,7 @@ static struct commit *create_commit(struct repository *repo, obj = parse_object(repo, &ret); out: + repo_unuse_commit_buffer(the_repository, based_on, message); free_commit_extra_headers(extra); free_commit_list(parents); strbuf_release(&msg);
Makes sense. This one _looks_ like a leak that I'd expect to hit in our test suite as it's not part of an error path.
quoted hunk ↗ jump to hunk
diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 30075b67be8..dfc7e85ae96 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c@@ -186,8 +186,10 @@ static void insert_records_from_trailers(struct shortlog *log, commit_buffer = repo_logmsg_reencode(the_repository, commit, NULL, ctx->output_encoding); body = strstr(commit_buffer, "\n\n"); - if (!body) + if (!body) { + repo_unuse_commit_buffer(the_repository, commit, commit_buffer); return; + } trailer_iterator_init(&iter, body); while (trailer_iterator_advance(&iter)) {
Should this one maybe be converted into a `goto out` so that we can release resources in a single location, only? Something like the below patch. Patrick
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 30075b67be8..dd08bc40161 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c@@ -177,7 +177,7 @@ static void insert_records_from_trailers(struct shortlog *log, struct strbuf ident = STRBUF_INIT; if (!log->trailers.nr) - return; + goto out; /* * Using repo_format_commit_message("%B") would be simpler here, but
@@ -187,7 +187,7 @@ static void insert_records_from_trailers(struct shortlog *log, ctx->output_encoding); body = strstr(commit_buffer, "\n\n"); if (!body) - return; + goto out; trailer_iterator_init(&iter, body); while (trailer_iterator_advance(&iter)) {
@@ -206,6 +206,7 @@ static void insert_records_from_trailers(struct shortlog *log, } trailer_iterator_release(&iter); +out: strbuf_release(&ident); repo_unuse_commit_buffer(the_repository, commit, commit_buffer); }