Hi,
this is the second version of my patch series that removes global state
from "builtin/config.c". Changes compared to v1:
- Reinstated a comment in patch 5.
- Fixed a memory leak in patch 9.
- A couple of commit message fixes.
The series continues to build on top of ps/config-subcommands.
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 | 968 ++++++++++++++++++++++++++--------------------
config.c | 4 +-
config.h | 2 +-
t/t1300-config.sh | 9 +-
4 files changed, 550 insertions(+), 433 deletions(-)
Range-diff against v1:
1: 0ba7628126 = 1: 0ba7628126 builtin/config: stop printing full usage on misuse
2: 663e1f74f8 = 2: 663e1f74f8 builtin/config: move legacy mode into its own function
3: 1239c151d0 = 3: 1239c151d0 builtin/config: move subcommand options into `cmd_config()`
4: 82964510c5 = 4: 82964510c5 builtin/config: move legacy options into `cmd_config()`
5: 2e308393ed ! 5: 0a6ecae2cc builtin/config: move actions into `cmd_config_actions()`
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
comment = git_config_prepare_comment_string(comment_arg);
- if (actions & PAGING_ACTIONS)
++ /*
++ * The following actions may produce more than one line of output and
++ * should therefore be paged.
++ */
+ if (actions & (ACTION_LIST | ACTION_GET_ALL | ACTION_GET_REGEXP | ACTION_GET_URLMATCH))
setup_auto_pager("config", 1);
6: edfd7caa39 = 6: 7ab99a27c1 builtin/config: check for writeability after source is set up
7: bfba68aa1e = 7: 1460d3a36c config: make the config source const
8: 6bff0410e9 = 8: 018ed0226b builtin/config: refactor functions to have common exit paths
9: a96c122280 ! 9: b5d43b6f85 builtin/config: move location options into local variables
@@ builtin/config.c: static char *default_user_config(void)
- given_config_source.file = git_global_config();
- if (!given_config_source.file)
+ if (opts->use_global_config) {
-+ opts->source.file = xstrdup_or_null(git_global_config());
++ opts->source.file = git_global_config();
+ if (!opts->source.file)
/*
* It is unknown if HOME/.gitconfig exists, so
10: 06c1e08fc4 = 10: d66e14af30 builtin/config: move display options into local variables
11: 9610897662 = 11: 63436c3416 builtin/config: move type options into display options
12: eb79462861 = 12: 106b8ac8a2 builtin/config: move default value into display options
13: b9ebfbd667 = 13: 8a6b555b58 builtin/config: move `respect_includes_opt` into location options
14: 2b40b784fe ! 14: 0dd22bf51a builtin/config: convert `do_not_match` to a local variable
@@ Commit message
builtin/config: convert `do_not_match` to a local variable
The `do_not_match` variable is used by the `format_config()` callback as
- an indicator whteher or not the passed regular expression is negated. It
+ an indicator whether or not the passed regular expression is negated. It
is only ever set up by its only caller, `collect_config()` and can thus
easily be moved into the `collect_config_data` structure.
15: 71d1b7a51b = 15: b656951f0c builtin/config: convert `value_pattern` to a local variable
16: c3b340f119 = 16: b56a07bda0 builtin/config: convert `regexp` to a local variable
17: 835cc0acfb = 17: 323cb05120 builtin/config: convert `key_regexp` to a local variable
18: 2aee7ec5d8 ! 18: e972e63be8 builtin/config: convert `key` to a local variable
@@ Commit message
The `key` variable is used by the `get_value()` function for two
purposes:
- - It is used to store the result of `git_config_parse_key()`, which si
+ - It is used to store the result of `git_config_parse_key()`, which is
then passed on to `collect_config()`.
- It is used as a store to convert the provided key to an
all-lowercase key when `use_key_regexp` is set.
- Both of these cases don't warrant a global variable at all. In the
- former case we can pass the key via `struct collect_config_data`. And in
- the latter case we really only want to have it as a temporary local
- variable such that we can free associated memory.
+ Neither of these cases warrant a global variable at all. In the former
+ case we can pass the key via `struct collect_config_data`. And in the
+ latter case we really only want to have it as a temporary local variable
+ such that we can free associated memory.
Refactor the code accordingly to reduce our reliance on global state.
19: 625216a774 ! 19: d83c3d085e builtin/config: track "fixed value" option via flags only
@@ Commit message
is not aware that this is tracked via two separate mechanisms.
Refactor the code to use the flag exclusively. We already pass it to all
- the require callsites anyway, except for `collect_config()`.
+ the required callsites anyway, except for `collect_config()`.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
20: 05254d512b = 20: 294bcd96a4 builtin/config: convert flags to a local variable
21: 3a5f059789 = 21: 0496b958e2 builtin/config: pass data between callbacks via local variables
--
2.45.GIT
Attachment:
signature.asc
Description: PGP signature
