Hi, we have quite a lot of global state in git-config(1). For one, this global state is used to track options passed by the user. And second, there is a lot of global state that is really only used to pass data between caller and callbacks. This global state makes it quite hard to understand interactions at times. This only became harder now with the introduction of subcommands, where a whole lot less of the global state is even relevant in the first place. As an example, this global state has made it quite easy for a bug to sneak into the new subcommands where `check_write()` was called before the global data it access is initialized (see patch 6). This patch series refactors the code such that we have no more global state in "builtin/config.c". It's rather long, but most of the patches are really quite trivial and only move code around. So overall, I think it shouldn't be too bad to review this. But please, let me know if you disagree and I'll happily split this series into two. The patch series depends on 7b91d310ce (builtin/config: display subcommand help, 2024-05-06). Thanks! Patrick Patrick Steinhardt (21): builtin/config: stop printing full usage on misuse builtin/config: move legacy mode into its own function builtin/config: move subcommand options into `cmd_config()` builtin/config: move legacy options into `cmd_config()` builtin/config: move actions into `cmd_config_actions()` builtin/config: check for writeability after source is set up config: make the config source const builtin/config: refactor functions to have common exit paths builtin/config: move location options into local variables builtin/config: move display options into local variables builtin/config: move type options into display options builtin/config: move default value into display options builtin/config: move `respect_includes_opt` into location options builtin/config: convert `do_not_match` to a local variable builtin/config: convert `value_pattern` to a local variable builtin/config: convert `regexp` to a local variable builtin/config: convert `key_regexp` to a local variable builtin/config: convert `key` to a local variable builtin/config: track "fixed value" option via flags only builtin/config: convert flags to a local variable builtin/config: pass data between callbacks via local variables builtin/config.c | 964 +++++++++++++++++++++++++--------------------- config.c | 4 +- config.h | 2 +- t/t1300-config.sh | 9 +- 4 files changed, 546 insertions(+), 433 deletions(-) -- 2.45.GIT
Attachment:
signature.asc
Description: PGP signature