Re: [git-for-windows] Git for Windows v2.24.0-rc0, was Re: [ANNOUNCE] Git v2.24.0-rc0

From: Jeff King <peff@peff.net>
Date: Oct 24, 2019, 23:34 UTC
Message-ID: <20191024233432.GB32602@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1910250052350.46@tvgsbejvaqbjf.bet>

On Fri, Oct 25, 2019 at 01:08:16AM +0200, Johannes Schindelin wrote:

Show 14 quoted lines

> diff --git a/config.c b/config.c
> index e7052b39773..8e2f4748c49 100644
> --- a/config.c
> +++ b/config.c
> @@ -1658,8 +1658,10 @@ static int git_config_from_blob_ref(config_fn_t fn,
>  const char *git_etc_gitconfig(void)
>  {
>  	static const char *system_wide;
> -	if (!system_wide)
> +	if (!system_wide) {
>  		system_wide = system_path(ETC_GITCONFIG);
> +		normalize_path_copy((char *)system_wide, system_wide);
> +	}
>  	return system_wide;

This cast made me wonder why it was OK to write to system_wide. The answer is that system_path() hands ownership of the memory to us, since 59362e560d (system_path(): always return free'able memory to the caller, 2014-11-24). So I think the better solution than the cast is to drop the "const" from its declaration to better indicate our ownership within the function.

I also wondered how we know that system_wide is a large enough buffer, but I guess normalizing always makes things smaller. It would be nice if normalize_path_copy() said so in its docstring, but that is certainly not new to your patch. :)

-Peff