Re: [PATCH 3/3] diff: remove parseopts member of struct diff_options

From: "René Scharfe" <l.s.r@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Git List" <git@vger.kernel.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jeff King" <peff@peff.net>
Subject: Re: [PATCH 3/3] diff: remove parseopts member of struct diff_options
Date: Thu, 1 Dec 2022 08:52:37 +0100	[thread overview]
Message-ID: <20b5d6d1-ec09-e09f-4f92-5925fb9c96f8@web.de> (raw)
In-Reply-To: <xmqqcz93ud8o.fsf@gitster.g>

Am 01.12.22 um 02:25 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> repo_diff_setup() builds the struct option array with git diff's command
>> line options and stores a pointer to it in the parseopts member of
>> struct diff_options.  The array is freed by diff_setup_done(), but not
>> by release_revisions().  repo_init_revisions() calls repo_diff_setup(),
>> thus calling repo_init_revisions() then release_revisions() leaks it.
>>
>> We could free the array in release_revisions() as well to plug that
>> leak, but there is a better way: Only build it when needed.  Move the
>> get_diff_parseopts() calls to the two places that use the array, free it
>> after use and get rid of the parseopts member.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>>  diff.c | 17 ++++++++---------
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> I think this hunk is missing?

Yes. O_o

>
>  diff.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git c/diff.h w/diff.h
> index 5229f20486..6840499844 100644
> --- c/diff.h
> +++ w/diff.h
> @@ -394,7 +394,6 @@ struct diff_options {
>  	unsigned color_moved_ws_handling;
>
>  	struct repository *repo;
> -	struct option *parseopts;
>  	struct strmap *additional_path_headers;
>
>  	int no_free;