Hi, this is the third version of my patch series that removes global state from "builtin/config.c". There is only a single change compared to v2, namely another set of memory leak fixes for patch 9. I have double and triple checked now that this does address all the leaks, and all the CI leak jobs (well, all jobs in fact [1]) do pass now. Sorry for the noise and not doing this properly in v2! [1]: https://gitlab.com/gitlab-org/git/-/pipelines/1291267388 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 | 970 ++++++++++++++++++++++++++-------------------- config.c | 4 +- config.h | 2 +- t/t1300-config.sh | 9 +- 4 files changed, 552 insertions(+), 433 deletions(-) Range-diff against v2: 1: 0ba7628126 = 1: 32380a12fd builtin/config: stop printing full usage on misuse 2: 663e1f74f8 = 2: 8b07b280a9 builtin/config: move legacy mode into its own function 3: 1239c151d0 = 3: b1de0ff74d builtin/config: move subcommand options into `cmd_config()` 4: 82964510c5 = 4: 1a0c6a8384 builtin/config: move legacy options into `cmd_config()` 5: 0a6ecae2cc = 5: 4950ddcecd builtin/config: move actions into `cmd_config_actions()` 6: 7ab99a27c1 = 6: 6de9097fb2 builtin/config: check for writeability after source is set up 7: 1460d3a36c = 7: 24053f8f4f config: make the config source const 8: 018ed0226b = 8: 9dc1d00f71 builtin/config: refactor functions to have common exit paths 9: b5d43b6f85 ! 9: 2ede4a204b builtin/config: move location options into local variables @@ builtin/config.c: static const char *const builtin_config_edit_usage[] = { +struct config_location_options { + struct git_config_source source; + struct config_options options; ++ char *file_to_free; + int use_global_config; + int use_system_config; + int use_local_config; @@ builtin/config.c: static char *default_user_config(void) - use_worktree_config + - !!given_config_source.file + !!given_config_source.blob > 1) { + if (!opts->source.file) -+ opts->source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT)); -+ else -+ opts->source.file = xstrdup(opts->source.file); ++ opts->source.file = opts->file_to_free = ++ xstrdup_or_null(getenv(CONFIG_ENVIRONMENT)); + + if (opts->use_global_config + opts->use_system_config + + opts->use_local_config + opts->use_worktree_config + @@ 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 = git_global_config(); ++ opts->source.file = opts->file_to_free = git_global_config(); + if (!opts->source.file) /* * It is unknown if HOME/.gitconfig exists, so @@ builtin/config.c: static void handle_config_location(const char *prefix) - } else if (use_worktree_config) { + opts->source.scope = CONFIG_SCOPE_GLOBAL; + } else if (opts->use_system_config) { -+ opts->source.file = xstrdup_or_null(git_system_config()); ++ opts->source.file = opts->file_to_free = git_system_config(); + opts->source.scope = CONFIG_SCOPE_SYSTEM; + } else if (opts->use_local_config) { -+ opts->source.file = xstrdup_or_null(git_pathdup("config")); ++ opts->source.file = opts->file_to_free = git_pathdup("config"); + opts->source.scope = CONFIG_SCOPE_LOCAL; + } else if (opts->use_worktree_config) { struct worktree **worktrees = get_worktrees(); if (the_repository->repository_format_worktree_config) - given_config_source.file = git_pathdup("config.worktree"); -+ opts->source.file = git_pathdup("config.worktree"); ++ opts->source.file = opts->file_to_free = ++ git_pathdup("config.worktree"); else if (worktrees[0] && worktrees[1]) die(_("--worktree cannot be used with multiple " "working trees unless the config\n" @@ builtin/config.c: static void handle_config_location(const char *prefix) else - given_config_source.file = git_pathdup("config"); - given_config_source.scope = CONFIG_SCOPE_LOCAL; -+ opts->source.file = git_pathdup("config"); ++ opts->source.file = opts->file_to_free = ++ git_pathdup("config"); + opts->source.scope = CONFIG_SCOPE_LOCAL; free_worktrees(worktrees); - } else if (given_config_source.file) { @@ builtin/config.c: static void handle_config_location(const char *prefix) - given_config_source.scope = CONFIG_SCOPE_COMMAND; + } else if (opts->source.file) { + if (!is_absolute_path(opts->source.file) && prefix) -+ opts->source.file = ++ opts->source.file = opts->file_to_free = + prefix_filename(prefix, opts->source.file); + opts->source.scope = CONFIG_SCOPE_COMMAND; + } else if (opts->source.blob) { @@ builtin/config.c: static void handle_config_location(const char *prefix) +static void location_options_release(struct config_location_options *opts) +{ -+ free((char *) opts->source.file); ++ free(opts->file_to_free); +} + static void handle_nul(void) { 10: d66e14af30 ! 10: 4d157942e6 builtin/config: move display options into local variables @@ builtin/config.c: static int get_urlmatch(const struct config_location_options * &matched->kvi); fwrite(buf.buf, 1, buf.len, stdout); @@ builtin/config.c: static void location_options_release(struct config_location_options *opts) - free((char *) opts->source.file); + free(opts->file_to_free); } -static void handle_nul(void) { 11: 63436c3416 = 11: 5579371ad1 builtin/config: move type options into display options 12: 106b8ac8a2 = 12: 05a072d5d1 builtin/config: move default value into display options 13: 8a6b555b58 = 13: 15d45ef7d4 builtin/config: move `respect_includes_opt` into location options 14: 0dd22bf51a = 14: a729286cc5 builtin/config: convert `do_not_match` to a local variable 15: b656951f0c = 15: 821bc68212 builtin/config: convert `value_pattern` to a local variable 16: b56a07bda0 = 16: bac242caf0 builtin/config: convert `regexp` to a local variable 17: 323cb05120 = 17: 746bdf8733 builtin/config: convert `key_regexp` to a local variable 18: e972e63be8 = 18: f1f390f499 builtin/config: convert `key` to a local variable 19: d83c3d085e = 19: e4dbb4707e builtin/config: track "fixed value" option via flags only 20: 294bcd96a4 = 20: abe33015b7 builtin/config: convert flags to a local variable 21: 0496b958e2 = 21: a5cb075fcd builtin/config: pass data between callbacks via local variables -- 2.45.GIT
Attachment:
signature.asc
Description: PGP signature
