Re: [PATCH v2 04/27] format-patch: don't leak "extra_headers" or "ref_message_ids"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 24, 2022, 04:33 UTC
Message-ID: <xmqqfsn8m0la.fsf@gitster.g>
In-Reply-To: <patch-v2-04.27-64a76629e23-20220323T203149Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 5 quoted lines

> Fix two memory leaks in "struct rev_info" by freeing that memory in
> cmd_format_patch(). These two are unusual special-cases in being in
> the "struct rev_info", but not being "owned" by the code in
> revision.c. I.e. they're members of the struct so that this code in
> "builtin/log.c" can pass information code in log-tree.c.

Hmph, if we are eventually adding a new API function to clear a rev-info structure, shouldn't these members be released there instead, I wonder.

This is the only user of this member, so it does not matter too much either way, though.

Show 32 quoted lines

> diff --git a/builtin/log.c b/builtin/log.c
> index 634dc782cce..6f9928fabfe 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1747,6 +1747,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	struct commit *commit;
>  	struct commit **list = NULL;
>  	struct rev_info rev;
> +	char *to_free = NULL;
>  	struct setup_revision_opt s_r_opt;
>  	int nr = 0, total, i;
>  	int use_stdout = 0;
> @@ -1947,7 +1948,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		strbuf_addch(&buf, '\n');
>  	}
>  
> -	rev.extra_headers = strbuf_detach(&buf, NULL);
> +	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);
>  
>  	if (from) {
>  		if (split_ident_line(&rev.from_ident, from, strlen(from)))
> @@ -2284,6 +2285,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	strbuf_release(&rdiff1);
>  	strbuf_release(&rdiff2);
>  	strbuf_release(&rdiff_title);
> +	free(to_free);
> +	if (rev.ref_message_ids)
> +		string_list_clear(rev.ref_message_ids, 0);
> +	free(rev.ref_message_ids);
>  	UNLEAK(rev);
>  	return 0;
>  }