Skip to content

Commit c009bc8

Browse files
chooglengitster
authored andcommitted
config.c: don't assign to "cf_global" directly
To make "cf_global" easier to remove, replace all direct assignments to it with function calls. This refactor has an additional maintainability benefit: all of these functions were manually implementing stack pop/push semantics on "struct config_source", so replacing them with function calls allows us to only implement this logic once. In this process, perform some now-obvious clean ups: - Drop some unnecessary "cf_global" assignments in populate_remote_urls(). Since it was introduced in 399b198 (config: include file if remote URL matches a glob, 2022-01-18), it has stored and restored the value of "cf_global" to ensure that it doesn't get accidentally mutated. However, this was never necessary since "do_config_from()" already pushes/pops "cf_global" further down the call chain. - Zero out every "struct config_source" with a dedicated initializer. This matters because the "struct config_source" is assigned to "cf_global" and we later 'pop the stack' by assigning "cf_global = cf_global->prev", but "cf_global->prev" could be pointing to uninitialized garbage. Fortunately, this has never bothered us since we never try to read "cf_global" except while iterating through config, in which case, "cf_global" is either set to a sensible value (when parsing a file), or it is ignored (when iterating a configset). Later in the series, zero-ing out memory will also let us enforce the constraint that "cf_global" and "current_config_kvi" are never non-NULL together. Signed-off-by: Glen Choo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c97f3ed commit c009bc8

File tree

1 file changed

+24
-13
lines changed

1 file changed

+24
-13
lines changed

config.c

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ struct config_source {
4949
int (*do_ungetc)(int c, struct config_source *conf);
5050
long (*do_ftell)(struct config_source *c);
5151
};
52+
#define CONFIG_SOURCE_INIT { 0 }
5253

5354
/*
5455
* These variables record the "current" config source, which
@@ -79,6 +80,22 @@ static struct key_value_info *current_config_kvi;
7980
*/
8081
static enum config_scope current_parsing_scope;
8182

83+
static inline void config_reader_push_source(struct config_source *top)
84+
{
85+
top->prev = cf_global;
86+
cf_global = top;
87+
}
88+
89+
static inline struct config_source *config_reader_pop_source()
90+
{
91+
struct config_source *ret;
92+
if (!cf_global)
93+
BUG("tried to pop config source, but we weren't reading config");
94+
ret = cf_global;
95+
cf_global = cf_global->prev;
96+
return ret;
97+
}
98+
8299
static int pack_compression_seen;
83100
static int zlib_compression_seen;
84101

@@ -346,22 +363,19 @@ static void populate_remote_urls(struct config_include_data *inc)
346363
{
347364
struct config_options opts;
348365

349-
struct config_source *store_cf = cf_global;
350366
struct key_value_info *store_kvi = current_config_kvi;
351367
enum config_scope store_scope = current_parsing_scope;
352368

353369
opts = *inc->opts;
354370
opts.unconditional_remote_url = 1;
355371

356-
cf_global = NULL;
357372
current_config_kvi = NULL;
358373
current_parsing_scope = 0;
359374

360375
inc->remote_urls = xmalloc(sizeof(*inc->remote_urls));
361376
string_list_init_dup(inc->remote_urls);
362377
config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts);
363378

364-
cf_global = store_cf;
365379
current_config_kvi = store_kvi;
366380
current_parsing_scope = store_scope;
367381
}
@@ -715,12 +729,10 @@ int git_config_from_parameters(config_fn_t fn, void *data)
715729
struct strvec to_free = STRVEC_INIT;
716730
int ret = 0;
717731
char *envw = NULL;
718-
struct config_source source;
732+
struct config_source source = CONFIG_SOURCE_INIT;
719733

720-
memset(&source, 0, sizeof(source));
721-
source.prev = cf_global;
722734
source.origin_type = CONFIG_ORIGIN_CMDLINE;
723-
cf_global = &source;
735+
config_reader_push_source(&source);
724736

725737
env = getenv(CONFIG_COUNT_ENVIRONMENT);
726738
if (env) {
@@ -778,7 +790,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
778790
strbuf_release(&envvar);
779791
strvec_clear(&to_free);
780792
free(envw);
781-
cf_global = source.prev;
793+
config_reader_pop_source();
782794
return ret;
783795
}
784796

@@ -1949,20 +1961,19 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data,
19491961
int ret;
19501962

19511963
/* push config-file parsing state stack */
1952-
top->prev = cf_global;
19531964
top->linenr = 1;
19541965
top->eof = 0;
19551966
top->total_len = 0;
19561967
strbuf_init(&top->value, 1024);
19571968
strbuf_init(&top->var, 1024);
1958-
cf_global = top;
1969+
config_reader_push_source(top);
19591970

19601971
ret = git_parse_source(top, fn, data, opts);
19611972

19621973
/* pop config-file parsing state stack */
19631974
strbuf_release(&top->value);
19641975
strbuf_release(&top->var);
1965-
cf_global = top->prev;
1976+
config_reader_pop_source();
19661977

19671978
return ret;
19681979
}
@@ -1972,7 +1983,7 @@ static int do_config_from_file(config_fn_t fn,
19721983
const char *name, const char *path, FILE *f,
19731984
void *data, const struct config_options *opts)
19741985
{
1975-
struct config_source top;
1986+
struct config_source top = CONFIG_SOURCE_INIT;
19761987
int ret;
19771988

19781989
top.u.file = f;
@@ -2024,7 +2035,7 @@ int git_config_from_mem(config_fn_t fn,
20242035
const char *name, const char *buf, size_t len,
20252036
void *data, const struct config_options *opts)
20262037
{
2027-
struct config_source top;
2038+
struct config_source top = CONFIG_SOURCE_INIT;
20282039

20292040
top.u.buf.buf = buf;
20302041
top.u.buf.len = len;

0 commit comments

Comments
 (0)