Re: [PATCH v3 5/5] upload-pack: make uploadpack.packObjectsHook protected
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jun 2, 2022, 13:18 UTC
- Message-ID
- <3873f70e-0dd6-e469-5d0e-0f6847136f0a@github.com>
- In-Reply-To
- <e25d5907cd1e3894f19ffbfb3310175fd660563b.1653685761.git.gitgitgadget@gmail.com>
On 5/27/2022 5:09 PM, Glen Choo via GitGitGadget wrote:
Show 5 quoted lines
> From: Glen Choo <chooglen@google.com> > > 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.
Show 10 quoted lines
> @@ -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);
> - }
> -...
Show 9 quoted lines
> +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.
Show 13 quoted lines
> @@ -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