diff mbox series

[v2,06/14] config.c: pass kvi in configsets

Message ID f57c1007cad106a6387c77ac4d06208d39bfb9f8.1685472133.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series config: remove global state from config iteration | expand

Commit Message

Glen Choo May 30, 2023, 6:42 p.m. UTC
From: Glen Choo <chooglen@google.com>

Trivially pass "struct key_value_info" to config callbacks in
configset_iter(). Then, in config callbacks that are only used with
configsets, use the "kvi" arg to replace calls to current_config_*(),
and delete current_config_line() because it has no remaining callers.

This leaves builtin/config.c and config.c as the only remaining users of
current_config_*().

Signed-off-by: Glen Choo <chooglen@google.com>
---
 builtin/remote.c       |  8 ++++----
 config.c               | 34 +++++++++++++++-------------------
 config.h               |  2 +-
 remote.c               |  6 +++---
 t/helper/test-config.c | 10 +++++-----
 5 files changed, 28 insertions(+), 32 deletions(-)

Comments

Jonathan Tan June 1, 2023, 10:21 p.m. UTC | #1
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Glen Choo <chooglen@google.com>
> 
> Trivially pass "struct key_value_info" to config callbacks in
> configset_iter(). Then, in config callbacks that are only used with
> configsets, use the "kvi" arg to replace calls to current_config_*(),

OK, so the end result of this patch is, indeed, that configset_iter()
passes kvi, and this is proven by some callbacks being changed to use
the kvi provided, and still functioning. I did not verify that the
callbacks in this patch were the *only* callbacks that are only used
with configsets, but I don't think that is important for the purpose of
reviewing this patch - as long as current_config_* has all been removed
at the end of this patchset, then things are fine.

> and delete current_config_line() because it has no remaining callers.

Ah, great that we can remove one of the global-reliant functions as a
result of this.
diff mbox series

Patch

diff --git a/builtin/remote.c b/builtin/remote.c
index edb4a9ddd7f..0a18c49f1f8 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -646,17 +646,17 @@  struct push_default_info
 };
 
 static int config_read_push_default(const char *key, const char *value,
-	struct key_value_info *kvi UNUSED, void *cb)
+	struct key_value_info *kvi, void *cb)
 {
 	struct push_default_info* info = cb;
 	if (strcmp(key, "remote.pushdefault") ||
 	    !value || strcmp(value, info->old_name))
 		return 0;
 
-	info->scope = current_config_scope();
+	info->scope = kvi->scope;
 	strbuf_reset(&info->origin);
-	strbuf_addstr(&info->origin, current_config_name());
-	info->linenr = current_config_line();
+	strbuf_addstr(&info->origin, config_origin_type_name(kvi->origin_type));
+	info->linenr = kvi->linenr;
 
 	return 0;
 }
diff --git a/config.c b/config.c
index 1bc2e35a3e0..66078b22eef 100644
--- a/config.c
+++ b/config.c
@@ -2301,19 +2301,18 @@  static void configset_iter(struct config_reader *reader, struct config_set *set,
 	struct string_list *values;
 	struct config_set_element *entry;
 	struct configset_list *list = &set->list;
+	struct key_value_info *kvi;
 
 	for (i = 0; i < list->nr; i++) {
 		entry = list->items[i].e;
 		value_index = list->items[i].value_index;
 		values = &entry->value_list;
+		kvi = values->items[value_index].util;
 
 		config_reader_set_kvi(reader, values->items[value_index].util);
 
-		if (fn(entry->key, values->items[value_index].string, NULL, data) < 0)
-			git_die_config_linenr(entry->key,
-					      reader->config_kvi->filename,
-					      reader->config_kvi->linenr);
-
+		if (fn(entry->key, values->items[value_index].string, kvi, data) < 0)
+			git_die_config_linenr(entry->key, kvi->filename, kvi->linenr);
 		config_reader_set_kvi(reader, NULL);
 	}
 }
@@ -3951,13 +3950,8 @@  static int reader_origin_type(struct config_reader *reader,
 	return 0;
 }
 
-const char *current_config_origin_type(void)
+const char *config_origin_type_name(enum config_origin_type type)
 {
-	enum config_origin_type type = CONFIG_ORIGIN_UNKNOWN;
-
-	if (reader_origin_type(&the_reader, &type))
-		BUG("current_config_origin_type called outside config callback");
-
 	switch (type) {
 	case CONFIG_ORIGIN_BLOB:
 		return "blob";
@@ -3974,6 +3968,16 @@  const char *current_config_origin_type(void)
 	}
 }
 
+const char *current_config_origin_type(void)
+{
+	enum config_origin_type type = CONFIG_ORIGIN_UNKNOWN;
+
+	if (reader_origin_type(&the_reader, &type))
+		BUG("current_config_origin_type called outside config callback");
+
+	return config_origin_type_name(type);
+}
+
 const char *config_scope_name(enum config_scope scope)
 {
 	switch (scope) {
@@ -4021,14 +4025,6 @@  enum config_scope current_config_scope(void)
 		return the_reader.parsing_scope;
 }
 
-int current_config_line(void)
-{
-	if (the_reader.config_kvi)
-		return the_reader.config_kvi->linenr;
-	else
-		return the_reader.source->linenr;
-}
-
 int lookup_config(const char **mapping, int nr_mapping, const char *var)
 {
 	int i;
diff --git a/config.h b/config.h
index 9e0c1f12165..ea4ffe37a55 100644
--- a/config.h
+++ b/config.h
@@ -367,7 +367,7 @@  int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
 enum config_scope current_config_scope(void);
 const char *current_config_origin_type(void);
 const char *current_config_name(void);
-int current_config_line(void);
+const char *config_origin_type_name(enum config_origin_type type);
 
 /*
  * Match and parse a config key of the form:
diff --git a/remote.c b/remote.c
index 10868a963f2..266601157e3 100644
--- a/remote.c
+++ b/remote.c
@@ -349,7 +349,7 @@  static void read_branches_file(struct remote_state *remote_state,
 }
 
 static int handle_config(const char *key, const char *value,
-			 struct key_value_info *kvi UNUSED, void *cb)
+			 struct key_value_info *kvi, void *cb)
 {
 	const char *name;
 	size_t namelen;
@@ -414,8 +414,8 @@  static int handle_config(const char *key, const char *value,
 	}
 	remote = make_remote(remote_state, name, namelen);
 	remote->origin = REMOTE_CONFIG;
-	if (current_config_scope() == CONFIG_SCOPE_LOCAL ||
-	    current_config_scope() == CONFIG_SCOPE_WORKTREE)
+	if (kvi->scope == CONFIG_SCOPE_LOCAL ||
+	    kvi->scope == CONFIG_SCOPE_WORKTREE)
 		remote->configured_in_repo = 1;
 	if (!strcmp(subkey, "mirror"))
 		remote->mirror = git_config_bool(key, value);
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 00cd49e5145..7027ffa187f 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -43,7 +43,7 @@ 
  */
 
 static int iterate_cb(const char *var, const char *value,
-		      struct key_value_info *kvi UNUSED, void *data UNUSED)
+		      struct key_value_info *kvi, void *data UNUSED)
 {
 	static int nr;
 
@@ -52,10 +52,10 @@  static int iterate_cb(const char *var, const char *value,
 
 	printf("key=%s\n", var);
 	printf("value=%s\n", value ? value : "(null)");
-	printf("origin=%s\n", current_config_origin_type());
-	printf("name=%s\n", current_config_name());
-	printf("lno=%d\n", current_config_line());
-	printf("scope=%s\n", config_scope_name(current_config_scope()));
+	printf("origin=%s\n", config_origin_type_name(kvi->origin_type));
+	printf("name=%s\n", kvi->filename ? kvi->filename : "");
+	printf("lno=%d\n", kvi->linenr);
+	printf("scope=%s\n", config_scope_name(kvi->scope));
 
 	return 0;
 }