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>

diff --git a/config.c b/config.c
index e4a76739365812c4f0786d0954a312fdb82f19f9..6627fad71cff81c53be24015dd29bba243390bd4 100644 (file)
--- a/config.c
+++ b/config.c
@@ -49,6 +49,7 @@ struct config_source {
        int (*do_ungetc)(int c, struct config_source *conf);
        long (*do_ftell)(struct config_source *c);
 };
+#define CONFIG_SOURCE_INIT { 0 }
 
 /*
  * These variables record the "current" config source, which
@@ -79,6 +80,22 @@ static struct key_value_info *current_config_kvi;
  */
 static enum config_scope current_parsing_scope;
 
+static inline void config_reader_push_source(struct config_source *top)
+{
+       top->prev = cf_global;
+       cf_global = top;
+}
+
+static inline struct config_source *config_reader_pop_source()
+{
+       struct config_source *ret;
+       if (!cf_global)
+               BUG("tried to pop config source, but we weren't reading config");
+       ret = cf_global;
+       cf_global = cf_global->prev;
+       return ret;
+}
+
 static int pack_compression_seen;
 static int zlib_compression_seen;
 
@@ -346,14 +363,12 @@ static void populate_remote_urls(struct config_include_data *inc)
 {
        struct config_options opts;
 
-       struct config_source *store_cf = cf_global;
        struct key_value_info *store_kvi = current_config_kvi;
        enum config_scope store_scope = current_parsing_scope;
 
        opts = *inc->opts;
        opts.unconditional_remote_url = 1;
 
-       cf_global = NULL;
        current_config_kvi = NULL;
        current_parsing_scope = 0;
 
@@ -361,7 +376,6 @@ static void populate_remote_urls(struct config_include_data *inc)
        string_list_init_dup(inc->remote_urls);
        config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts);
 
-       cf_global = store_cf;
        current_config_kvi = store_kvi;
        current_parsing_scope = store_scope;
 }
@@ -715,12 +729,10 @@ int git_config_from_parameters(config_fn_t fn, void *data)
        struct strvec to_free = STRVEC_INIT;
        int ret = 0;
        char *envw = NULL;
-       struct config_source source;
+       struct config_source source = CONFIG_SOURCE_INIT;
 
-       memset(&source, 0, sizeof(source));
-       source.prev = cf_global;
        source.origin_type = CONFIG_ORIGIN_CMDLINE;
-       cf_global = &source;
+       config_reader_push_source(&source);
 
        env = getenv(CONFIG_COUNT_ENVIRONMENT);
        if (env) {
@@ -778,7 +790,7 @@ out:
        strbuf_release(&envvar);
        strvec_clear(&to_free);
        free(envw);
-       cf_global = source.prev;
+       config_reader_pop_source();
        return ret;
 }
 
@@ -1949,20 +1961,19 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data,
        int ret;
 
        /* push config-file parsing state stack */
-       top->prev = cf_global;
        top->linenr = 1;
        top->eof = 0;
        top->total_len = 0;
        strbuf_init(&top->value, 1024);
        strbuf_init(&top->var, 1024);
-       cf_global = top;
+       config_reader_push_source(top);
 
        ret = git_parse_source(top, fn, data, opts);
 
        /* pop config-file parsing state stack */
        strbuf_release(&top->value);
        strbuf_release(&top->var);
-       cf_global = top->prev;
+       config_reader_pop_source();
 
        return ret;
 }
@@ -1972,7 +1983,7 @@ static int do_config_from_file(config_fn_t fn,
                const char *name, const char *path, FILE *f,
                void *data, const struct config_options *opts)
 {
-       struct config_source top;
+       struct config_source top = CONFIG_SOURCE_INIT;
        int ret;
 
        top.u.file = f;
@@ -2024,7 +2035,7 @@ int git_config_from_mem(config_fn_t fn,
                        const char *name, const char *buf, size_t len,
                        void *data, const struct config_options *opts)
 {
-       struct config_source top;
+       struct config_source top = CONFIG_SOURCE_INIT;
 
        top.u.buf.buf = buf;
        top.u.buf.len = len;