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

From: Junio C Hamano <[email-masked]>
Date: Wed, 12 Oct 2022 14:07:52 -0700

"Glen Choo via GitGitGadget" <[email-masked]> writes:

> -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(-)