[PATCH v1 0/6] globals: clean up some global usages

[PATCH v1 0/6] globals: clean up some global usages

[Ben Boeckel]

Hi,

This patchset moves a few of the configuration parsing functions around
so that they live in the source file which uses the variables that are
initialized through those functions. This allows the variables involved
to be `static` rather than global variables.

Additionally, a global which is only written to is removed
(`core_compression_level`), a file-local variable declared `extern` is
now `static`, and explicit `0` initialization of globals is removed.

In the course of making this patch, I found a number of other global
variables which could be moved into being static if more configuration
parsing functions were created and moved to the relevant file instead of
being parsed as part of, say, "all `core.*` settings" in a single
function. If wanted, I can prepare a patchset for these as well (or
provide the list for discussion of how best to proceed, but it is
currently just stored in a WIP commit I have locally as comments on each
variable).

Thanks,

--Ben

---

Ben Boeckel (6):
  branch: move `git_default_branch_config` to `branch.c`
  mailmap: move `git_default_mailmap_config` to `mailmap.c`
  apply: move `apply_default_*whitespace` to `apply.c`
  config: remove `core_compression_level`
  refs/debug: declare file-local variable to be static
  globals: remove explicit `0` initialization from globals

 apply.c       |  3 +++
 branch.c      | 32 ++++++++++++++++++++++++++++++++
 branch.h      |  5 +++++
 cache.h       |  6 ------
 config.c      | 43 +------------------------------------------
 environment.c |  4 ----
 mailmap.c     | 16 ++++++++++++++--
 mailmap.h     |  2 ++
 object-file.c |  2 +-
 progress.c    |  2 +-
 refs/debug.c  |  4 ++--
 11 files changed, 61 insertions(+), 58 deletions(-)


base-commit: eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687
-- 
2.31.1

[PATCH v1 1/6] branch: move `git_default_branch_config` to `branch.c`

[Ben Boeckel]

This allows branch-specific variables to be tracked locally in
`branch.c` instead of globally in `environment.c`.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 branch.c      | 32 ++++++++++++++++++++++++++++++++
 branch.h      |  5 +++++
 cache.h       |  1 -
 config.c      | 30 ------------------------------
 environment.c |  1 -
 5 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/branch.c b/branch.c
index 7a88a4861e..1ef52db89d 100644
--- a/branch.c
+++ b/branch.c
@@ -9,6 +9,38 @@
 #include "commit.h"
 #include "worktree.h"
 
+static enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+
+int git_default_branch_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "branch.autosetupmerge")) {
+		if (value && !strcasecmp(value, "always")) {
+			git_branch_track = BRANCH_TRACK_ALWAYS;
+			return 0;
+		}
+		git_branch_track = git_config_bool(var, value);
+		return 0;
+	}
+	if (!strcmp(var, "branch.autosetuprebase")) {
+		if (!value)
+			return config_error_nonbool(var);
+		else if (!strcmp(value, "never"))
+			autorebase = AUTOREBASE_NEVER;
+		else if (!strcmp(value, "local"))
+			autorebase = AUTOREBASE_LOCAL;
+		else if (!strcmp(value, "remote"))
+			autorebase = AUTOREBASE_REMOTE;
+		else if (!strcmp(value, "always"))
+			autorebase = AUTOREBASE_ALWAYS;
+		else
+			return error(_("malformed value for %s"), var);
+		return 0;
+	}
+
+	/* Add other config variables here and to Documentation/config.txt. */
+	return 0;
+}
+
 struct tracking {
 	struct refspec_item spec;
 	char *src;
diff --git a/branch.h b/branch.h
index df0be61506..801ff25f92 100644
--- a/branch.h
+++ b/branch.h
@@ -17,6 +17,11 @@ extern enum branch_track git_branch_track;
 
 /* Functions for acting on the information about branches. */
 
+/*
+ * Parses branch-related configuration options.
+ */
+int git_default_branch_config(const char *var, const char *value);
+
 /*
  * Creates a new branch, where:
  *
diff --git a/cache.h b/cache.h
index ba04ff8bd3..6ea1ea5854 100644
--- a/cache.h
+++ b/cache.h
@@ -1030,7 +1030,6 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
-extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
 
 enum object_creation_mode {
diff --git a/config.c b/config.c
index f33abeab85..840be51710 100644
--- a/config.c
+++ b/config.c
@@ -1574,36 +1574,6 @@ static int git_default_i18n_config(const char *var, const char *value)
 	return 0;
 }
 
-static int git_default_branch_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "branch.autosetupmerge")) {
-		if (value && !strcasecmp(value, "always")) {
-			git_branch_track = BRANCH_TRACK_ALWAYS;
-			return 0;
-		}
-		git_branch_track = git_config_bool(var, value);
-		return 0;
-	}
-	if (!strcmp(var, "branch.autosetuprebase")) {
-		if (!value)
-			return config_error_nonbool(var);
-		else if (!strcmp(value, "never"))
-			autorebase = AUTOREBASE_NEVER;
-		else if (!strcmp(value, "local"))
-			autorebase = AUTOREBASE_LOCAL;
-		else if (!strcmp(value, "remote"))
-			autorebase = AUTOREBASE_REMOTE;
-		else if (!strcmp(value, "always"))
-			autorebase = AUTOREBASE_ALWAYS;
-		else
-			return error(_("malformed value for %s"), var);
-		return 0;
-	}
-
-	/* Add other config variables here and to Documentation/config.txt. */
-	return 0;
-}
-
 static int git_default_push_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "push.default")) {
diff --git a/environment.c b/environment.c
index 2f27008424..5d45152731 100644
--- a/environment.c
+++ b/environment.c
@@ -60,7 +60,6 @@ int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding = "SHIFT-JIS";
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
-- 
2.31.1

[PATCH v1 2/6] mailmap: move `git_default_mailmap_config` to `mailmap.c`

[Ben Boeckel]

This allows mailmap-specific variables to be tracked locally in
`mailmap.c` instead of globally.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 cache.h   |  2 --
 config.c  | 12 +-----------
 mailmap.c | 16 ++++++++++++++--
 mailmap.h |  2 ++
 4 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/cache.h b/cache.h
index 6ea1ea5854..551a6cb5cf 100644
--- a/cache.h
+++ b/cache.h
@@ -1718,8 +1718,6 @@ int author_ident_sufficiently_given(void);
 
 extern const char *git_commit_encoding;
 extern const char *git_log_output_encoding;
-extern const char *git_mailmap_file;
-extern const char *git_mailmap_blob;
 
 /* IO helper functions */
 void maybe_flush_or_die(FILE *, const char *);
diff --git a/config.c b/config.c
index 840be51710..c45001adbe 100644
--- a/config.c
+++ b/config.c
@@ -15,6 +15,7 @@
 #include "strbuf.h"
 #include "quote.h"
 #include "hashmap.h"
+#include "mailmap.h"
 #include "string-list.h"
 #include "object-store.h"
 #include "utf8.h"
@@ -1603,17 +1604,6 @@ static int git_default_push_config(const char *var, const char *value)
 	return 0;
 }
 
-static int git_default_mailmap_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "mailmap.file"))
-		return git_config_pathname(&git_mailmap_file, var, value);
-	if (!strcmp(var, "mailmap.blob"))
-		return git_config_string(&git_mailmap_blob, var, value);
-
-	/* Add other config variables here and to Documentation/config.txt. */
-	return 0;
-}
-
 int git_default_config(const char *var, const char *value, void *cb)
 {
 	if (starts_with(var, "core."))
diff --git a/mailmap.c b/mailmap.c
index d1f7c0d272..e1d9a2bbdb 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include "string-list.h"
 #include "mailmap.h"
 #include "object-store.h"
@@ -12,8 +13,19 @@ static inline void debug_mm(const char *format, ...) {}
 static inline const char *debug_str(const char *s) { return s; }
 #endif
 
-const char *git_mailmap_file;
-const char *git_mailmap_blob;
+static const char *git_mailmap_file;
+static const char *git_mailmap_blob;
+
+int git_default_mailmap_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "mailmap.file"))
+		return git_config_pathname(&git_mailmap_file, var, value);
+	if (!strcmp(var, "mailmap.blob"))
+		return git_config_string(&git_mailmap_blob, var, value);
+
+	/* Add other config variables here and to Documentation/config.txt. */
+	return 0;
+}
 
 struct mailmap_info {
 	char *name;
diff --git a/mailmap.h b/mailmap.h
index 7e99fccb46..48bfe9fac7 100644
--- a/mailmap.h
+++ b/mailmap.h
@@ -3,6 +3,8 @@
 
 struct string_list;
 
+int git_default_mailmap_config(const char *var, const char *value);
+
 int read_mailmap(struct string_list *map);
 void clear_mailmap(struct string_list *map);
 
-- 
2.31.1

[PATCH v1 3/6] apply: move `apply_default_*whitespace` to `apply.c`

[Ben Boeckel]

These variables are only set and used in this file, so they can become
`static`.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 apply.c       | 3 +++
 cache.h       | 2 --
 environment.c | 2 --
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/apply.c b/apply.c
index 44bc31d6eb..15dcd8b7d7 100644
--- a/apply.c
+++ b/apply.c
@@ -23,6 +23,9 @@
 #include "apply.h"
 #include "entry.h"
 
+static char *apply_default_whitespace;
+static char *apply_default_ignorewhitespace;
+
 struct gitdiff_data {
 	struct strbuf *root;
 	int linenr;
diff --git a/cache.h b/cache.h
index 551a6cb5cf..dc6c4172cb 100644
--- a/cache.h
+++ b/cache.h
@@ -944,8 +944,6 @@ extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int warn_ambiguous_refs;
 extern int warn_on_object_refname_ambiguity;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
 extern const char *git_hooks_path;
 extern int zlib_compression_level;
diff --git a/environment.c b/environment.c
index 5d45152731..0cc086d847 100644
--- a/environment.c
+++ b/environment.c
@@ -36,8 +36,6 @@ int repository_format_precious_objects;
 int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
 const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
-- 
2.31.1

[PATCH v1 4/6] config: remove `core_compression_level`

[Ben Boeckel]

This variable is just stored to after parsing `core.compression` and
then ignored, so it can just be completely removed.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 cache.h       | 1 -
 config.c      | 1 -
 environment.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/cache.h b/cache.h
index dc6c4172cb..4fd134863c 100644
--- a/cache.h
+++ b/cache.h
@@ -947,7 +947,6 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *git_attributes_file;
 extern const char *git_hooks_path;
 extern int zlib_compression_level;
-extern int core_compression_level;
 extern int pack_compression_level;
 extern size_t packed_git_window_size;
 extern size_t packed_git_limit;
diff --git a/config.c b/config.c
index c45001adbe..b241a56cfb 100644
--- a/config.c
+++ b/config.c
@@ -1401,7 +1401,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
 			level = Z_DEFAULT_COMPRESSION;
 		else if (level < 0 || level > Z_BEST_COMPRESSION)
 			die(_("bad zlib compression level %d"), level);
-		core_compression_level = level;
 		core_compression_seen = 1;
 		if (!zlib_compression_seen)
 			zlib_compression_level = level;
diff --git a/environment.c b/environment.c
index 0cc086d847..3b995434fc 100644
--- a/environment.c
+++ b/environment.c
@@ -39,7 +39,6 @@ const char *git_log_output_encoding;
 const char *git_attributes_file;
 const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
-int core_compression_level;
 int pack_compression_level = Z_DEFAULT_COMPRESSION;
 int fsync_object_files;
 size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
-- 
2.31.1

[PATCH v1 5/6] refs/debug: declare file-local variable to be static

[Ben Boeckel]

Declaring a variable in a source file as `extern` and then defining it
later is not very useful. It can instead be declared as `static`.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 refs/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/refs/debug.c b/refs/debug.c
index 7db4abccc3..f5609408f3 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -9,7 +9,7 @@ struct debug_ref_store {
 	struct ref_store *refs;
 };
 
-extern struct ref_storage_be refs_be_debug;
+static struct ref_storage_be refs_be_debug;
 
 struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_store *store)
 {
@@ -413,7 +413,7 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
 	return res;
 }
 
-struct ref_storage_be refs_be_debug = {
+static struct ref_storage_be refs_be_debug = {
 	NULL,
 	"debug",
 	NULL,
-- 
2.31.1

[PATCH v1 6/6] globals: remove explicit `0` initialization from globals

[Ben Boeckel]

Git relies on implicit zero-initialization of global variables.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 object-file.c | 2 +-
 progress.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/object-file.c b/object-file.c
index ecca5a8da0..6a04013342 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1503,7 +1503,7 @@ static int loose_object_info(struct repository *r,
 	return (status < 0) ? status : 0;
 }
 
-int obj_read_use_lock = 0;
+int obj_read_use_lock;
 pthread_mutex_t obj_read_mutex;
 
 void enable_obj_read_lock(void)
diff --git a/progress.c b/progress.c
index 680c6a8bf9..4fb4233b67 100644
--- a/progress.c
+++ b/progress.c
@@ -52,7 +52,7 @@ static volatile sig_atomic_t progress_update;
  * for 'test-tool progress'.
  */
 int progress_testing;
-uint64_t progress_test_ns = 0;
+uint64_t progress_test_ns;
 void progress_test_force_update(void)
 {
 	progress_update = 1;
-- 
2.31.1