Commit b78f4ab082030bc02ee2e1a51eca441bfb6f1e8f
2018-08-16T12:22:03
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138
diff --git a/src/config_entries.c b/src/config_entries.c
index 2e02c31..fccce27 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 02292e2..6fdbc41 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 7a7f35f..e8740d3 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)