Re: [PATCH v2 08/22] config: fix leaking comment character config

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2024, 17:12 UTC
Message-ID: <xmqq34nfhrb9.fsf@gitster.g>
In-Reply-To: <8fbd72a1002d1a285847c62b5524041927a7b4d4.1723121979.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 12 quoted lines

> diff --git a/config.c b/config.c
> index 6421894614..cb78b652ee 100644
> --- a/config.c
> +++ b/config.c
> @@ -1596,7 +1596,9 @@ static int git_default_core_config(const char *var, const char *value,
>  		else if (value[0]) {
>  			if (strchr(value, '\n'))
>  				return error(_("%s cannot contain newline"), var);
> -			comment_line_str = xstrdup(value);
> +			free(comment_line_str_allocated);
> +			comment_line_str = comment_line_str_allocated =
> +				xstrdup(value);

If you are to follow the _to_free pattern, you do not have to allocate here, no? We borrow the value in the configset and point at it via comment_line_str, and clear comment_line_str_to_free because there is nothing to free now. I.e.

			comment_line_str = value;
			FREE_AND_NULL(comment_line_str_allocated);

I still think the approach taken by the previous iteration was simpler and much less error prone, though.