Commit 04f57d51605ffe624aab37919c03e50a5ae3b16a

Patrick Steinhardt 2018-08-10T13:33:02

config_entries: pull out implementation of entry store The configuration entry store that is used for configuration files needs to keep track of all entries in two different structures: - a singly linked list is being used to be able to iterate through configuration files in the order they have been found - a string map is being used to efficiently look up configuration entries by their key This store is thus something that may be used by other, future backends as well to abstract away implementation details and iteration over the entries. Pull out the necessary functions from "config_file.c" and moves them into their own "config_entries.c" module. For now, this is simply moving over code without any renames and/or refactorings to help reviewing.

diff --git a/src/config_entries.c b/src/config_entries.c
new file mode 100644
index 0000000..644e60c
--- /dev/null
+++ b/src/config_entries.c
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "config_entries.h"
+
+static void config_entry_list_free(config_entry_list *list)
+{
+	config_entry_list *next;
+
+	while (list != NULL) {
+		next = list->next;
+
+		git__free((char*) list->entry->name);
+		git__free((char *) list->entry->value);
+		git__free(list->entry);
+		git__free(list);
+
+		list = next;
+	};
+}
+
+static void config_entry_list_append(config_entry_list **list, config_entry_list *entry)
+{
+	if (*list)
+		(*list)->last->next = entry;
+	else
+		*list = entry;
+	(*list)->last = entry;
+}
+
+int diskfile_entries_alloc(diskfile_entries **out)
+{
+	diskfile_entries *entries;
+	int error;
+
+	entries = git__calloc(1, sizeof(diskfile_entries));
+	GITERR_CHECK_ALLOC(entries);
+
+	git_atomic_set(&entries->refcount, 1);
+
+	if ((error = git_strmap_alloc(&entries->map)) < 0)
+		git__free(entries);
+	else
+		*out = entries;
+
+	return error;
+}
+
+void diskfile_entries_free(diskfile_entries *entries)
+{
+	config_entry_list *list = NULL, *next;
+
+	if (!entries)
+		return;
+
+	if (git_atomic_dec(&entries->refcount) != 0)
+		return;
+
+	git_strmap_foreach_value(entries->map, list, config_entry_list_free(list));
+	git_strmap_free(entries->map);
+
+	list = entries->list;
+	while (list != NULL) {
+		next = list->next;
+		git__free(list);
+		list = next;
+	}
+
+	git__free(entries);
+}
+
+int diskfile_entries_append(diskfile_entries *entries, git_config_entry *entry)
+{
+	git_strmap_iter pos;
+	config_entry_list *existing, *var;
+	int error = 0;
+
+	var = git__calloc(1, sizeof(config_entry_list));
+	GITERR_CHECK_ALLOC(var);
+	var->entry = entry;
+
+	pos = git_strmap_lookup_index(entries->map, entry->name);
+	if (!git_strmap_valid_index(entries->map, pos)) {
+		/*
+		 * We only ever inspect `last` from the first config
+		 * entry in a multivar. In case where this new entry is
+		 * the first one in the entry map, it will also be the
+		 * last one at the time of adding it, which is
+		 * why we set `last` here to itself. Otherwise we
+		 * do not have to set `last` and leave it set to
+		 * `NULL`.
+		 */
+		var->last = var;
+
+		git_strmap_insert(entries->map, entry->name, var, &error);
+
+		if (error > 0)
+			error = 0;
+	} else {
+		existing = git_strmap_value_at(entries->map, pos);
+		config_entry_list_append(&existing, var);
+	}
+
+	var = git__calloc(1, sizeof(config_entry_list));
+	GITERR_CHECK_ALLOC(var);
+	var->entry = entry;
+	config_entry_list_append(&entries->list, var);
+
+	return error;
+}
+
+void config_iterator_free(
+	git_config_iterator* iter)
+{
+	iter->backend->free(iter->backend);
+	git__free(iter);
+}
+
+int config_iterator_next(
+	git_config_entry **entry,
+	git_config_iterator *iter)
+{
+	git_config_file_iter *it = (git_config_file_iter *) iter;
+
+	if (!it->head)
+		return GIT_ITEROVER;
+
+	*entry = it->head->entry;
+	it->head = it->head->next;
+
+	return 0;
+}
diff --git a/src/config_entries.h b/src/config_entries.h
new file mode 100644
index 0000000..fbb901b
--- /dev/null
+++ b/src/config_entries.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "common.h"
+
+#include "git2/sys/config.h"
+#include "config.h"
+
+typedef struct config_entry_list {
+	struct config_entry_list *next;
+	struct config_entry_list *last;
+	git_config_entry *entry;
+} config_entry_list;
+
+typedef struct {
+	git_atomic refcount;
+	git_strmap *map;
+	config_entry_list *list;
+} diskfile_entries;
+
+typedef struct git_config_file_iter {
+	git_config_iterator parent;
+	config_entry_list *head;
+} git_config_file_iter;
+
+int diskfile_entries_alloc(diskfile_entries **out);
+void diskfile_entries_free(diskfile_entries *entries);
+/* Add or append the new config option */
+int diskfile_entries_append(diskfile_entries *entries, git_config_entry *entry);
+
+void config_iterator_free(git_config_iterator* iter);
+int config_iterator_next(git_config_entry **entry,
+	git_config_iterator *iter);
diff --git a/src/config_file.c b/src/config_file.c
index a14358d..1934ec2 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -17,32 +17,16 @@
 #include "strmap.h"
 #include "array.h"
 #include "config_parse.h"
+#include "config_entries.h"
 
 #include <ctype.h>
 #include <sys/types.h>
 #include <regex.h>
 
-typedef struct config_entry_list {
-	struct config_entry_list *next;
-	struct config_entry_list *last;
-	git_config_entry *entry;
-} config_entry_list;
-
-typedef struct git_config_file_iter {
-	git_config_iterator parent;
-	config_entry_list *head;
-} git_config_file_iter;
-
 /* Max depth for [include] directives */
 #define MAX_INCLUDE_DEPTH 10
 
 typedef struct {
-	git_atomic refcount;
-	git_strmap *map;
-	config_entry_list *list;
-} diskfile_entries;
-
-typedef struct {
 	git_config_backend parent;
 	/* mutex to coordinate accessing the values */
 	git_mutex values_mutex;
@@ -89,95 +73,6 @@ static int config_error_readonly(void)
 	return -1;
 }
 
-static void config_entry_list_free(config_entry_list *list)
-{
-	config_entry_list *next;
-
-	while (list != NULL) {
-		next = list->next;
-
-		git__free((char*) list->entry->name);
-		git__free((char *) list->entry->value);
-		git__free(list->entry);
-		git__free(list);
-
-		list = next;
-	};
-}
-
-static void config_entry_list_append(config_entry_list **list, config_entry_list *entry)
-{
-	if (*list)
-		(*list)->last->next = entry;
-	else
-		*list = entry;
-	(*list)->last = entry;
-}
-
-/* Add or append the new config option */
-static int diskfile_entries_append(diskfile_entries *entries, git_config_entry *entry)
-{
-	git_strmap_iter pos;
-	config_entry_list *existing, *var;
-	int error = 0;
-
-	var = git__calloc(1, sizeof(config_entry_list));
-	GITERR_CHECK_ALLOC(var);
-	var->entry = entry;
-
-	pos = git_strmap_lookup_index(entries->map, entry->name);
-	if (!git_strmap_valid_index(entries->map, pos)) {
-		/*
-		 * We only ever inspect `last` from the first config
-		 * entry in a multivar. In case where this new entry is
-		 * the first one in the entry map, it will also be the
-		 * last one at the time of adding it, which is
-		 * why we set `last` here to itself. Otherwise we
-		 * do not have to set `last` and leave it set to
-		 * `NULL`.
-		 */
-		var->last = var;
-
-		git_strmap_insert(entries->map, entry->name, var, &error);
-
-		if (error > 0)
-			error = 0;
-	} else {
-		existing = git_strmap_value_at(entries->map, pos);
-		config_entry_list_append(&existing, var);
-	}
-
-	var = git__calloc(1, sizeof(config_entry_list));
-	GITERR_CHECK_ALLOC(var);
-	var->entry = entry;
-	config_entry_list_append(&entries->list, var);
-
-	return error;
-}
-
-static void diskfile_entries_free(diskfile_entries *entries)
-{
-	config_entry_list *list = NULL, *next;
-
-	if (!entries)
-		return;
-
-	if (git_atomic_dec(&entries->refcount) != 0)
-		return;
-
-	git_strmap_foreach_value(entries->map, list, config_entry_list_free(list));
-	git_strmap_free(entries->map);
-
-	list = entries->list;
-	while (list != NULL) {
-		next = list->next;
-		git__free(list);
-		list = next;
-	}
-
-	git__free(entries);
-}
-
 /**
  * Take the current values map from the backend and increase its
  * refcount. This is its own function to make sure we use the mutex to
@@ -200,24 +95,6 @@ static diskfile_entries *diskfile_entries_take(diskfile_header *h)
 	return entries;
 }
 
-static int diskfile_entries_alloc(diskfile_entries **out)
-{
-	diskfile_entries *entries;
-	int error;
-
-	entries = git__calloc(1, sizeof(diskfile_entries));
-	GITERR_CHECK_ALLOC(entries);
-
-	git_atomic_set(&entries->refcount, 1);
-
-	if ((error = git_strmap_alloc(&entries->map)) < 0)
-		git__free(entries);
-	else
-		*out = entries;
-
-	return error;
-}
-
 static void config_file_clear(struct config_file *file)
 {
 	struct config_file *include;
@@ -348,28 +225,6 @@ static void backend_free(git_config_backend *_backend)
 	git__free(backend);
 }
 
-static void config_iterator_free(
-	git_config_iterator* iter)
-{
-	iter->backend->free(iter->backend);
-	git__free(iter);
-}
-
-static int config_iterator_next(
-	git_config_entry **entry,
-	git_config_iterator *iter)
-{
-	git_config_file_iter *it = (git_config_file_iter *) iter;
-
-	if (!it->head)
-		return GIT_ITEROVER;
-
-	*entry = it->head->entry;
-	it->head = it->head->next;
-
-	return 0;
-}
-
 static int config_iterator_new(
 	git_config_iterator **iter,
 	struct git_config_backend* backend)