Re: [PATCH 01/20] mailinfo: fix leaking header data

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

Patrick Steinhardt <ps@pks.im> writes:

> We populate the `mailinfo` arrays `p_hdr_data` and `s_hdr_data` with
> data parsed from the mail headers. These arrays may end up being only
> partially populated with gaps in case some of the headers do not parse
> properly. This causes memory leaks because `strbuf_list_free()` will
> stop iterating once it hits the first `NULL` pointer in the backing
> array.
>
> Fix this by open-coding a variant of `strbuf_list_free()` that knows to
> iterate through all headers.

Well spotted.  An array of strbuf is often a wrong data structure
for anything, but luckily we use it and suffer from a bug like this
only rarely.

The fix makes sense.  Will queue.



>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  mailinfo.c          | 17 +++++++++++++++--
>  t/t5100-mailinfo.sh |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/mailinfo.c b/mailinfo.c
> index 94b9b0abf22..a4fa64994ac 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -1290,8 +1290,21 @@ void clear_mailinfo(struct mailinfo *mi)
>  	strbuf_release(&mi->inbody_header_accum);
>  	free(mi->message_id);
>  
> -	strbuf_list_free(mi->p_hdr_data);
> -	strbuf_list_free(mi->s_hdr_data);
> +	for (size_t i = 0; header[i]; i++) {
> +		if (!mi->p_hdr_data[i])
> +			continue;
> +		strbuf_release(mi->p_hdr_data[i]);
> +		free(mi->p_hdr_data[i]);
> +	}
> +	free(mi->p_hdr_data);
> +
> +	for (size_t i = 0; header[i]; i++) {
> +		if (!mi->s_hdr_data[i])
> +			continue;
> +		strbuf_release(mi->s_hdr_data[i]);
> +		free(mi->s_hdr_data[i]);
> +	}
> +	free(mi->s_hdr_data);
>  
>  	while (mi->content < mi->content_top) {
>  		free(*(mi->content_top));
> diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
> index c8d06554541..065156c1f39 100755
> --- a/t/t5100-mailinfo.sh
> +++ b/t/t5100-mailinfo.sh
> @@ -5,6 +5,7 @@
>  
>  test_description='git mailinfo and git mailsplit test'
>  
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  
>  DATA="$TEST_DIRECTORY/t5100"