Re: [PATCH] Allow built-ins to also use -c var=val via alias

[Jeff King <peff@peff.net>]

On Tue, May 24, 2011 at 05:46:18PM -0400, Jeff King wrote:

> I think the right fix is simply to drop the "don't re-check the
> environment after the first time" logic. It's not expensive to parse
> compared to parsing config files, which is when we would do it. We can
> just drop the existing list and reparse. You can even get rid of the
> whole list and drop a bunch of code, I think, like:

Ack, wrong patch. That one doesn't even come close to compiling.

Try this (still not well tested, though).

---
 cache.h  |    2 -
 config.c |   68 ++++++++++++++++++++++++-------------------------------------
 2 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/cache.h b/cache.h
index 28a921d..69e09a1 100644
--- a/cache.h
+++ b/cache.h
@@ -1037,8 +1037,6 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
 extern int git_default_config(const char *, const char *, void *);
 extern int git_config_from_file(config_fn_t fn, const char *, void *);
 extern void git_config_push_parameter(const char *text);
-extern int git_config_parse_parameter(const char *text);
-extern int git_config_parse_environment(void);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
 extern int git_config(config_fn_t fn, void *);
 extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
diff --git a/config.c b/config.c
index 9d36848..90d5e6d 100644
--- a/config.c
+++ b/config.c
@@ -20,14 +20,6 @@ static int zlib_compression_seen;
 
 const char *config_exclusive_filename = NULL;
 
-struct config_item {
-	struct config_item *next;
-	char *name;
-	char *value;
-};
-static struct config_item *config_parameters;
-static struct config_item **config_parameters_tail = &config_parameters;
-
 static void lowercase(char *p)
 {
 	for (; *p; p++)
@@ -47,9 +39,9 @@ void git_config_push_parameter(const char *text)
 	strbuf_release(&env);
 }
 
-int git_config_parse_parameter(const char *text)
+int git_config_parse_parameter(const char *text,
+			       char **name, char **value)
 {
-	struct config_item *ct;
 	struct strbuf tmp = STRBUF_INIT;
 	struct strbuf **pair;
 	strbuf_addstr(&tmp, text);
@@ -61,25 +53,24 @@ int git_config_parse_parameter(const char *text)
 		strbuf_list_free(pair);
 		return -1;
 	}
-	ct = xcalloc(1, sizeof(struct config_item));
-	ct->name = strbuf_detach(pair[0], NULL);
+	*name = strbuf_detach(pair[0], NULL);
 	if (pair[1]) {
 		strbuf_trim(pair[1]);
-		ct->value = strbuf_detach(pair[1], NULL);
+		*value = strbuf_detach(pair[1], NULL);
 	}
 	strbuf_list_free(pair);
-	lowercase(ct->name);
-	*config_parameters_tail = ct;
-	config_parameters_tail = &ct->next;
+	lowercase(*name);
 	return 0;
 }
 
-int git_config_parse_environment(void) {
+int git_config_from_parameters(config_fn_t fn, void *data)
+{
 	const char *env = getenv(CONFIG_DATA_ENVIRONMENT);
 	char *envw;
 	const char **argv = NULL;
 	int nr = 0, alloc = 0;
 	int i;
+	int ret = 0;
 
 	if (!env)
 		return 0;
@@ -92,17 +83,25 @@ int git_config_parse_environment(void) {
 	}
 
 	for (i = 0; i < nr; i++) {
-		if (git_config_parse_parameter(argv[i]) < 0) {
+		char *name, *value;
+		if (git_config_parse_parameter(argv[i], &name, &value) < 0) {
 			error("bogus config parameter: %s", argv[i]);
-			free(argv);
-			free(envw);
-			return -1;
+			ret = -1;
+			goto out;
 		}
+		if (fn(name, value, data) < 0) {
+			ret = -1;
+			goto out;
+		}
+		free(name);
+		free(value);
+		ret++;
 	}
 
+out:
 	free(argv);
 	free(envw);
-	return 0;
+	return ret;
 }
 
 static int get_next_char(void)
@@ -837,25 +836,10 @@ int git_config_system(void)
 	return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
 }
 
-int git_config_from_parameters(config_fn_t fn, void *data)
-{
-	static int loaded_environment;
-	const struct config_item *ct;
-
-	if (!loaded_environment) {
-		if (git_config_parse_environment() < 0)
-			return -1;
-		loaded_environment = 1;
-	}
-	for (ct = config_parameters; ct; ct = ct->next)
-		if (fn(ct->name, ct->value, data) < 0)
-			return -1;
-	return 0;
-}
-
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
+	int n;
 	const char *home = NULL;
 
 	/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
@@ -882,9 +866,11 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	ret += git_config_from_parameters(fn, data);
-	if (config_parameters)
-		found += 1;
+	n = git_config_from_parameters(fn, data);
+	if (n < 0)
+		ret += n;
+	else
+		found += n;
 
 	return ret == 0 ? found : ret;
 }