Re: [PATCH] repo_logmsg_reencode: fix memory leak when use repo_logmsg_reencode()

On Wed, Jun 04, 2025 at 03:10:07AM +0000, Lidong Yan via GitGitGadget wrote:

quoted

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

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);
}