config.c: create config_reader and the_reader

Create "struct config_reader" to hold the state of the config source currently being read. Then, create a static instance of it, "the_reader", and use "the_reader.source" to replace references to "cf_global" in public functions. This doesn't create much immediate benefit (since we're mostly replacing static variables with a bigger static variable), but it prepares us for a future where this state doesn't have to be global; "struct config_reader" (or a similar struct) could be provided by the caller, or constructed internally by a function like "do_config_from()". A more typical approach would be to put this struct on "the_repository", but that's a worse fit for this use case since config reading is not scoped to a repository. E.g. we can read config before the repository is known ("read_very_early_config()"), blatantly ignore the repo ("read_protected_config()"), or read only from a file ("git_config_from_file()"). This is especially evident in t5318 and t9210, where test-tool and scalar parse config but don't fully initialize "the_repository". We could have also replaced the references to "cf_global" in callback functions (which are the only ones left), but we'll eventually plumb "the_reader" through the callback "*data" arg, so that would be unnecessary churn. Until we remove "cf_global" altogether, add logic to "config_reader_*_source()" to keep "cf_global" and "the_reader.source" in sync. 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 0c60285147441fbf93e435b4b022fe362b616a5a
1 file changed +51 -31
config.c
+51 -31
@@ -51,6 +51,16 @@ struct config_source {
51 };
52 #define CONFIG_SOURCE_INIT { 0 }
53
54 +struct config_reader {
55 + struct config_source *source;
56 +};
57 +/*
58 + * Where possible, prefer to accept "struct config_reader" as an arg than to use
59 + * "the_reader". "the_reader" should only be used if that is infeasible, e.g. in
60 + * a public function.
61 + */
62 +static struct config_reader the_reader;
63 +
64 /*
65 * These variables record the "current" config source, which
66 * can be accessed by parsing callbacks.
@@ -66,6 +76,9 @@ struct config_source {
76 * at the variables, it's either a bug for it to be called in the first place,
77 * or it's a function which can be reused for non-config purposes, and should
78 * fall back to some sane behavior).
79 + *
80 + * FIXME "cf_global" has been replaced by "the_reader.source", remove
81 + * "cf_global" once we plumb "the_reader" through all of the callback functions.
82 */
83 static struct config_source *cf_global;
84 static struct key_value_info *current_config_kvi;
@@ -80,19 +93,24 @@ static struct key_value_info *current_config_kvi;
93 */
94 static enum config_scope current_parsing_scope;
95
83 -static inline void config_reader_push_source(struct config_source *top)
96 +static inline void config_reader_push_source(struct config_reader *reader,
97 + struct config_source *top)
98 {
85 - top->prev = cf_global;
86 - cf_global = top;
99 + top->prev = reader->source;
100 + reader->source = top;
101 + /* FIXME remove this when cf_global is removed. */
102 + cf_global = reader->source;
103 }
104
89 -static inline struct config_source *config_reader_pop_source()
105 +static inline struct config_source *config_reader_pop_source(struct config_reader *reader)
106 {
107 struct config_source *ret;
92 - if (!cf_global)
108 + if (!reader->source)
109 BUG("tried to pop config source, but we weren't reading config");
94 - ret = cf_global;
95 - cf_global = cf_global->prev;
110 + ret = reader->source;
111 + reader->source = reader->source->prev;
112 + /* FIXME remove this when cf_global is removed. */
113 + cf_global = reader->source;
114 return ret;
115 }
116
@@ -732,7 +750,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
750 struct config_source source = CONFIG_SOURCE_INIT;
751
752 source.origin_type = CONFIG_ORIGIN_CMDLINE;
735 - config_reader_push_source(&source);
753 + config_reader_push_source(&the_reader, &source);
754
755 env = getenv(CONFIG_COUNT_ENVIRONMENT);
756 if (env) {
@@ -790,7 +808,7 @@ out:
808 strbuf_release(&envvar);
809 strvec_clear(&to_free);
810 free(envw);
793 - config_reader_pop_source();
811 + config_reader_pop_source(&the_reader);
812 return ret;
813 }
814
@@ -1325,7 +1343,7 @@ int git_config_int(const char *name, const char *value)
1343 {
1344 int ret;
1345 if (!git_parse_int(value, &ret))
1328 - die_bad_number(cf_global, name, value);
1346 + die_bad_number(the_reader.source, name, value);
1347 return ret;
1348 }
1349
@@ -1333,7 +1351,7 @@ int64_t git_config_int64(const char *name, const char *value)
1351 {
1352 int64_t ret;
1353 if (!git_parse_int64(value, &ret))
1336 - die_bad_number(cf_global, name, value);
1354 + die_bad_number(the_reader.source, name, value);
1355 return ret;
1356 }
1357
@@ -1341,7 +1359,7 @@ unsigned long git_config_ulong(const char *name, const char *value)
1359 {
1360 unsigned long ret;
1361 if (!git_parse_ulong(value, &ret))
1344 - die_bad_number(cf_global, name, value);
1362 + die_bad_number(the_reader.source, name, value);
1363 return ret;
1364 }
1365
@@ -1349,7 +1367,7 @@ ssize_t git_config_ssize_t(const char *name, const char *value)
1367 {
1368 ssize_t ret;
1369 if (!git_parse_ssize_t(value, &ret))
1352 - die_bad_number(cf_global, name, value);
1370 + die_bad_number(the_reader.source, name, value);
1371 return ret;
1372 }
1373
@@ -1955,7 +1973,8 @@ int git_default_config(const char *var, const char *value, void *cb)
1973 * fgetc, ungetc, ftell of top need to be initialized before calling
1974 * this function.
1975 */
1958 -static int do_config_from(struct config_source *top, config_fn_t fn, void *data,
1976 +static int do_config_from(struct config_reader *reader,
1977 + struct config_source *top, config_fn_t fn, void *data,
1978 const struct config_options *opts)
1979 {
1980 int ret;
@@ -1966,22 +1985,23 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data,
1985 top->total_len = 0;
1986 strbuf_init(&top->value, 1024);
1987 strbuf_init(&top->var, 1024);
1969 - config_reader_push_source(top);
1988 + config_reader_push_source(reader, top);
1989
1990 ret = git_parse_source(top, fn, data, opts);
1991
1992 /* pop config-file parsing state stack */
1993 strbuf_release(&top->value);
1994 strbuf_release(&top->var);
1976 - config_reader_pop_source();
1995 + config_reader_pop_source(reader);
1996
1997 return ret;
1998 }
1999
1981 -static int do_config_from_file(config_fn_t fn,
1982 - const enum config_origin_type origin_type,
1983 - const char *name, const char *path, FILE *f,
1984 - void *data, const struct config_options *opts)
2000 +static int do_config_from_file(struct config_reader *reader,
2001 + config_fn_t fn,
2002 + const enum config_origin_type origin_type,
2003 + const char *name, const char *path, FILE *f,
2004 + void *data, const struct config_options *opts)
2005 {
2006 struct config_source top = CONFIG_SOURCE_INIT;
2007 int ret;
@@ -1996,15 +2016,15 @@ static int do_config_from_file(config_fn_t fn,
2016 top.do_ftell = config_file_ftell;
2017
2018 flockfile(f);
1999 - ret = do_config_from(&top, fn, data, opts);
2019 + ret = do_config_from(reader, &top, fn, data, opts);
2020 funlockfile(f);
2021 return ret;
2022 }
2023
2024 static int git_config_from_stdin(config_fn_t fn, void *data)
2025 {
2006 - return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin,
2007 - data, NULL);
2026 + return do_config_from_file(&the_reader, fn, CONFIG_ORIGIN_STDIN, "",
2027 + NULL, stdin, data, NULL);
2028 }
2029
2030 int git_config_from_file_with_options(config_fn_t fn, const char *filename,
@@ -2018,8 +2038,8 @@ int git_config_from_file_with_options(config_fn_t fn, const char *filename,
2038 BUG("filename cannot be NULL");
2039 f = fopen_or_warn(filename, "r");
2040 if (f) {
2021 - ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename,
2022 - filename, f, data, opts);
2041 + ret = do_config_from_file(&the_reader, fn, CONFIG_ORIGIN_FILE,
2042 + filename, filename, f, data, opts);
2043 fclose(f);
2044 }
2045 return ret;
@@ -2048,7 +2068,7 @@ int git_config_from_mem(config_fn_t fn,
2068 top.do_ungetc = config_buf_ungetc;
2069 top.do_ftell = config_buf_ftell;
2070
2051 - return do_config_from(&top, fn, data, opts);
2071 + return do_config_from(&the_reader, &top, fn, data, opts);
2072 }
2073
2074 int git_config_from_blob_oid(config_fn_t fn,
@@ -3797,8 +3817,8 @@ const char *current_config_origin_type(void)
3817 int type;
3818 if (current_config_kvi)
3819 type = current_config_kvi->origin_type;
3800 - else if(cf_global)
3801 - type = cf_global->origin_type;
3820 + else if(the_reader.source)
3821 + type = the_reader.source->origin_type;
3822 else
3823 BUG("current_config_origin_type called outside config callback");
3824
@@ -3843,8 +3863,8 @@ const char *current_config_name(void)
3863 const char *name;
3864 if (current_config_kvi)
3865 name = current_config_kvi->filename;
3846 - else if (cf_global)
3847 - name = cf_global->name;
3866 + else if (the_reader.source)
3867 + name = the_reader.source->name;
3868 else
3869 BUG("current_config_name called outside config callback");
3870 return name ? name : "";
@@ -3863,7 +3883,7 @@ int current_config_line(void)
3883 if (current_config_kvi)
3884 return current_config_kvi->linenr;
3885 else
3866 - return cf_global->linenr;
3886 + return the_reader.source->linenr;
3887 }
3888
3889 int lookup_config(const char **mapping, int nr_mapping, const char *var)