@matheusbsbs / matheus-bagliano / commits / c009bc898b

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 399b198489 (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 <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Mar 28, 2023 at 17:51 UTC c009bc898bafadea8add49198cd30c40666b5f33

1 file changed +24 -13

config.c

+24 -13

@@ -49,6 +49,7 @@ struct config_source {

49 int (*do_ungetc)(int c, struct config_source *conf);

50 long (*do_ftell)(struct config_source *c);

51 };

52 +#define CONFIG_SOURCE_INIT { 0 }

54 /*

55 * These variables record the "current" config source, which

@@ -79,6 +80,22 @@ static struct key_value_info *current_config_kvi;

80 */

81 static enum config_scope current_parsing_scope;

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 +

99 static int pack_compression_seen;

100 static int zlib_compression_seen;

101

@@ -346,14 +363,12 @@ static void populate_remote_urls(struct config_include_data *inc)

363 {

364 struct config_options opts;

365

349 - struct config_source *store_cf = cf_global;

366 struct key_value_info *store_kvi = current_config_kvi;

367 enum config_scope store_scope = current_parsing_scope;

368

369 opts = *inc->opts;

370 opts.unconditional_remote_url = 1;

371

356 - cf_global = NULL;

372 current_config_kvi = NULL;

373 current_parsing_scope = 0;

374

@@ -361,7 +376,6 @@ static void populate_remote_urls(struct config_include_data *inc)

376 string_list_init_dup(inc->remote_urls);

377 config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts);

378

364 - cf_global = store_cf;

379 current_config_kvi = store_kvi;

380 current_parsing_scope = store_scope;

381 }

@@ -715,12 +729,10 @@ int git_config_from_parameters(config_fn_t fn, void *data)

729 struct strvec to_free = STRVEC_INIT;

730 int ret = 0;

731 char *envw = NULL;

718 - struct config_source source;

732 + struct config_source source = CONFIG_SOURCE_INIT;

733

720 - memset(&source, 0, sizeof(source));

721 - source.prev = cf_global;

734 source.origin_type = CONFIG_ORIGIN_CMDLINE;

723 - cf_global = &source;

735 + config_reader_push_source(&source);

736

737 env = getenv(CONFIG_COUNT_ENVIRONMENT);

738 if (env) {

@@ -778,7 +790,7 @@ out:

790 strbuf_release(&envvar);

791 strvec_clear(&to_free);

792 free(envw);

781 - cf_global = source.prev;

793 + config_reader_pop_source();

794 return ret;

795 }

796

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

1961 int ret;

1962

1963 /* push config-file parsing state stack */

1952 - top->prev = cf_global;

1964 top->linenr = 1;

1965 top->eof = 0;

1966 top->total_len = 0;

1967 strbuf_init(&top->value, 1024);

1968 strbuf_init(&top->var, 1024);

1958 - cf_global = top;

1969 + config_reader_push_source(top);

1970

1971 ret = git_parse_source(top, fn, data, opts);

1972

1973 /* pop config-file parsing state stack */

1974 strbuf_release(&top->value);

1975 strbuf_release(&top->var);

1965 - cf_global = top->prev;

1976 + config_reader_pop_source();

1977

1978 return ret;

1979 }

@@ -1972,7 +1983,7 @@ static int do_config_from_file(config_fn_t fn,

1983 const char *name, const char *path, FILE *f,

1984 void *data, const struct config_options *opts)

1985 {

1975 - struct config_source top;

1986 + struct config_source top = CONFIG_SOURCE_INIT;

1987 int ret;

1988

1989 top.u.file = f;

@@ -2024,7 +2035,7 @@ int git_config_from_mem(config_fn_t fn,

2035 const char *name, const char *buf, size_t len,

2036 void *data, const struct config_options *opts)

2037 {

2027 - struct config_source top;

2038 + struct config_source top = CONFIG_SOURCE_INIT;

2039

2040 top.u.buf.buf = buf;

2041 top.u.buf.len = len;