config: working multivar iterator

Implement the foreach version as a wrapper around the iterator.

include/git2/config.h

@@ -341,6 +341,16 @@ GIT_EXTERN(int) git_config_get_string(const char **out, const git_config *cfg, c
  */
 GIT_EXTERN(int) git_config_get_multivar_foreach(const git_config *cfg, const char *name, const char *regexp, git_config_foreach_cb callback, void *payload);
 
+/**
+ * Get each value of a multivar
+ *
+ * @param out pointer to store the iterator
+ * @param cfg where to look for the variable
+ * @param name the variable's name
+ * @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);
 /**
  * Set the value of an integer config variable in the config file
  * with the highest level (usually the local one).

include/git2/sys/config.h

@@ -39,7 +39,7 @@ struct git_config_iterator {
 	 * Return the current entry and advance the iterator. The
 	 * memory belongs to the library.
 	 */
-	int (*next)(git_config_entry *entry, git_config_iterator *iter);
+	int (*next)(git_config_entry **entry, git_config_iterator *iter);
 
 	/**
 	 * Free the iterator

src/config.c

@@ -327,7 +327,7 @@ int git_config_backend_foreach_match(
 	int (*fn)(const git_config_entry *, void *),
 	void *data)
 {
-	git_config_entry entry;
+	git_config_entry *entry;
 	git_config_iterator* iter;
 	regex_t regex;
 	int result = 0;
@@ -347,11 +347,11 @@ int git_config_backend_foreach_match(
 
 	while(!(iter->next(&entry, iter) < 0)) {
 		/* skip non-matching keys if regexp was provided */
-		if (regexp && regexec(&regex, entry.name, 0, NULL, 0) != 0)
+		if (regexp && regexec(&regex, entry->name, 0, NULL, 0) != 0)
 			continue;
 
 		/* abort iterator on non-zero return value */
-		if (fn(&entry, data)) {
+		if (fn(entry, data)) {
 			giterr_clear();
 			result = GIT_EUSER;
 			goto cleanup;
@@ -578,35 +578,36 @@ int git_config_get_multivar_foreach(
 	const git_config *cfg, const char *name, const char *regexp,
 	git_config_foreach_cb cb, void *payload)
 {
-	file_internal *internal;
-	git_config_backend *file;
-	int ret = GIT_ENOTFOUND, err;
-	size_t i;
+	int err, found;
+	git_config_iterator *iter;
+	git_config_entry *entry;
 
-	/*
-	 * This loop runs the "wrong" way 'round because we need to
-	 * look at every value from the most general to most specific
-	 */
-	for (i = cfg->files.length; i > 0; --i) {
-		internal = git_vector_get(&cfg->files, i - 1);
-		if (!internal || !internal->file)
-			continue;
-		file = internal->file;
-
-		if (!(err = file->get_multivar_foreach(file, name, regexp, cb, payload)))
-			ret = 0;
-		else if (err != GIT_ENOTFOUND)
+	if ((err = git_config_get_multivar(&iter, cfg, name, regexp)) < 0)
 		return err;
+
+	found = 0;
+	while ((err = iter->next(&entry, iter)) == 0) {
+		found = 1;
+		if(cb(entry, payload)) {
+			iter->free(iter);
+			return GIT_EUSER;
+		}
 	}
 
-	return (ret == GIT_ENOTFOUND) ? config_error_notfound(name) : 0;
+	if (err == GIT_ITEROVER)
+		err = 0;
+
+	if (found == 0 && err == 0)
+		err = config_error_notfound(name);
+
+	return err;
 }
 
 typedef struct {
 	git_config_iterator parent;
 	git_config_iterator *current;
-	const char *name;
-	const char *regexp;
+	char *name;
+	char *regexp;
 	const git_config *cfg;
 	size_t i;
 } multivar_iter;
@@ -627,7 +628,7 @@ static int find_next_backend(size_t *out, const git_config *cfg, size_t i)
 	return -1;
 }
 
-static int multivar_iter_next_empty(git_config_entry *entry, git_config_iterator *_iter)
+static int multivar_iter_next_empty(git_config_entry **entry, git_config_iterator *_iter)
 {
 	GIT_UNUSED(entry);
 	GIT_UNUSED(_iter);
@@ -635,20 +636,21 @@ static int multivar_iter_next_empty(git_config_entry *entry, git_config_iterator
 	return GIT_ITEROVER;
 }
 
-static int multivar_iter_next(git_config_entry *entry, git_config_iterator *_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;
+	int error = 0;
 
 	if (current != NULL &&
-	    (error = current->next(entry, current)) == 0)
+	    (error = current->next(entry, current)) == 0) {
 		return 0;
+	}
 
-	if (error != GIT_ITEROVER)
+	if (error < 0 && error != GIT_ITEROVER)
 		return error;
 
 	do {
@@ -657,10 +659,15 @@ static int multivar_iter_next(git_config_entry *entry, git_config_iterator *_ite
 
 		internal = git_vector_get(&iter->cfg->files, i - 1);
 		backend = internal->file;
-		if ((error = backend->get_multivar(&iter->current, backend, iter->name, iter->regexp)) < 0)
-			return -1;
+		iter->i = i - 1;
+
+		error = backend->get_multivar(&iter->current, backend, iter->name, iter->regexp);
+		if (error == GIT_ENOTFOUND)
+			continue;
+
+		if (error < 0)
+			return error;
 
-		iter->i = i;
 		return iter->current->next(entry, iter->current);
 
 	} while(1);
@@ -668,6 +675,15 @@ static int multivar_iter_next(git_config_entry *entry, git_config_iterator *_ite
 	return GIT_ITEROVER;
 }
 
+void multivar_iter_free(git_config_iterator *_iter)
+{
+	multivar_iter *iter = (multivar_iter *) _iter;
+
+	git__free(iter->name);
+	git__free(iter->regexp);
+	git__free(iter);
+}
+
 int git_config_get_multivar(git_config_iterator **out, const git_config *cfg, const char *name, const char *regexp)
 {
 	multivar_iter *iter;
@@ -676,6 +692,15 @@ int git_config_get_multivar(git_config_iterator **out, const git_config *cfg, co
 	iter = git__calloc(1, sizeof(multivar_iter));
 	GITERR_CHECK_ALLOC(iter);
 
+	iter->name = git__strdup(name);
+	GITERR_CHECK_ALLOC(iter->name);
+
+	if (regexp != NULL) {
+		iter->regexp = git__strdup(regexp);
+		GITERR_CHECK_ALLOC(iter->regexp);
+	}
+
+	iter->parent.free = multivar_iter_free;
 	if (find_next_backend(&i, cfg, cfg->files.length) < 0)
 		iter->parent.next = multivar_iter_next_empty;
 	else
@@ -683,8 +708,6 @@ int git_config_get_multivar(git_config_iterator **out, const git_config *cfg, co
 
 	iter->i = cfg->files.length;
 	iter->cfg = cfg;
-	iter->name = name;
-	iter->regexp = regexp;
 
 	*out = (git_config_iterator *) iter;
 

src/config_file.c

@@ -261,7 +261,7 @@ static void config_iterator_free(
 }
 
 static int config_iterator_next(
-	git_config_entry *entry,
+	git_config_entry **entry,
 	git_config_iterator *iter)
 {
 	git_config_file_iter *it = (git_config_file_iter *) iter;
@@ -282,9 +282,7 @@ static int config_iterator_next(
 		return -1;
 	}
 
-	entry->name = key;
-	entry->value = var->entry->value;
-	entry->level = var->entry->level;
+	*entry = var->entry;
 	it->next_var = CVAR_LIST_NEXT(var);
 
 	return 0;
@@ -433,19 +431,18 @@ static void foreach_iter_free(git_config_iterator *_iter)
 	git__free(iter);
 }
 
-static int foreach_iter_next(git_config_entry *out, git_config_iterator *_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->name = var->entry->name;
-		out->value = var->entry->value;
-
+		*out = var->entry;
 		iter->var = var->next;
 		return 0;
 	}
@@ -455,10 +452,11 @@ static int foreach_iter_next(git_config_entry *out, git_config_iterator *_iter)
 		git_config_entry *entry = var->entry;
 		regex_t *regex = &iter->regex;;
 		if (regexec(regex, entry->value, 0, NULL, 0) == 0) {
-			out->name = entry->name;
-			out->value = entry->value;
+			*out = entry;
+			iter->var = var->next;
 			return 0;
 		}
+		var = var->next;
 	} while(var != NULL);
 
 	return GIT_ITEROVER;
@@ -550,7 +548,7 @@ static int config_get_multivar_foreach(
 			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) < 0)
+				if (fn(var->entry, data))
 					break;
 			}
 

tests-clar/config/multivar.c

@@ -114,11 +114,11 @@ void test_config_multivar__add(void)
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, NULL, cb, &n));
-	cl_assert(n == 3);
+	cl_assert_equal_i(n, 3);
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, "otherplace", cb, &n));
-	cl_assert(n == 1);
+	cl_assert_equal_i(n, 1);
 
 	git_config_free(cfg);
 
@@ -128,11 +128,11 @@ void test_config_multivar__add(void)
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, NULL, cb, &n));
-	cl_assert(n == 3);
+	cl_assert_equal_i(n, 3);
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, "otherplace", cb, &n));
-	cl_assert(n == 1);
+	cl_assert_equal_i(n, 1);
 
 	git_config_free(cfg);
 }
@@ -148,7 +148,7 @@ void test_config_multivar__add_new(void)
 	cl_git_pass(git_config_set_multivar(cfg, var, "", "variable"));
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, var, NULL, cb, &n));
-	cl_assert(n == 1);
+	cl_assert_equal_i(n, 1);
 
 	git_config_free(cfg);
 }
@@ -191,7 +191,7 @@ void test_config_multivar__replace_multiple(void)
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, "otherplace", cb, &n));
-	cl_assert(n == 2);
+	cl_assert_equal_i(n, 2);
 
 	git_config_free(cfg);
 
@@ -199,7 +199,7 @@ void test_config_multivar__replace_multiple(void)
 
 	n = 0;
 	cl_git_pass(git_config_get_multivar_foreach(cfg, _name, "otherplace", cb, &n));
-	cl_assert(n == 2);
+	cl_assert_equal_i(n, 2);
 
 	git_config_free(cfg);
 }