config: don't special-case the multivar iterator

Build it on top of the normal iterator instead, which lets use re-use
a lot of code.

include/git2/config.h

@@ -350,7 +350,7 @@ GIT_EXTERN(int) git_config_get_multivar_foreach(const git_config *cfg, const cha
  * @param regexp regular expression to filter which variables we're
  * interested in. Use NULL to indicate all
  */
-GIT_EXTERN(int) git_config_get_multivar(git_config_iterator **out, const git_config *cfg, const char *name, const char *regexp);
+GIT_EXTERN(int) git_config_multivar_iterator_new(git_config_iterator **out, const git_config *cfg, const char *name, const char *regexp);
 
 /**
  * Return the current entry and advance the iterator

include/git2/sys/config.h

@@ -58,8 +58,6 @@ struct git_config_backend {
 	/* Open means open the file/database and parse if necessary */
 	int (*open)(struct git_config_backend *, git_config_level_t level);
 	int (*get)(const struct git_config_backend *, const char *key, const git_config_entry **entry);
-	int (*get_multivar_foreach)(struct git_config_backend *, const char *key, const char *regexp, git_config_foreach_cb callback, void *payload);
-	int (*get_multivar)(git_config_iterator **, struct git_config_backend *, const char *name, const char *regexp);
 	int (*set)(struct git_config_backend *, const char *key, const char *value);
 	int (*set_multivar)(git_config_backend *cfg, const char *name, const char *regexp, const char *value);
 	int (*del)(struct git_config_backend *, const char *key);

src/config.c

@@ -747,7 +747,7 @@ int git_config_get_multivar_foreach(
 	git_config_iterator *iter;
 	git_config_entry *entry;
 
-	if ((err = git_config_get_multivar(&iter, cfg, name, regexp)) < 0)
+	if ((err = git_config_multivar_iterator_new(&iter, cfg, name, regexp)) < 0)
 		return err;
 
 	found = 0;
@@ -771,92 +771,82 @@ int git_config_get_multivar_foreach(
 
 typedef struct {
 	git_config_iterator parent;
-	git_config_iterator *current;
+	git_config_iterator *iter;
 	char *name;
-	char *regexp;
-	const git_config *cfg;
-	size_t i;
+	regex_t regex;
+	int have_regex;
 } multivar_iter;
 
 static int multivar_iter_next(git_config_entry **entry, git_config_iterator *_iter)
 {
 	multivar_iter *iter = (multivar_iter *) _iter;
-	git_config_iterator *current = iter->current;
-	file_internal *internal;
-	git_config_backend *backend;
-	size_t i;
 	int error = 0;
 
-	if (current != NULL &&
-	    (error = current->next(entry, current)) == 0) {
-		return 0;
-	}
-
-	if (error < 0 && error != GIT_ITEROVER)
-		return error;
-
-	do {
-		if (find_next_backend(&i, iter->cfg, iter->i) < 0)
-			return GIT_ITEROVER;
-
-		internal = git_vector_get(&iter->cfg->files, i - 1);
-		backend = internal->file;
-		iter->i = i - 1;
-
-		if (iter->current)
-			iter->current->free(current);
-
-		iter->current = NULL;
-		error = backend->get_multivar(&iter->current, backend, iter->name, iter->regexp);
-		if (error == GIT_ENOTFOUND)
+	while ((error = iter->iter->next(entry, iter->iter)) == 0) {
+		if (git__strcmp(iter->name, (*entry)->name))
 			continue;
 
-		if (error < 0)
-			return error;
-
-		return iter->current->next(entry, iter->current);
+		if (!iter->have_regex)
+			return 0;
 
-	} while(1);
+		if (regexec(&iter->regex, (*entry)->value, 0, NULL, 0) == 0)
+			return 0;
+	}
 
-	return GIT_ITEROVER;
+	return error;
 }
 
 void multivar_iter_free(git_config_iterator *_iter)
 {
 	multivar_iter *iter = (multivar_iter *) _iter;
 
-	if (iter->current)
-		iter->current->free(iter->current);
+	iter->iter->free(iter->iter);
 
 	git__free(iter->name);
-	git__free(iter->regexp);
+	regfree(&iter->regex);
 	git__free(iter);
 }
 
-int git_config_get_multivar(git_config_iterator **out, const git_config *cfg, const char *name, const char *regexp)
+int git_config_multivar_iterator_new(git_config_iterator **out, const git_config *cfg, const char *name, const char *regexp)
 {
-	multivar_iter *iter;
+	multivar_iter *iter = NULL;
+	git_config_iterator *inner = NULL;
+	int error;
+
+	if ((error = git_config_iterator_new(&inner, cfg)) < 0)
+		return error;
 
 	iter = git__calloc(1, sizeof(multivar_iter));
 	GITERR_CHECK_ALLOC(iter);
 
-	iter->name = git__strdup(name);
-	GITERR_CHECK_ALLOC(iter->name);
+	if ((error = git_config__normalize_name(name, &iter->name)) < 0)
+		goto on_error;
 
 	if (regexp != NULL) {
-		iter->regexp = git__strdup(regexp);
-		GITERR_CHECK_ALLOC(iter->regexp);
+		error = regcomp(&iter->regex, regexp, REG_EXTENDED);
+		if (error < 0) {
+			giterr_set_regex(&iter->regex, error);
+			error = -1;
+			regfree(&iter->regex);
+			goto on_error;
+		}
+
+		iter->have_regex = 1;
 	}
 
+	iter->iter = inner;
 	iter->parent.free = multivar_iter_free;
 	iter->parent.next = multivar_iter_next;
 
-	iter->i = cfg->files.length;
-	iter->cfg = cfg;
-
 	*out = (git_config_iterator *) iter;
 
 	return 0;
+
+on_error:
+
+	inner->free(inner);
+	git__free(iter);
+	return error;
 }
 
 int git_config_set_multivar(git_config *cfg, const char *name, const char *regexp, const char *value)
@@ -1125,6 +1115,41 @@ fail_parse:
 	return -1;
 }
 
+/* Take something the user gave us and make it nice for our hash function */
+int git_config__normalize_name(const char *in, char **out)
+{
+	char *name, *fdot, *ldot;
+
+	assert(in && out);
+
+	name = git__strdup(in);
+	GITERR_CHECK_ALLOC(name);
+
+	fdot = strchr(name, '.');
+	ldot = strrchr(name, '.');
+
+	if (fdot == NULL || fdot == name || ldot == NULL || !ldot[1])
+		goto invalid;
+
+	/* Validate and downcase up to first dot and after last dot */
+	if (git_config_file_normalize_section(name, fdot) < 0 ||
+		git_config_file_normalize_section(ldot + 1, NULL) < 0)
+		goto invalid;
+
+	/* If there is a middle range, make sure it doesn't have newlines */
+	while (fdot < ldot)
+		if (*fdot++ == '\n')
+			goto invalid;
+
+	*out = name;
+	return 0;
+
+invalid:
+	git__free(name);
+	giterr_set(GITERR_CONFIG, "Invalid config item name '%s'", in);
+	return GIT_EINVALIDSPEC;
+}
+
 struct rename_data {
 	git_config *config;
 	git_buf *name;

src/config.h

@@ -49,4 +49,7 @@ extern int git_config_rename_section(
  */
 extern int git_config_file__ondisk(struct git_config_backend **out, const char *path);
 
+extern int git_config__normalize_name(const char *in, char **out);
+
+
 #endif

src/config_file.c

@@ -136,41 +136,6 @@ int git_config_file_normalize_section(char *start, char *end)
 	return 0;
 }
 
-/* Take something the user gave us and make it nice for our hash function */
-static int normalize_name(const char *in, char **out)
-{
-	char *name, *fdot, *ldot;
-
-	assert(in && out);
-
-	name = git__strdup(in);
-	GITERR_CHECK_ALLOC(name);
-
-	fdot = strchr(name, '.');
-	ldot = strrchr(name, '.');
-
-	if (fdot == NULL || fdot == name || ldot == NULL || !ldot[1])
-		goto invalid;
-
-	/* Validate and downcase up to first dot and after last dot */
-	if (git_config_file_normalize_section(name, fdot) < 0 ||
-		git_config_file_normalize_section(ldot + 1, NULL) < 0)
-		goto invalid;
-
-	/* If there is a middle range, make sure it doesn't have newlines */
-	while (fdot < ldot)
-		if (*fdot++ == '\n')
-			goto invalid;
-
-	*out = name;
-	return 0;
-
-invalid:
-	git__free(name);
-	giterr_set(GITERR_CONFIG, "Invalid config item name '%s'", in);
-	return GIT_EINVALIDSPEC;
-}
-
 static void free_vars(git_strmap *values)
 {
 	cvar_t *var = NULL;
@@ -314,7 +279,7 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val
 	khiter_t pos;
 	int rval, ret;
 
-	if ((rval = normalize_name(name, &key)) < 0)
+	if ((rval = git_config__normalize_name(name, &key)) < 0)
 		return rval;
 
 	/*
@@ -397,7 +362,7 @@ static int config_get(const git_config_backend *cfg, const char *name, const git
 	khiter_t pos;
 	int error;
 
-	if ((error = normalize_name(name, &key)) < 0)
+	if ((error = git_config__normalize_name(name, &key)) < 0)
 		return error;
 
 	pos = git_strmap_lookup_index(b->values, key);
@@ -412,162 +377,6 @@ static int config_get(const git_config_backend *cfg, const char *name, const git
 	return 0;
 }
 
-typedef struct {
-	git_config_iterator parent;
-	cvar_t *var;
-	regex_t regex;
-	int have_regex;
-} foreach_iter;
-
-static void foreach_iter_free(git_config_iterator *_iter)
-{
-	foreach_iter *iter = (foreach_iter *) _iter;
-
-	if (iter->have_regex)
-		regfree(&iter->regex);
-
-	git__free(iter);
-}
-
-static int foreach_iter_next(git_config_entry **out, git_config_iterator *_iter)
-{
-	foreach_iter *iter = (foreach_iter *) _iter;
-
-	cvar_t* var = iter->var;
-
-
-	if (var == NULL)
-		return GIT_ITEROVER;
-
-	if (!iter->have_regex) {
-		*out = var->entry;
-		iter->var = var->next;
-		return 0;
-	}
-
-	/* For the regex case, we must loop until we find something we like */
-	do {
-		git_config_entry *entry = var->entry;
-		regex_t *regex = &iter->regex;;
-		if (regexec(regex, entry->value, 0, NULL, 0) == 0) {
-			*out = entry;
-			iter->var = var->next;
-			return 0;
-		}
-		var = var->next;
-	} while(var != NULL);
-
-	return GIT_ITEROVER;
-}
-
-static int config_get_multivar(git_config_iterator **out, git_config_backend *_backend,
-			       const char *name, const char *regexp)
-{
-	foreach_iter *iter;
-	diskfile_backend *b = (diskfile_backend *) _backend;
-
-	char *key;
-	khiter_t pos;
-	int error = 0;
-
-	if ((error = normalize_name(name, &key)) < 0)
-		return error;
-
-	pos = git_strmap_lookup_index(b->values, key);
-	git__free(key);
-
-	if (!git_strmap_valid_index(b->values, pos))
-		return GIT_ENOTFOUND;
-
-	iter = git__calloc(1, sizeof(foreach_iter));
-	GITERR_CHECK_ALLOC(iter);
-
-	iter->var = git_strmap_value_at(b->values, pos);
-
-	if (regexp != NULL) {
-		int result;
-
-		result = regcomp(&iter->regex, regexp, REG_EXTENDED);
-		if (result < 0) {
-			giterr_set_regex(&iter->regex, result);
-			regfree(&iter->regex);
-			return -1;
-		}
-		iter->have_regex = 1;
-	}
-
-	iter->parent.free = foreach_iter_free;
-	iter->parent.next = foreach_iter_next;
-
-	*out = (git_config_iterator *) iter;
-
-	return 0;
- }
-
-static int config_get_multivar_foreach(
-	git_config_backend *cfg,
-	const char *name,
-	const char *regex_str,
-	int (*fn)(const git_config_entry *, void *),
-	void *data)
-{
-	cvar_t *var;
-	diskfile_backend *b = (diskfile_backend *)cfg;
-	char *key;
-	khiter_t pos;
-	int error;
-
-	if ((error = normalize_name(name, &key)) < 0)
-		return error;
-
-	pos = git_strmap_lookup_index(b->values, key);
-	git__free(key);
-
-	if (!git_strmap_valid_index(b->values, pos))
-		return GIT_ENOTFOUND;
-
-	var = git_strmap_value_at(b->values, pos);
-
-	if (regex_str != NULL) {
-		regex_t regex;
-		int result;
-
-		/* regex matching; build the regex */
-		result = regcomp(&regex, regex_str, REG_EXTENDED);
-		if (result < 0) {
-			giterr_set_regex(&regex, result);
-			regfree(&regex);
-			return -1;
-		}
-
-		/* and throw the callback only on the variables that
-		 * match the regex */
-		do {
-			if (regexec(&regex, var->entry->value, 0, NULL, 0) == 0) {
-				/* early termination by the user is not an error;
-				 * just break and return successfully */
-				if (fn(var->entry, data))
-					break;
-			}
-
-			var = var->next;
-		} while (var != NULL);
-		regfree(&regex);
-	} else {
-		/* no regex; go through all the variables */
-		do {
-			/* early termination by the user is not an error;
-			 * just break and return successfully */
-			if (fn(var->entry, data) < 0)
-				break;
-
-			var = var->next;
-		} while (var != NULL);
-	}
-
-	return 0;
-}
-
 static int config_set_multivar(
 	git_config_backend *cfg, const char *name, const char *regexp, const char *value)
 {
@@ -581,7 +390,7 @@ static int config_set_multivar(
 
 	assert(regexp);
 
-	if ((result = normalize_name(name, &key)) < 0)
+	if ((result = git_config__normalize_name(name, &key)) < 0)
 		return result;
 
 	pos = git_strmap_lookup_index(b->values, key);
@@ -654,7 +463,7 @@ static int config_delete(git_config_backend *cfg, const char *name)
 	int result;
 	khiter_t pos;
 
-	if ((result = normalize_name(name, &key)) < 0)
+	if ((result = git_config__normalize_name(name, &key)) < 0)
 		return result;
 
 	pos = git_strmap_lookup_index(b->values, key);
@@ -694,8 +503,6 @@ int git_config_file__ondisk(git_config_backend **out, const char *path)
 
 	backend->parent.open = config_open;
 	backend->parent.get = config_get;
-	backend->parent.get_multivar_foreach = config_get_multivar_foreach;
-	backend->parent.get_multivar = config_get_multivar;
 	backend->parent.set = config_set;
 	backend->parent.set_multivar = config_set_multivar;
 	backend->parent.del = config_delete;

tests-clar/config/multivar.c

@@ -76,7 +76,7 @@ static void check_get_multivar(git_config *cfg, int expected)
 	git_config_entry *entry;
 	int n = 0;
 
-	cl_git_pass(git_config_get_multivar(&iter, cfg, _name, NULL));
+	cl_git_pass(git_config_multivar_iterator_new(&iter, cfg, _name, NULL));
 
 	while (git_config_next(&entry, iter) == 0)
 		n++;