{"thread":{"id":"56185","subject":"[PATCH v1 0/6] globals: clean up some global usages","startedAt":"2021-07-31T02:42:32Z","lastAt":"2021-07-31T02:42:42Z","messageCount":7,"participants":["Ben Boeckel"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"431610","messageId":"20210731024221.2113906-1-mathstuf@gmail.com","threadId":"56185","inReplyTo":null,"subject":"[PATCH v1 0/6] globals: clean up some global usages","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:15Z","receivedAt":"2021-07-31T02:42:32Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"Hi,\n\nThis patchset moves a few of the configuration parsing functions around\nso that they live in the source file which uses the variables that are\ninitialized through those functions. This allows the variables involved\nto be `static` rather than global variables.\n\nAdditionally, a global which is only written to is removed\n(`core_compression_level`), a file-local variable declared `extern` is\nnow `static`, and explicit `0` initialization of globals is removed.\n\nIn the course of making this patch, I found a number of other global\nvariables which could be moved into being static if more configuration\nparsing functions were created and moved to the relevant file instead of\nbeing parsed as part of, say, \"all `core.*` settings\" in a single\nfunction. If wanted, I can prepare a patchset for these as well (or\nprovide the list for discussion of how best to proceed, but it is\ncurrently just stored in a WIP commit I have locally as comments on each\nvariable).\n\nThanks,\n\n--Ben\n\n---\n\nBen Boeckel (6):\n  branch: move `git_default_branch_config` to `branch.c`\n  mailmap: move `git_default_mailmap_config` to `mailmap.c`\n  apply: move `apply_default_*whitespace` to `apply.c`\n  config: remove `core_compression_level`\n  refs/debug: declare file-local variable to be static\n  globals: remove explicit `0` initialization from globals\n\n apply.c       |  3 +++\n branch.c      | 32 ++++++++++++++++++++++++++++++++\n branch.h      |  5 +++++\n cache.h       |  6 ------\n config.c      | 43 +------------------------------------------\n environment.c |  4 ----\n mailmap.c     | 16 ++++++++++++++--\n mailmap.h     |  2 ++\n object-file.c |  2 +-\n progress.c    |  2 +-\n refs/debug.c  |  4 ++--\n 11 files changed, 61 insertions(+), 58 deletions(-)\n\n\nbase-commit: eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687\n-- \n2.31.1\n\n"},{"id":"431611","messageId":"20210731024221.2113906-2-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 1/6] branch: move `git_default_branch_config` to `branch.c`","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:16Z","receivedAt":"2021-07-31T02:42:36Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"This allows branch-specific variables to be tracked locally in\n`branch.c` instead of globally in `environment.c`.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n branch.c      | 32 ++++++++++++++++++++++++++++++++\n branch.h      |  5 +++++\n cache.h       |  1 -\n config.c      | 30 ------------------------------\n environment.c |  1 -\n 5 files changed, 37 insertions(+), 32 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 7a88a4861e..1ef52db89d 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -9,6 +9,38 @@\n #include \"commit.h\"\n #include \"worktree.h\"\n \n+static enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n+\n+int git_default_branch_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"branch.autosetupmerge\")) {\n+\t\tif (value && !strcasecmp(value, \"always\")) {\n+\t\t\tgit_branch_track = BRANCH_TRACK_ALWAYS;\n+\t\t\treturn 0;\n+\t\t}\n+\t\tgit_branch_track = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"branch.autosetuprebase\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\telse if (!strcmp(value, \"never\"))\n+\t\t\tautorebase = AUTOREBASE_NEVER;\n+\t\telse if (!strcmp(value, \"local\"))\n+\t\t\tautorebase = AUTOREBASE_LOCAL;\n+\t\telse if (!strcmp(value, \"remote\"))\n+\t\t\tautorebase = AUTOREBASE_REMOTE;\n+\t\telse if (!strcmp(value, \"always\"))\n+\t\t\tautorebase = AUTOREBASE_ALWAYS;\n+\t\telse\n+\t\t\treturn error(_(\"malformed value for %s\"), var);\n+\t\treturn 0;\n+\t}\n+\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n+\n struct tracking {\n \tstruct refspec_item spec;\n \tchar *src;\ndiff --git a/branch.h b/branch.h\nindex df0be61506..801ff25f92 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -17,6 +17,11 @@ extern enum branch_track git_branch_track;\n \n /* Functions for acting on the information about branches. */\n \n+/*\n+ * Parses branch-related configuration options.\n+ */\n+int git_default_branch_config(const char *var, const char *value);\n+\n /*\n  * Creates a new branch, where:\n  *\ndiff --git a/cache.h b/cache.h\nindex ba04ff8bd3..6ea1ea5854 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1030,7 +1030,6 @@ enum push_default_type {\n \tPUSH_DEFAULT_UNSPECIFIED\n };\n \n-extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n \n enum object_creation_mode {\ndiff --git a/config.c b/config.c\nindex f33abeab85..840be51710 100644\n--- a/config.c\n+++ b/config.c\n@@ -1574,36 +1574,6 @@ static int git_default_i18n_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_branch_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"branch.autosetupmerge\")) {\n-\t\tif (value && !strcasecmp(value, \"always\")) {\n-\t\t\tgit_branch_track = BRANCH_TRACK_ALWAYS;\n-\t\t\treturn 0;\n-\t\t}\n-\t\tgit_branch_track = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(var, \"branch.autosetuprebase\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\telse if (!strcmp(value, \"never\"))\n-\t\t\tautorebase = AUTOREBASE_NEVER;\n-\t\telse if (!strcmp(value, \"local\"))\n-\t\t\tautorebase = AUTOREBASE_LOCAL;\n-\t\telse if (!strcmp(value, \"remote\"))\n-\t\t\tautorebase = AUTOREBASE_REMOTE;\n-\t\telse if (!strcmp(value, \"always\"))\n-\t\t\tautorebase = AUTOREBASE_ALWAYS;\n-\t\telse\n-\t\t\treturn error(_(\"malformed value for %s\"), var);\n-\t\treturn 0;\n-\t}\n-\n-\t/* Add other config variables here and to Documentation/config.txt. */\n-\treturn 0;\n-}\n-\n static int git_default_push_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"push.default\")) {\ndiff --git a/environment.c b/environment.c\nindex 2f27008424..5d45152731 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -60,7 +60,6 @@ int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;\n char *check_roundtrip_encoding = \"SHIFT-JIS\";\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n #ifndef OBJECT_CREATION_MODE\n #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n-- \n2.31.1\n\n"},{"id":"431612","messageId":"20210731024221.2113906-4-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 3/6] apply: move `apply_default_*whitespace` to `apply.c`","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:18Z","receivedAt":"2021-07-31T02:42:42Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"These variables are only set and used in this file, so they can become\n`static`.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n apply.c       | 3 +++\n cache.h       | 2 --\n environment.c | 2 --\n 3 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 44bc31d6eb..15dcd8b7d7 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -23,6 +23,9 @@\n #include \"apply.h\"\n #include \"entry.h\"\n \n+static char *apply_default_whitespace;\n+static char *apply_default_ignorewhitespace;\n+\n struct gitdiff_data {\n \tstruct strbuf *root;\n \tint linenr;\ndiff --git a/cache.h b/cache.h\nindex 551a6cb5cf..dc6c4172cb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -944,8 +944,6 @@ extern int assume_unchanged;\n extern int prefer_symlink_refs;\n extern int warn_ambiguous_refs;\n extern int warn_on_object_refname_ambiguity;\n-extern char *apply_default_whitespace;\n-extern char *apply_default_ignorewhitespace;\n extern const char *git_attributes_file;\n extern const char *git_hooks_path;\n extern int zlib_compression_level;\ndiff --git a/environment.c b/environment.c\nindex 5d45152731..0cc086d847 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -36,8 +36,6 @@ int repository_format_precious_objects;\n int repository_format_worktree_config;\n const char *git_commit_encoding;\n const char *git_log_output_encoding;\n-char *apply_default_whitespace;\n-char *apply_default_ignorewhitespace;\n const char *git_attributes_file;\n const char *git_hooks_path;\n int zlib_compression_level = Z_BEST_SPEED;\n-- \n2.31.1\n\n"},{"id":"431613","messageId":"20210731024221.2113906-3-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 2/6] mailmap: move `git_default_mailmap_config` to `mailmap.c`","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:17Z","receivedAt":"2021-07-31T02:42:42Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"This allows mailmap-specific variables to be tracked locally in\n`mailmap.c` instead of globally.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n cache.h   |  2 --\n config.c  | 12 +-----------\n mailmap.c | 16 ++++++++++++++--\n mailmap.h |  2 ++\n 4 files changed, 17 insertions(+), 15 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 6ea1ea5854..551a6cb5cf 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1718,8 +1718,6 @@ int author_ident_sufficiently_given(void);\n \n extern const char *git_commit_encoding;\n extern const char *git_log_output_encoding;\n-extern const char *git_mailmap_file;\n-extern const char *git_mailmap_blob;\n \n /* IO helper functions */\n void maybe_flush_or_die(FILE *, const char *);\ndiff --git a/config.c b/config.c\nindex 840be51710..c45001adbe 100644\n--- a/config.c\n+++ b/config.c\n@@ -15,6 +15,7 @@\n #include \"strbuf.h\"\n #include \"quote.h\"\n #include \"hashmap.h\"\n+#include \"mailmap.h\"\n #include \"string-list.h\"\n #include \"object-store.h\"\n #include \"utf8.h\"\n@@ -1603,17 +1604,6 @@ static int git_default_push_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_mailmap_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"mailmap.file\"))\n-\t\treturn git_config_pathname(&git_mailmap_file, var, value);\n-\tif (!strcmp(var, \"mailmap.blob\"))\n-\t\treturn git_config_string(&git_mailmap_blob, var, value);\n-\n-\t/* Add other config variables here and to Documentation/config.txt. */\n-\treturn 0;\n-}\n-\n int git_default_config(const char *var, const char *value, void *cb)\n {\n \tif (starts_with(var, \"core.\"))\ndiff --git a/mailmap.c b/mailmap.c\nindex d1f7c0d272..e1d9a2bbdb 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"config.h\"\n #include \"string-list.h\"\n #include \"mailmap.h\"\n #include \"object-store.h\"\n@@ -12,8 +13,19 @@ static inline void debug_mm(const char *format, ...) {}\n static inline const char *debug_str(const char *s) { return s; }\n #endif\n \n-const char *git_mailmap_file;\n-const char *git_mailmap_blob;\n+static const char *git_mailmap_file;\n+static const char *git_mailmap_blob;\n+\n+int git_default_mailmap_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"mailmap.file\"))\n+\t\treturn git_config_pathname(&git_mailmap_file, var, value);\n+\tif (!strcmp(var, \"mailmap.blob\"))\n+\t\treturn git_config_string(&git_mailmap_blob, var, value);\n+\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n \n struct mailmap_info {\n \tchar *name;\ndiff --git a/mailmap.h b/mailmap.h\nindex 7e99fccb46..48bfe9fac7 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -3,6 +3,8 @@\n \n struct string_list;\n \n+int git_default_mailmap_config(const char *var, const char *value);\n+\n int read_mailmap(struct string_list *map);\n void clear_mailmap(struct string_list *map);\n \n-- \n2.31.1\n\n"},{"id":"431614","messageId":"20210731024221.2113906-5-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 4/6] config: remove `core_compression_level`","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:19Z","receivedAt":"2021-07-31T02:42:42Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"This variable is just stored to after parsing `core.compression` and\nthen ignored, so it can just be completely removed.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n cache.h       | 1 -\n config.c      | 1 -\n environment.c | 1 -\n 3 files changed, 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex dc6c4172cb..4fd134863c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -947,7 +947,6 @@ extern int warn_on_object_refname_ambiguity;\n extern const char *git_attributes_file;\n extern const char *git_hooks_path;\n extern int zlib_compression_level;\n-extern int core_compression_level;\n extern int pack_compression_level;\n extern size_t packed_git_window_size;\n extern size_t packed_git_limit;\ndiff --git a/config.c b/config.c\nindex c45001adbe..b241a56cfb 100644\n--- a/config.c\n+++ b/config.c\n@@ -1401,7 +1401,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\tlevel = Z_DEFAULT_COMPRESSION;\n \t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n \t\t\tdie(_(\"bad zlib compression level %d\"), level);\n-\t\tcore_compression_level = level;\n \t\tcore_compression_seen = 1;\n \t\tif (!zlib_compression_seen)\n \t\t\tzlib_compression_level = level;\ndiff --git a/environment.c b/environment.c\nindex 0cc086d847..3b995434fc 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -39,7 +39,6 @@ const char *git_log_output_encoding;\n const char *git_attributes_file;\n const char *git_hooks_path;\n int zlib_compression_level = Z_BEST_SPEED;\n-int core_compression_level;\n int pack_compression_level = Z_DEFAULT_COMPRESSION;\n int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n-- \n2.31.1\n\n"},{"id":"431615","messageId":"20210731024221.2113906-6-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 5/6] refs/debug: declare file-local variable to be static","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:20Z","receivedAt":"2021-07-31T02:42:42Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"Declaring a variable in a source file as `extern` and then defining it\nlater is not very useful. It can instead be declared as `static`.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n refs/debug.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 7db4abccc3..f5609408f3 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -9,7 +9,7 @@ struct debug_ref_store {\n \tstruct ref_store *refs;\n };\n \n-extern struct ref_storage_be refs_be_debug;\n+static struct ref_storage_be refs_be_debug;\n \n struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_store *store)\n {\n@@ -413,7 +413,7 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,\n \treturn res;\n }\n \n-struct ref_storage_be refs_be_debug = {\n+static struct ref_storage_be refs_be_debug = {\n \tNULL,\n \t\"debug\",\n \tNULL,\n-- \n2.31.1\n\n"},{"id":"431616","messageId":"20210731024221.2113906-7-mathstuf@gmail.com","threadId":"56185","inReplyTo":"20210731024221.2113906-1-mathstuf@gmail.com","subject":"[PATCH v1 6/6] globals: remove explicit `0` initialization from globals","fromName":"Ben Boeckel","fromEmail":"mathstuf@gmail.com","sentAt":"2021-07-31T02:42:21Z","receivedAt":"2021-07-31T02:42:42Z","isPatch":true,"sender":{"key":"mathstuf@gmail.com","avatar":"https://avatars.githubusercontent.com/u/97253?v=4"},"body":"Git relies on implicit zero-initialization of global variables.\n\nSigned-off-by: Ben Boeckel <mathstuf@gmail.com>\n---\n object-file.c | 2 +-\n progress.c    | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex ecca5a8da0..6a04013342 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1503,7 +1503,7 @@ static int loose_object_info(struct repository *r,\n \treturn (status < 0) ? status : 0;\n }\n \n-int obj_read_use_lock = 0;\n+int obj_read_use_lock;\n pthread_mutex_t obj_read_mutex;\n \n void enable_obj_read_lock(void)\ndiff --git a/progress.c b/progress.c\nindex 680c6a8bf9..4fb4233b67 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -52,7 +52,7 @@ static volatile sig_atomic_t progress_update;\n  * for 'test-tool progress'.\n  */\n int progress_testing;\n-uint64_t progress_test_ns = 0;\n+uint64_t progress_test_ns;\n void progress_test_force_update(void)\n {\n \tprogress_update = 1;\n-- \n2.31.1\n\n"}]}