Subject: Re: [PATCH v3 5/5] upload-pack: make uploadpack.packObjectsHook protected

From: Derrick Stolee <[email-masked]>
Date: Thu, 2 Jun 2022 09:18:16 -0400

On 5/27/2022 5:09 PM, Glen Choo via GitGitGadget wrote:
> From: Glen Choo <[email-masked]>
> 
> Now that protected config includes "-c", "uploadpack.packObjectsHook"
> behaves identically to a 'Protected config only' variable. Refactor it
> to use git_protected_config() and mark it 'Protected config only'.

I'm really glad to see this simplification at the end of your series.

> @@ -1321,18 +1321,21 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)
>  		data->advertise_sid = git_config_bool(var, value);
>  	}
>  
> -	if (current_config_scope() != CONFIG_SCOPE_LOCAL &&
> -	    current_config_scope() != CONFIG_SCOPE_WORKTREE) {
> -		if (!strcmp("uploadpack.packobjectshook", var))
> -			return git_config_string(&data->pack_objects_hook, var, value);
> -	}
> -

...

> +static int upload_pack_protected_config(const char *var, const char *value, void *cb_data)
> +{
> +	struct upload_pack_data *data = cb_data;
> +
> +	if (!strcmp("uploadpack.packobjectshook", var))
> +		return git_config_string(&data->pack_objects_hook, var, value);
> +	return 0;
> +}
> +

This is much cleaner.

> @@ -1342,6 +1345,7 @@ void upload_pack(const int advertise_refs, const int stateless_rpc,
>  	upload_pack_data_init(&data);
>  
>  	git_config(upload_pack_config, &data);
> +	git_protected_config(upload_pack_protected_config, &data);
>  
>  	data.stateless_rpc = stateless_rpc;
>  	data.timeout = timeout;
> @@ -1697,6 +1701,7 @@ int upload_pack_v2(struct repository *r, struct packet_reader *request)
>  	data.use_sideband = LARGE_PACKET_MAX;
>  
>  	git_config(upload_pack_config, &data);
> +	git_protected_config(upload_pack_protected_config, &data);

It's unfortunate that there are two places that need this change.
Is it worth adding a static helper that executes these?

static void get_upload_pack_config(void *data)
{
	git_config(upload_pack_config, data);
	git_protected_config(upload_pack_protected_config, data);
}

Thanks,
-Stolee