Re: [PATCH 27/34] shortlog: release strbuf after use in insert_one_record()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 6, 2017, 19:51 UTC
Message-ID: <xmqqtw0f61xr.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170830180037.20950-8-l.s.r@web.de>

Rene Scharfe <l.s.r@web.de> writes:

Show 17 quoted lines

> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  builtin/shortlog.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> index 43c4799ea9..48af16c681 100644
> --- a/builtin/shortlog.c
> +++ b/builtin/shortlog.c
> @@ -50,66 +50,67 @@ static int compare_by_list(const void *a1, const void *a2)
>  static void insert_one_record(struct shortlog *log,
>  			      const char *author,
>  			      const char *oneline)
>  {
> ...
>  	item = string_list_insert(&log->list, namemailbuf.buf);
> +	strbuf_release(&namemailbuf);

As log->list.strdup_strings is set to true in shortlog_init(), namemailbuf.buf will leak without this.

An alterative, as this is the only place we add to log->list, could be to make log->list take ownership of the string by not adding a _release() here but instead _detach(), I guess.

It is also curious that at the end of shortlog_output(), we set the log->list.strdup_strings again to true immediately before calling string_list_clear() on it. I suspect that is unnecessary?