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