2025年6月4日 15:56，Patrick Steinhardt <ps@xxxxxx> 写道：
>
> 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);
> }
>
Replace return with goto out do looks better. Ident initialized
to STRBUF_INIT means ident->alloc = 0, so release on it is also safe.
