Skip to content

Commit b78f4ab

Browse files
committed
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 parent d49b136 commit b78f4ab

File tree

3 files changed

+53
-13
lines changed

3 files changed

+53
-13
lines changed

src/config_entries.c

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ typedef struct config_entry_list {
1515

1616
typedef struct config_entries_iterator {
1717
git_config_iterator parent;
18+
git_config_entries *entries;
1819
config_entry_list *head;
1920
} config_entries_iterator;
2021

@@ -66,6 +67,40 @@ int git_config_entries_new(git_config_entries **out)
6667
return error;
6768
}
6869

70+
int git_config_entries_dup(git_config_entries **out, git_config_entries *entries)
71+
{
72+
git_config_entries *result = NULL;
73+
config_entry_list *head;
74+
int error;
75+
76+
if ((error = git_config_entries_new(&result)) < 0)
77+
goto out;
78+
79+
for (head = entries->list; head; head = head->next) {
80+
git_config_entry *dup;
81+
82+
dup = git__calloc(1, sizeof(git_config_entry));
83+
dup->name = git__strdup(head->entry->name);
84+
GITERR_CHECK_ALLOC(dup->name);
85+
if (head->entry->value) {
86+
dup->value = git__strdup(head->entry->value);
87+
GITERR_CHECK_ALLOC(dup->value);
88+
}
89+
dup->level = head->entry->level;
90+
dup->include_depth = head->entry->include_depth;
91+
92+
if ((error = git_config_entries_append(result, dup)) < 0)
93+
goto out;
94+
}
95+
96+
*out = result;
97+
result = NULL;
98+
99+
out:
100+
git_config_entries_free(result);
101+
return error;
102+
}
103+
69104
void git_config_entries_incref(git_config_entries *entries)
70105
{
71106
GIT_REFCOUNT_INC(entries);
@@ -184,11 +219,11 @@ int git_config_entries_get_unique(git_config_entry **out, git_config_entries *en
184219
return 0;
185220
}
186221

187-
void config_iterator_free(
188-
git_config_iterator* iter)
222+
void config_iterator_free(git_config_iterator *iter)
189223
{
190-
iter->backend->free(iter->backend);
191-
git__free(iter);
224+
config_entries_iterator *it = (config_entries_iterator *) iter;
225+
git_config_entries_free(it->entries);
226+
git__free(it);
192227
}
193228

194229
int config_iterator_next(
@@ -206,17 +241,18 @@ int config_iterator_next(
206241
return 0;
207242
}
208243

209-
int git_config_entries_iterator_new(git_config_iterator **out, git_config_backend *backend, git_config_entries *entries)
244+
int git_config_entries_iterator_new(git_config_iterator **out, git_config_entries *entries)
210245
{
211246
config_entries_iterator *it;
212247

213248
it = git__calloc(1, sizeof(config_entries_iterator));
214249
GITERR_CHECK_ALLOC(it);
215-
it->parent.backend = backend;
216-
it->head = entries->list;
217250
it->parent.next = config_iterator_next;
218251
it->parent.free = config_iterator_free;
252+
it->head = entries->list;
253+
it->entries = entries;
219254

255+
git_config_entries_incref(entries);
220256
*out = &it->parent;
221257

222258
return 0;

src/config_entries.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
typedef struct git_config_entries git_config_entries;
1414

1515
int git_config_entries_new(git_config_entries **out);
16+
int git_config_entries_dup(git_config_entries **out, git_config_entries *entries);
1617
void git_config_entries_incref(git_config_entries *entries);
1718
void git_config_entries_free(git_config_entries *entries);
1819
/* Add or append the new config option */
1920
int git_config_entries_append(git_config_entries *entries, git_config_entry *entry);
2021
int git_config_entries_get(git_config_entry **out, git_config_entries *entries, const char *key);
2122
int git_config_entries_get_unique(git_config_entry **out, git_config_entries *entries, const char *key);
22-
int git_config_entries_iterator_new(git_config_iterator **out, git_config_backend *backend, git_config_entries *entries);
23+
int git_config_entries_iterator_new(git_config_iterator **out, git_config_entries *entries);

src/config_file.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,20 @@ static int config_iterator_new(
229229
git_config_iterator **iter,
230230
struct git_config_backend* backend)
231231
{
232-
git_config_backend *snapshot;
233232
diskfile_header *bh = (diskfile_header *) backend;
233+
git_config_entries *entries;
234234
int error;
235235

236-
if ((error = config_snapshot(&snapshot, backend)) < 0)
236+
if ((error = git_config_entries_dup(&entries, bh->entries)) < 0)
237237
return error;
238238

239-
if ((error = snapshot->open(snapshot, bh->level, bh->repo)) < 0)
240-
return error;
239+
if ((error = git_config_entries_iterator_new(iter, entries)) < 0)
240+
goto out;
241241

242-
return git_config_entries_iterator_new(iter, snapshot, bh->entries);
242+
out:
243+
/* Let iterator delete duplicated entries when it's done */
244+
git_config_entries_free(entries);
245+
return error;
243246
}
244247

245248
static int config_set(git_config_backend *cfg, const char *name, const char *value)

0 commit comments

Comments
 (0)