config_entries: refactor entries iterator memory ownership

Right now, the config file code requires us to pass in its backend to
the config entry iterator. This is required with the current code, as
the config file backend will first create a read-only snapshot which is
then passed to the iterator just for that purpose. So after the iterator
is getting free'd, the code needs to make sure that the snapshot gets
free'd, as well.

By now, though, we can easily refactor the code to be more efficient and
remove the reverse dependency from iterator to backend. Instead of
creating a read-only snapshot (which also requires us to re-parse the
complete configuration file), we can simply duplicate the config entries
and pass those to the iterator. Like that, the iterator only needs to
make sure to free the duplicated config entries, which is trivial to do
and clears up memory ownership by a lot.

3 files changed, 53 insertions, 13 deletions

diff --git a/src/config_entries.c b/src/config_entries.c
index 2e02c31dc..fccce2773 100644
--- a/src/config_entries.c
+++ b/src/config_entries.c
@@ -15,6 +15,7 @@ typedef struct config_entry_list {
 
 typedef struct config_entries_iterator {
 	git_config_iterator parent;
+	git_config_entries *entries;
 	config_entry_list *head;
 } config_entries_iterator;
 
@@ -66,6 +67,40 @@ int git_config_entries_new(git_config_entries **out)
 	return error;
 }
 
+int git_config_entries_dup(git_config_entries **out, git_config_entries *entries)
+{
+	git_config_entries *result = NULL;
+	config_entry_list *head;
+	int error;
+
+	if ((error = git_config_entries_new(&result)) < 0)
+		goto out;
+
+	for (head = entries->list; head; head = head->next) {
+		git_config_entry *dup;
+
+		dup = git__calloc(1, sizeof(git_config_entry));
+		dup->name = git__strdup(head->entry->name);
+		GITERR_CHECK_ALLOC(dup->name);
+		if (head->entry->value) {
+			dup->value = git__strdup(head->entry->value);
+			GITERR_CHECK_ALLOC(dup->value);
+		}
+		dup->level = head->entry->level;
+		dup->include_depth = head->entry->include_depth;
+
+		if ((error = git_config_entries_append(result, dup)) < 0)
+			goto out;
+	}
+
+	*out = result;
+	result = NULL;
+
+out:
+	git_config_entries_free(result);
+	return error;
+}
+
 void git_config_entries_incref(git_config_entries *entries)
 {
 	GIT_REFCOUNT_INC(entries);
@@ -184,11 +219,11 @@ int git_config_entries_get_unique(git_config_entry **out, git_config_entries *en
 	return 0;
 }
 
-void config_iterator_free(
-	git_config_iterator* iter)
+void config_iterator_free(git_config_iterator *iter)
 {
-	iter->backend->free(iter->backend);
-	git__free(iter);
+	config_entries_iterator *it = (config_entries_iterator *) iter;
+	git_config_entries_free(it->entries);
+	git__free(it);
 }
 
 int config_iterator_next(
@@ -206,17 +241,18 @@ int config_iterator_next(
 	return 0;
 }
 
-int git_config_entries_iterator_new(git_config_iterator **out, git_config_backend *backend, git_config_entries *entries)
+int git_config_entries_iterator_new(git_config_iterator **out, git_config_entries *entries)
 {
 	config_entries_iterator *it;
 
 	it = git__calloc(1, sizeof(config_entries_iterator));
 	GITERR_CHECK_ALLOC(it);
-	it->parent.backend = backend;
-	it->head = entries->list;
 	it->parent.next = config_iterator_next;
 	it->parent.free = config_iterator_free;
+	it->head = entries->list;
+	it->entries = entries;
 
+	git_config_entries_incref(entries);
 	*out = &it->parent;
 
 	return 0;
diff --git a/src/config_entries.h b/src/config_entries.h
index 02292e27a..6fdbc41ba 100644
--- a/src/config_entries.h
+++ b/src/config_entries.h
@@ -13,10 +13,11 @@
 typedef struct git_config_entries git_config_entries;
 
 int git_config_entries_new(git_config_entries **out);
+int git_config_entries_dup(git_config_entries **out, git_config_entries *entries);
 void git_config_entries_incref(git_config_entries *entries);
 void git_config_entries_free(git_config_entries *entries);
 /* Add or append the new config option */
 int git_config_entries_append(git_config_entries *entries, git_config_entry *entry);
 int git_config_entries_get(git_config_entry **out, git_config_entries *entries, const char *key);
 int git_config_entries_get_unique(git_config_entry **out, git_config_entries *entries, const char *key);
-int git_config_entries_iterator_new(git_config_iterator **out, git_config_backend *backend, git_config_entries *entries);
+int git_config_entries_iterator_new(git_config_iterator **out, git_config_entries *entries);
diff --git a/src/config_file.c b/src/config_file.c
index 7a7f35f06..e8740d35f 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -229,17 +229,20 @@ static int config_iterator_new(
 	git_config_iterator **iter,
 	struct git_config_backend* backend)
 {
-	git_config_backend *snapshot;
 	diskfile_header *bh = (diskfile_header *) backend;
+	git_config_entries *entries;
 	int error;
 
-	if ((error = config_snapshot(&snapshot, backend)) < 0)
+	if ((error = git_config_entries_dup(&entries, bh->entries)) < 0)
 		return error;
 
-	if ((error = snapshot->open(snapshot, bh->level, bh->repo)) < 0)
-		return error;
+	if ((error = git_config_entries_iterator_new(iter, entries)) < 0)
+		goto out;
 
-	return git_config_entries_iterator_new(iter, snapshot, bh->entries);
+out:
+	/* Let iterator delete duplicated entries when it's done */
+	git_config_entries_free(entries);
+	return error;
 }
 
 static int config_set(git_config_backend *cfg, const char *name, const char *value)