Re: [PATCH 2/2] config: respect includes in protected config

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2022, 21:07 UTC
Message-ID: <xmqqilkokaxj.fsf@gitster.g>
In-Reply-To: <0ff5b5741a519c63e65ef57d7d0b3148c38c1a52.1665603814.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 35 quoted lines

> -int git_configset_add_parameters(struct config_set *cs)
> -{
> -	return git_config_from_parameters(config_set_callback, cs);
> -}
> -
>  int git_configset_get_value(struct config_set *cs, const char *key, const char **value)
>  {
>  	const struct string_list *values = NULL;
> @@ -2641,24 +2636,15 @@ int repo_config_get_pathname(struct repository *repo,
>  /* Read values into protected_config. */
>  static void read_protected_config(void)
>  {
> -	char *xdg_config = NULL, *user_config = NULL, *system_config = NULL;
> -
> +	struct config_options opts = {
> +		.respect_includes = 1,
> +		.ignore_repo = 1,
> +		.ignore_worktree = 1,
> +		.system_gently = 1,
> +	};
>  	git_configset_init(&protected_config);
> -
> -	system_config = git_system_config();
> -	git_global_config(&user_config, &xdg_config);
> -
> -	if (system_config)
> -		git_configset_add_file(&protected_config, system_config);
> -	if (xdg_config)
> -		git_configset_add_file(&protected_config, xdg_config);
> -	if (user_config)
> -		git_configset_add_file(&protected_config, user_config);
> -	git_configset_add_parameters(&protected_config);
> -	free(system_config);
> -	free(xdg_config);
> -	free(user_config);

Compared to the above hand-crafted sequence, we seem to be a lot more careful in config.c::do_git_config_sequence() with respect to error checking, which is good. We can lose the custom helper to read from command line parameters, which is also nice.

> +	config_with_options(config_set_callback, &protected_config,
> +			    NULL, &opts);
>  }

Very nice, code reduction with an additional feature. I wish all fixes were like this.

>  config.c                        | 30 ++++++++----------------------
>  t/t0033-safe-directory.sh       |  2 +-
>  t/t0035-safe-bare-repository.sh |  2 +-
>  3 files changed, 10 insertions(+), 24 deletions(-)