Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

[Junio C Hamano <gitster@pobox.com>]

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

> Use string_list_append_nodup() instead of string_list_append() to hand
> over ownership of a detached strbuf and thus avoid leaking its memory.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  builtin/fmt-merge-msg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 22034f87e7..8e8a15ea4a 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -377,7 +377,8 @@ static void shortlog(const char *name,
>  			string_list_append(&subjects,
>  					   oid_to_hex(&commit->object.oid));
>  		else
> -			string_list_append(&subjects, strbuf_detach(&sb, NULL));
> +			string_list_append_nodup(&subjects,
> +						 strbuf_detach(&sb, NULL));
>  	}
>  
>  	if (opts->credit_people)

What is leaked comes from strbuf, so the title is not a lie, but I
tend to think that this leak is caused by a somewhat strange
string_list API.  The subjects string-list is initialized as a "dup"
kind, but a caller that wants to avoid leaking can (and should) use
_nodup() call to add a string without duping.  It all feels a bit
too convoluted.

The patch looks good.