[PATCH 0/7] Rewrite `git_config()` using config-set API

@@ -244,6 +244,7 @@ static int get_next_char(void)
 		cf->linenr++;
 	if (c == EOF) {
 		cf->eof = 1;
+		cf->linenr++;
 		c = '\n';
 	}
 	return c;

@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)
 {
 	int c;
 	char *value;
+	int ret;
 
 	/* Get the full name */
 	for (;;) {

@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)
 		if (!value)
 			return -1;
 	}
-	return fn(name->buf, value, data);
+	/*
+	 * We already consumed the \n, but we need linenr to point to
+	 * the line we just parsed during the call to fn to get
+	 * accurate line number in error messages.
+	 */
+	cf->linenr--;
+	ret = fn(name->buf, value, data);
+	cf->linenr++;
+	return ret;
 }
 
 static int get_extended_base_var(struct strbuf *name, int c)

@@ -1232,11 +1232,36 @@ int git_config_with_options(config_fn_t fn, void *data,
 	return ret;
 }
 
-int git_config(config_fn_t fn, void *data)
+static int git_config_raw(config_fn_t fn, void *data)
 {
 	return git_config_with_options(fn, data, NULL, 1);
 }
 
+static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+	int i;
+	struct string_list *strptr;
+	struct config_set_element *entry;
+	struct hashmap_iter iter;
+	hashmap_iter_init(&cs->config_hash, &iter);
+	while ((entry = hashmap_iter_next(&iter))) {
+		strptr = &entry->value_list;
+		for (i = 0; i < strptr->nr; i++) {
+			if (fn(entry->key, strptr->items[i].string, data) < 0)
+				die("bad config file line in (TODO: file/line info)");
+		}
+	}
+	return 0;
+}
+
+static void git_config_check_init(void);
+
+int git_config(config_fn_t fn, void *data)
+{
+	git_config_check_init();
+	return configset_iter(&the_config_set, fn, data);
+}
+
 static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
 {
 	struct config_set_element k;

@@ -1418,7 +1443,7 @@ static void git_config_check_init(void)
 	if (the_config_set.hash_initialized)
 		return;
 	git_configset_init(&the_config_set);
-	git_config(config_set_callback, &the_config_set);
+	git_config_raw(config_set_callback, &the_config_set);
 }
 
 void git_config_clear(void)

@@ -197,4 +197,12 @@ test_expect_success 'proper error on error in custom config files' '
 	test_cmp expect actual
 '
 
+test_expect_success 'check line errors for malformed values' '
+	cp .git/config .git/config.old &&
+	test_when_finished "mv .git/config.old .git/config" &&
+	echo "[alias]\n br" >.git/config &&
+	test_expect_code 128 git br 2>result &&
+	grep "fatal: bad config file line 2 in .git/config" result
+'
+
 test_done

@@ -1237,9 +1237,15 @@ static int git_config_raw(config_fn_t fn, void *data)
 	return git_config_with_options(fn, data, NULL, 1);
 }
 
+struct key_value_info {
+	const char *filename;
+	int linenr;
+};
+
 static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 {
 	int i;
+	struct key_value_info *kv_info;
 	struct string_list *strptr;
 	struct config_set_element *entry;
 	struct hashmap_iter iter;

@@ -1247,8 +1253,15 @@ static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 	while ((entry = hashmap_iter_next(&iter))) {
 		strptr = &entry->value_list;
 		for (i = 0; i < strptr->nr; i++) {
-			if (fn(entry->key, strptr->items[i].string, data) < 0)
-				die("bad config file line in (TODO: file/line info)");
+			if (fn(entry->key, strptr->items[i].string, data) < 0) {
+				kv_info = strptr->items[i].util;
+				if (!kv_info->linenr)
+					die("unable to parse command-line config");
+				else
+					die("bad config file line %d in %s",
+						kv_info->linenr,
+						kv_info->filename);
+			}
 		}
 	}
 	return 0;

@@ -1287,6 +1300,8 @@ static struct config_set_element *configset_find_element(struct config_set *cs,
 static int configset_add_value(struct config_set *cs, const char *key, const char *value)
 {
 	struct config_set_element *e;
+	struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+	struct string_list_item *si;
 	e = configset_find_element(cs, key);
 	/*
 	 * Since the keys are being fed by git_config*() callback mechanism, they

@@ -1299,7 +1314,16 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
 		string_list_init(&e->value_list, 1);
 		hashmap_add(&cs->config_hash, e);
 	}
-	string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);
+	si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);
+	if (cf) {
+		kv_info->filename = strintern(cf->name);
+		kv_info->linenr = cf->linenr;
+	} else {
+		/* for values read from `git_config_from_parameters()` */
+		kv_info->filename = strintern("parameter");
+		kv_info->linenr = 0;
+	}
+	si->util = kv_info;
 
 	return 0;
 }

@@ -1326,7 +1350,7 @@ void git_configset_clear(struct config_set *cs)
 	hashmap_iter_init(&cs->config_hash, &iter);
 	while ((entry = hashmap_iter_next(&iter))) {
 		free(entry->key);
-		string_list_clear(&entry->value_list, 0);
+		string_list_clear(&entry->value_list, 1);
 	}
 	hashmap_free(&cs->config_hash, 1);
 	cs->hash_initialized = 0;

@@ -119,6 +119,15 @@ test_expect_success 'find integer value for a key' '
 	check_config get_int lamb.chop 65
 '
 
+test_expect_success 'find string value for a key' '
+	check_config get_string case.baz hask
+'
+
+test_expect_success 'check line error when NULL string is queried' '
+	test_expect_code 128 test-config get_string case.foo 2>result &&
+	grep "fatal: bad config file line 7 in .git/config" result
+'
+
 test_expect_success 'find integer if value is non parse-able' '
 	check_config expect_code 128 get_int lamb.head
 '

@@ -16,6 +16,8 @@
  *
  * get_bool -> print bool value for the entered key or die
  *
+ * get_string -> print string value for the entered key or die
+ *
  * configset_get_value -> returns value with the highest priority for the entered key
  * 			from a config_set constructed from files entered as arguments.
  *

@@ -84,6 +86,14 @@ int main(int argc, char **argv)
 			printf("Value not found for \"%s\"\n", argv[2]);
 			goto exit1;
 		}
+	} else if (argc == 3 && !strcmp(argv[1], "get_string")) {
+		if (!git_config_get_string(argv[2], &v)) {
+			printf("%s\n", v);
+			goto exit0;
+		} else {
+			printf("Value not found for \"%s\"\n", argv[2]);
+			goto exit1;
+		}
 	} else if (!strcmp(argv[1], "configset_get_value")) {
 		for (i = 3; i < argc; i++) {
 			int err;

@@ -150,6 +150,11 @@ as well as retrieval for the queried variable, including:
 	Similar to `git_config_get_string`, but expands `~` or `~user` into
 	the user's home directory when found at the beginning of the path.
 
+`void git_die_config(const char *key)`::
+
+	Dies printing the line number and the file name of the highest
+	priority value for the configuration variable `key`.
+
 See test-config.c for usage examples.
 
 Value Parsing Helpers

@@ -1380,6 +1380,7 @@ extern int git_config_get_bool(const char *key, int *dest);
 extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);
 extern int git_config_get_maybe_bool(const char *key, int *dest);
 extern int git_config_get_pathname(const char *key, const char **dest);
+extern void git_die_config(const char *key);
 
 extern int committer_ident_sufficiently_given(void);
 extern int author_ident_sufficiently_given(void);

@@ -1491,8 +1491,12 @@ const struct string_list *git_config_get_value_multi(const char *key)
 
 int git_config_get_string(const char *key, const char **dest)
 {
+	int ret;
 	git_config_check_init();
-	return git_configset_get_string(&the_config_set, key, dest);
+	ret = git_configset_get_string(&the_config_set, key, dest);
+	if (ret < 0)
+		git_die_config(key);
+	return ret;
 }
 
 int git_config_get_int(const char *key, int *dest)

@@ -1527,8 +1531,24 @@ int git_config_get_maybe_bool(const char *key, int *dest)
 
 int git_config_get_pathname(const char *key, const char **dest)
 {
+	int ret;
 	git_config_check_init();
-	return git_configset_get_pathname(&the_config_set, key, dest);
+	ret = git_configset_get_pathname(&the_config_set, key, dest);
+	if (ret < 0)
+		git_die_config(key);
+	return ret;
+}
+
+void git_die_config(const char *key)
+{
+	const struct string_list *strptr;
+	struct key_value_info *kv_info;
+	strptr = git_config_get_value_multi(key);
+	kv_info = strptr->items[strptr->nr - 1].util;
+	if (!kv_info->linenr)
+		die("unable to parse command-line config");
+	else
+		die("bad config file line %d in %s",kv_info->linenr, kv_info->filename);
 }
 
 /*

@@ -2,6 +2,7 @@
 #include "userdiff.h"
 #include "cache.h"
 #include "attr.h"
+#include "string-list.h"
 
 static struct userdiff_driver *drivers;
 static int ndrivers;

@@ -211,9 +212,12 @@ int userdiff_config(const char *k, const char *v)
 	struct userdiff_driver *drv;
 	const char *name, *type;
 	int namelen;
+	char *subsection = NULL;
+	static struct string_list xflag = STRING_LIST_INIT_DUP;
 
 	if (parse_config_key(k, "diff", &name, &namelen, &type) || !name)
 		return 0;
+	subsection = xstrndup(name, namelen);
 
 	drv = userdiff_find_by_namelen(name, namelen);
 	if (!drv) {

@@ -224,10 +228,16 @@ int userdiff_config(const char *k, const char *v)
 		drv->binary = -1;
 	}
 
-	if (!strcmp(type, "funcname"))
+	if (!strcmp(type, "funcname") && !unsorted_string_list_has_string(&xflag, subsection)) {
+		free (subsection);
 		return parse_funcname(&drv->funcname, k, v, 0);
-	if (!strcmp(type, "xfuncname"))
+	}
+	if (!strcmp(type, "xfuncname")) {
+		string_list_append(&xflag, subsection);
+		free (subsection);
 		return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
+	}
+	free(subsection);
 	if (!strcmp(type, "binary"))
 		return parse_tristate(&drv->binary, k, v);
 	if (!strcmp(type, "command"))