Re: [PATCH 5/7] init: remove git_init_db_config() while fixing leaks

From: Jeff King <peff@peff.net>
Date: Mar 8, 2021, 19:29 UTC
Message-ID: <YEZ7Bl2nCcY1gAtf@coredump.intra.peff.net>
In-Reply-To: <d30365d967651c6090635ec564d2ed2d051e8aec.1615228580.git.gitgitgadget@gmail.com>

On Mon, Mar 08, 2021 at 06:36:18PM +0000, Andrzej Hunt via GitGitGadget wrote:

Show 30 quoted lines

> @@ -212,10 +200,7 @@ static int create_default_files(const char *template_path,
>  	int reinit;
>  	int filemode;
>  	struct strbuf err = STRBUF_INIT;
> -
> -	/* Just look for `init.templatedir` */
> -	init_db_template_dir = NULL; /* re-set in case it was set before */
> -	git_config(git_init_db_config, NULL);
> +	const char *init_template_dir = NULL;
>  
>  	/*
>  	 * First copy the templates -- we might have the default
> @@ -226,7 +211,8 @@ static int create_default_files(const char *template_path,
>  	 * values (since we've just potentially changed what's available on
>  	 * disk).
>  	 */
> -	copy_templates(template_path);
> +	git_config_get_value("init.templatedir", &init_template_dir);
> +	copy_templates(template_path, init_template_dir);
>  	git_config_clear();
>  	reset_shared_repository();
>  	git_config(git_default_config, NULL);
> @@ -422,8 +408,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
>  	}
>  	startup_info->have_repository = 1;
>  
> -	/* Just look for `core.hidedotfiles` */
> -	git_config(git_init_db_config, NULL);
> +	/* Ensure `core.hidedotfiles` is processed */
> +	git_config(platform_core_config, NULL);

There are some subtle ordering dependencies in init_db(), because it may start in one git repository, but then create and shift into another.

It's hard to see the ordering just from the diff. I think this change is OK. The platform_core_config bits are loaded at the same moment, and it's only the extra git_config() call in create_default_files() that goes away. That _could_ be overwriting the platform bits with something else, but I think that was not the intent of the code. And I think it's impossible, because the intervening calls are not moving from one repo to the other.

-Peff