From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 08 Aug 2024 17:12:26 GMT
Subject: Re: [PATCH v2 08/22] config: fix leaking comment character config
Message-ID: <xmqq34nfhrb9.fsf@gitster.g>
In-Reply-To: <8fbd72a1002d1a285847c62b5524041927a7b4d4.1723121979.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> 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.