From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Kyle Lippincott <spectral@google.com>,
Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v3 00/21] builtin/config: remove global state
Date: Wed, 15 May 2024 08:41:33 +0200 [thread overview]
Message-ID: <cover.1715755055.git.ps@pks.im> (raw)
In-Reply-To: <cover.1715339393.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 8139 bytes --]
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-05-15 7:59 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 11:24 [PATCH 00/21] builtin/config: remove global state Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 01/21] builtin/config: stop printing full usage on misuse Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 02/21] builtin/config: move legacy mode into its own function Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 03/21] builtin/config: move subcommand options into `cmd_config()` Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 04/21] builtin/config: move legacy " Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 05/21] builtin/config: move actions into `cmd_config_actions()` Patrick Steinhardt
2024-05-10 20:42 ` Kyle Lippincott
2024-05-13 10:20 ` Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 06/21] builtin/config: check for writeability after source is set up Patrick Steinhardt
2024-05-10 20:46 ` Kyle Lippincott
2024-05-13 10:21 ` Patrick Steinhardt
2024-05-10 11:24 ` [PATCH 07/21] config: make the config source const Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 08/21] builtin/config: refactor functions to have common exit paths Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 09/21] builtin/config: move location options into local variables Patrick Steinhardt
2024-05-10 11:34 ` Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 10/21] builtin/config: move display " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 11/21] builtin/config: move type options into display options Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 12/21] builtin/config: move default value " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 13/21] builtin/config: move `respect_includes_opt` into location options Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 14/21] builtin/config: convert `do_not_match` to a local variable Patrick Steinhardt
2024-05-11 16:42 ` Eric Sunshine
2024-05-10 11:25 ` [PATCH 15/21] builtin/config: convert `value_pattern` " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 16/21] builtin/config: convert `regexp` " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 17/21] builtin/config: convert `key_regexp` " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 18/21] builtin/config: convert `key` " Patrick Steinhardt
2024-05-10 11:25 ` [PATCH 19/21] builtin/config: track "fixed value" option via flags only Patrick Steinhardt
2024-05-11 16:52 ` Eric Sunshine
2024-05-10 11:26 ` [PATCH 20/21] builtin/config: convert flags to a local variable Patrick Steinhardt
2024-05-10 11:26 ` [PATCH 21/21] builtin/config: pass data between callbacks via local variables Patrick Steinhardt
2024-05-10 17:40 ` [PATCH 00/21] builtin/config: remove global state Junio C Hamano
2024-05-10 23:08 ` Kyle Lippincott
2024-05-13 10:21 ` [PATCH v2 " Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 01/21] builtin/config: stop printing full usage on misuse Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 02/21] builtin/config: move legacy mode into its own function Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 03/21] builtin/config: move subcommand options into `cmd_config()` Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 04/21] builtin/config: move legacy " Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 05/21] builtin/config: move actions into `cmd_config_actions()` Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 06/21] builtin/config: check for writeability after source is set up Patrick Steinhardt
2024-05-14 21:45 ` Taylor Blau
2024-05-15 5:58 ` Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 07/21] config: make the config source const Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 08/21] builtin/config: refactor functions to have common exit paths Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 09/21] builtin/config: move location options into local variables Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 10/21] builtin/config: move display " Patrick Steinhardt
2024-05-13 10:22 ` [PATCH v2 11/21] builtin/config: move type options into display options Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 12/21] builtin/config: move default value " Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 13/21] builtin/config: move `respect_includes_opt` into location options Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 14/21] builtin/config: convert `do_not_match` to a local variable Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 15/21] builtin/config: convert `value_pattern` " Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 16/21] builtin/config: convert `regexp` " Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 17/21] builtin/config: convert `key_regexp` " Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 18/21] builtin/config: convert `key` " Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 19/21] builtin/config: track "fixed value" option via flags only Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 20/21] builtin/config: convert flags to a local variable Patrick Steinhardt
2024-05-13 10:23 ` [PATCH v2 21/21] builtin/config: pass data between callbacks via local variables Patrick Steinhardt
2024-05-13 21:57 ` [PATCH v2 00/21] builtin/config: remove global state Kyle Lippincott
2024-05-14 14:48 ` Junio C Hamano
2024-05-14 14:52 ` Patrick Steinhardt
2024-05-15 5:53 ` Patrick Steinhardt
2024-05-15 6:41 ` Patrick Steinhardt [this message]
2024-05-15 6:41 ` [PATCH v3 01/21] builtin/config: stop printing full usage on misuse Patrick Steinhardt
2024-05-15 6:41 ` [PATCH v3 02/21] builtin/config: move legacy mode into its own function Patrick Steinhardt
2024-05-15 6:41 ` [PATCH v3 03/21] builtin/config: move subcommand options into `cmd_config()` Patrick Steinhardt
2024-05-15 6:41 ` [PATCH v3 04/21] builtin/config: move legacy " Patrick Steinhardt
2024-05-15 6:41 ` [PATCH v3 05/21] builtin/config: move actions into `cmd_config_actions()` Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 06/21] builtin/config: check for writeability after source is set up Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 07/21] config: make the config source const Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 08/21] builtin/config: refactor functions to have common exit paths Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 09/21] builtin/config: move location options into local variables Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 10/21] builtin/config: move display " Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 11/21] builtin/config: move type options into display options Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 12/21] builtin/config: move default value " Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 13/21] builtin/config: move `respect_includes_opt` into location options Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 14/21] builtin/config: convert `do_not_match` to a local variable Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 15/21] builtin/config: convert `value_pattern` " Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 16/21] builtin/config: convert `regexp` " Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 17/21] builtin/config: convert `key_regexp` " Patrick Steinhardt
2024-05-15 6:42 ` [PATCH v3 18/21] builtin/config: convert `key` " Patrick Steinhardt
2024-05-15 6:43 ` [PATCH v3 19/21] builtin/config: track "fixed value" option via flags only Patrick Steinhardt
2024-05-15 6:43 ` [PATCH v3 20/21] builtin/config: convert flags to a local variable Patrick Steinhardt
2024-05-15 6:43 ` [PATCH v3 21/21] builtin/config: pass data between callbacks via local variables Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1715755055.git.ps@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=spectral@google.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).