config: set up config_source for command-line config

peff · gitster · commit 3258258f51f4 · 2016-05-24T13:21:59.000-07:00
When we parse a config file, we set up the global "cf"
variable as a pointer to a "struct config_source" describing
the file we are parsing. This is used for error messages, as
well as for lookup functions like current_config_name().

The "cf" variable is NULL in two cases:

  1. When we are parsing command-line config, in which case
     there is no source file.

  2. When we are not parsing any config at all.

Callers like current_config_name() must assume we are in
case 1 if they see a NULL "cf". However, this means that if
they are accidentally used outside of a config parsing
callback, they will quietly return a bogus answer.

This might seem like an unlikely accident (why would you ask
for the current config file if you are not parsing config?),
but it's actually an easy mistake to make due to the
configset caching. git_config() serves the answers from a
configset cache, and any calls to current_config_name() will
claim that we are parsing command-line config, no matter
what the original source.

So let's distinguish these cases by having the command-line
config parser set up a config_source with a NULL name (which
callers already handle properly). We can use this to catch
programming errors in some cases, and to give better
messages to the user in others.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/config.c b/config.c
@@ -131,7 +131,9 @@ static int handle_path_include(const char *path, struct config_include_data *inc
 	if (!access_or_die(path, R_OK, 0)) {
 		if (++inc->depth > MAX_INCLUDE_DEPTH)
 			die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
-			    cf && cf->name ? cf->name : "the command line");
+			    !cf ? "<unknown>" :
+			    cf->name ? cf->name :
+			    "the command line");
 		ret = git_config_from_file(git_config_include, path, inc);
 		inc->depth--;
 	}
@@ -210,9 +212,15 @@ int git_config_from_parameters(config_fn_t fn, void *data)
 	const char **argv = NULL;
 	int nr = 0, alloc = 0;
 	int i;
+	struct config_source source;
 
 	if (!env)
 		return 0;
+
+	memset(&source, 0, sizeof(source));
+	source.prev = cf;
+	cf = &source;
+
 	/* sq_dequote will write over it */
 	envw = xstrdup(env);
 
@@ -231,6 +239,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
 out:
 	free(argv);
 	free(envw);
+	cf = source.prev;
 	return ret;
 }
 
@@ -1341,7 +1350,9 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
 	l_item->e = e;
 	l_item->value_index = e->value_list.nr - 1;
 
-	if (cf) {
+	if (!cf)
+		die("BUG: configset_add_value has no source");
+	if (cf->name) {
 		kv_info->filename = strintern(cf->name);
 		kv_info->linenr = cf->linenr;
 	} else {
@@ -2427,10 +2438,14 @@ int parse_config_key(const char *var,
 
 const char *current_config_origin_type(void)
 {
-	return cf && cf->origin_type ? cf->origin_type : "command line";
+	if (!cf)
+		die("BUG: current_config_origin_type called outside config callback");
+	return cf->origin_type ? cf->origin_type : "command line";
 }
 
 const char *current_config_name(void)
 {
-	return cf && cf->name ? cf->name : "";
+	if (!cf)
+		die("BUG: current_config_name called outside config callback");
+	return cf->name ? cf->name : "";
 }
