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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Thu, 24 Oct 2019, Jeff King wrote:

> On Fri, Oct 25, 2019 at 01:08:16AM +0200, Johannes Schindelin wrote:
>
> > 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.

Makes sense, I changed it to `static char *system_wide;`.

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

Well, `normalize_path_copy()`'s documentation says:

	It is okay if dst == src, but they should not overlap otherwise.

But yes, that does not state explicitly that the resulting string won't
be longer than the original one. It is currently true, though the
documentation hints at the possibility that a future version might
follow symbolic links (in which case it would no longer be true). But
then, that would technically not be normalization anymore, but
canonicalization.

Ciao,
Dscho