[PATCH v2 00/21] builtin/config: remove global state

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 6777 bytes --]

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@pks.im>
     
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]