config.c: remove current_config_kvi

Add ".config_kvi" to "struct config_reader" and replace "current_config_kvi" with "the_reader.config_kvi", plumbing "struct config_reader" where necesssary. Also, introduce a setter function for ".config_kvi", which allows us to enforce the contraint that only one of ".source" and ".config_kvi" can be set at a time (as documented in the comments). Because of this constraint, we know that "populate_remote_urls()" was never touching "current_config_kvi" when iterating through config files, so it doesn't need to store and restore that value. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Mar 28, 2023 at 17:51 UTC 9828453ff00b330c57daa3a8b672cbb5f0cdce34
1 file changed +43 -39
config.c
+43 -39
@@ -52,7 +52,24 @@ struct config_source {
52 #define CONFIG_SOURCE_INIT { 0 }
53
54 struct config_reader {
55 + /*
56 + * These members record the "current" config source, which can be
57 + * accessed by parsing callbacks.
58 + *
59 + * The "source" variable will be non-NULL only when we are actually
60 + * parsing a real config source (file, blob, cmdline, etc).
61 + *
62 + * The "config_kvi" variable will be non-NULL only when we are feeding
63 + * cached config from a configset into a callback.
64 + *
65 + * They cannot be non-NULL at the same time. If they are both NULL, then
66 + * we aren't parsing anything (and depending on the function looking at
67 + * the variables, it's either a bug for it to be called in the first
68 + * place, or it's a function which can be reused for non-config
69 + * purposes, and should fall back to some sane behavior).
70 + */
71 struct config_source *source;
72 + struct key_value_info *config_kvi;
73 };
74 /*
75 * Where possible, prefer to accept "struct config_reader" as an arg than to use
@@ -61,27 +78,6 @@ struct config_reader {
78 */
79 static struct config_reader the_reader;
80
64 -/*
65 - * FIXME The comments are temporarily out of date since "cf_global" has been
66 - * moved to the_reader, but not current_*.
67 - *
68 - * These variables record the "current" config source, which
69 - * can be accessed by parsing callbacks.
70 - *
71 - * The "cf_global" variable will be non-NULL only when we are actually
72 - * parsing a real config source (file, blob, cmdline, etc).
73 - *
74 - * The "current_config_kvi" variable will be non-NULL only when we are feeding
75 - * cached config from a configset into a callback.
76 - *
77 - * They should generally never be non-NULL at the same time. If they are both
78 - * NULL, then we aren't parsing anything (and depending on the function looking
79 - * at the variables, it's either a bug for it to be called in the first place,
80 - * or it's a function which can be reused for non-config purposes, and should
81 - * fall back to some sane behavior).
82 - */
83 -static struct key_value_info *current_config_kvi;
84 -
81 /*
82 * Similar to the variables above, this gives access to the "scope" of the
83 * current value (repo, global, etc). For cached values, it can be found via
@@ -95,6 +91,8 @@ static enum config_scope current_parsing_scope;
91 static inline void config_reader_push_source(struct config_reader *reader,
92 struct config_source *top)
93 {
94 + if (reader->config_kvi)
95 + BUG("source should not be set while iterating a config set");
96 top->prev = reader->source;
97 reader->source = top;
98 }
@@ -109,6 +107,14 @@ static inline struct config_source *config_reader_pop_source(struct config_reade
107 return ret;
108 }
109
110 +static inline void config_reader_set_kvi(struct config_reader *reader,
111 + struct key_value_info *kvi)
112 +{
113 + if (kvi && reader->source)
114 + BUG("kvi should not be set while parsing a config source");
115 + reader->config_kvi = kvi;
116 +}
117 +
118 static int pack_compression_seen;
119 static int zlib_compression_seen;
120
@@ -377,20 +383,17 @@ static void populate_remote_urls(struct config_include_data *inc)
383 {
384 struct config_options opts;
385
380 - struct key_value_info *store_kvi = current_config_kvi;
386 enum config_scope store_scope = current_parsing_scope;
387
388 opts = *inc->opts;
389 opts.unconditional_remote_url = 1;
390
386 - current_config_kvi = NULL;
391 current_parsing_scope = 0;
392
393 inc->remote_urls = xmalloc(sizeof(*inc->remote_urls));
394 string_list_init_dup(inc->remote_urls);
395 config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts);
396
393 - current_config_kvi = store_kvi;
397 current_parsing_scope = store_scope;
398 }
399
@@ -2257,7 +2260,8 @@ int config_with_options(config_fn_t fn, void *data,
2260 return ret;
2261 }
2262
2260 -static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
2263 +static void configset_iter(struct config_reader *reader, struct config_set *cs,
2264 + config_fn_t fn, void *data)
2265 {
2266 int i, value_index;
2267 struct string_list *values;
@@ -2269,14 +2273,14 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
2273 value_index = list->items[i].value_index;
2274 values = &entry->value_list;
2275
2272 - current_config_kvi = values->items[value_index].util;
2276 + config_reader_set_kvi(reader, values->items[value_index].util);
2277
2278 if (fn(entry->key, values->items[value_index].string, data) < 0)
2279 git_die_config_linenr(entry->key,
2276 - current_config_kvi->filename,
2277 - current_config_kvi->linenr);
2280 + reader->config_kvi->filename,
2281 + reader->config_kvi->linenr);
2282
2279 - current_config_kvi = NULL;
2283 + config_reader_set_kvi(reader, NULL);
2284 }
2285 }
2286
@@ -2614,7 +2618,7 @@ static void repo_config_clear(struct repository *repo)
2618 void repo_config(struct repository *repo, config_fn_t fn, void *data)
2619 {
2620 git_config_check_init(repo);
2617 - configset_iter(repo->config, fn, data);
2621 + configset_iter(&the_reader, repo->config, fn, data);
2622 }
2623
2624 int repo_config_get_value(struct repository *repo,
@@ -2720,7 +2724,7 @@ void git_protected_config(config_fn_t fn, void *data)
2724 {
2725 if (!protected_config.hash_initialized)
2726 read_protected_config();
2723 - configset_iter(&protected_config, fn, data);
2727 + configset_iter(&the_reader, &protected_config, fn, data);
2728 }
2729
2730 /* Functions used historically to read configuration from 'the_repository' */
@@ -3827,8 +3831,8 @@ int parse_config_key(const char *var,
3831 const char *current_config_origin_type(void)
3832 {
3833 int type;
3830 - if (current_config_kvi)
3831 - type = current_config_kvi->origin_type;
3834 + if (the_reader.config_kvi)
3835 + type = the_reader.config_kvi->origin_type;
3836 else if(the_reader.source)
3837 type = the_reader.source->origin_type;
3838 else
@@ -3873,8 +3877,8 @@ const char *config_scope_name(enum config_scope scope)
3877 const char *current_config_name(void)
3878 {
3879 const char *name;
3876 - if (current_config_kvi)
3877 - name = current_config_kvi->filename;
3880 + if (the_reader.config_kvi)
3881 + name = the_reader.config_kvi->filename;
3882 else if (the_reader.source)
3883 name = the_reader.source->name;
3884 else
@@ -3884,16 +3888,16 @@ const char *current_config_name(void)
3888
3889 enum config_scope current_config_scope(void)
3890 {
3887 - if (current_config_kvi)
3888 - return current_config_kvi->scope;
3891 + if (the_reader.config_kvi)
3892 + return the_reader.config_kvi->scope;
3893 else
3894 return current_parsing_scope;
3895 }
3896
3897 int current_config_line(void)
3898 {
3895 - if (current_config_kvi)
3896 - return current_config_kvi->linenr;
3899 + if (the_reader.config_kvi)
3900 + return the_reader.config_kvi->linenr;
3901 else
3902 return the_reader.source->linenr;
3903 }