8000 config: remove `free` ptr from `git_config_entry` · libgit2/libgit2@bd72fd5 · GitHub
[go: up one dir, main page]
Skip to content

Commit bd72fd5

Browse files
ethomsonethomson
committed
config: remove free ptr from git_config_entry
This is a leftover leaky abstraction. If consumers aren't meant to _call_ the `free` function then they shouldn't _see_ the free function. Move it out into a `git_config_backend_entry` that is, well, produced by the config backends. This makes our code messier but is an improvement for consumers.
1 parent 6423ffb commit bd72fd5

File tree

8 files changed

+94
-70
lines changed

8 files changed

+94
-70
lines changed

‎include/git2/config.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,6 @@ typedef struct git_config_entry {
9797

9898
/** Configuration level for the file this was found in */
9999
git_config_level_t level;
100-
101-
/**
102-
* Free function for this entry; for internal purposes. Callers
103-
* should call `git_config_entry_free` to free data.
104-
*/
105-
void GIT_CALLBACK(free)(struct git_config_entry *entry);
106100
} git_config_entry;
107101

108102
/**

‎include/git2/sys/config.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,16 @@
2020
*/
2121
GIT_BEGIN_DECL
2222

23+
typedef struct git_config_backend_entry {
24+
struct git_config_entry entry;
25+
26+
/**
27+
* Free function for this entry; for internal purposes. Callers
28+
* should call `git_config_entry_free` to free data.
29+
*/
30+
void GIT_CALLBACK(free)(struct git_config_backend_entry *entry);
31+
} git_config_backend_entry;
32+
2333
/**
2434
* Every iterator must have this struct as its first element, so the
2535
* API can talk to it. You'd define your iterator as
@@ -39,7 +49,7 @@ struct git_config_iterator {
3949
* Return the current entry and advance the iterator. The
4050
* memory belongs to the library.
4151
*/
42-
int GIT_CALLBACK(next)(git_config_entry **entry, git_config_iterator *iter);
52+
int GIT_CALLBACK(next)(git_config_backend_entry **entry, git_config_iterator *iter);
4353

4454
/**
4555
* Free the iterator
@@ -59,7 +69,7 @@ struct git_config_backend {
5969

6070
/* Open means open the file/database and parse if necessary */
6171
int GIT_CALLBACK(open)(struct git_config_backend *, git_config_level_t level, const git_repository *repo);
62-
int GIT_CALLBACK(get)(struct git_config_backend *, const char *key, git_config_entry **entry);
72+
int GIT_CALLBACK(get)(struct git_config_backend *, const char *key, git_config_backend_entry **entry);
6373
int GIT_CALLBACK(set)(struct git_config_backend *, const char *key, const char *value);
6474
int GIT_CALLBACK(set_multivar)(git_config_backend *cfg, const char *name, const char *regexp, const char *value);
6575
int GIT_CALLBACK(del)(struct git_config_backend *, const char *key);

‎src/libgit2/config.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,13 @@ typedef struct {
5050

5151
void git_config_entry_free(git_config_entry *entry)
5252
{
53+
git_config_backend_entry *be;
54+
5355
if (!entry)
5456
return;
5557

56-
entry->free(entry);
58+
be = (git_config_backend_entry *)entry;
59+
be->free(be);
5760
}
5861

5962
static void backend_instance_free(backend_instance *instance)
@@ -435,10 +438,12 @@ static int all_iter_next(git_config_entry **out, git_config_iterator *_iter)
435438
all_iter *iter = (all_iter *) _iter;
436439
backend_entry *entry;
437440
git_config_backend *backend;
441+
git_config_backend_entry *be;
438442
int error = 0;
439443

440444
if (iter->current != NULL &&
441-
(error = iter->current->next(out, iter->current)) == 0) {
445+
(error = iter->current->next(&be, iter->current)) == 0) {
446+
*out = &be->entry;
442447
return 0;
443448
}
444449

@@ -460,13 +465,18 @@ static int all_iter_next(git_config_entry **out, git_config_iterator *_iter)
460465

461466
iter->current = NULL;
462467
error = backend->iterator(&iter->current, backend);
468+
463469
if (error == GIT_ENOTFOUND)
464470
continue;
465471

466472
if (error < 0)
467473
return error;
468474

469-
error = iter->current->next(out, iter->current);
475+
if ((error = iter->current->next(&be, iter->current)) == 0) {
476+
*out = &be->entry;
477+
return 0;
478+
}
479+
470480
/* If this backend is empty, then keep going */
471481
if (error == GIT_ITEROVER)
472482
continue;

‎src/libgit2/config_backend.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,14 @@ GIT_INLINE(void) git_config_backend_free(git_config_backend *cfg)
5151
GIT_INLINE(int) git_config_backend_get_string(
5252
git_config_entry **out, git_config_backend *cfg, const char *name)
5353
{
54-
return cfg->get(cfg, name, out);
54+
git_config_backend_entry *be;
55+
int error;
56+
57+
if ((error = cfg->get(cfg, name, &be)) < 0)
58+
return error;
59+
60+
*out = &be->entry;
61+
return 0;
5562
}
5663

5764
GIT_INLINE(int) git_config_backend_set_string(

‎src/libgit2/config_file.c

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,8 @@ static int config_file_set(git_config_backend *cfg, const char *name, const char
310310
if (error != GIT_ENOTFOUND)
311311
goto out;
312312
error = 0;
313-
} else if ((!existing->base.value && !value) ||
314-
(existing->base.value && value && !strcmp(existing->base.value, value))) {
313+
} else if ((!existing->base.entry.value && !value) ||
314+
(existing->base.entry.value && value && !strcmp(existing->base.entry.value, value))) {
315315
/* don't update if old and new values already match */
316316
error = 0;
317317
goto out;
@@ -336,7 +336,10 @@ static int config_file_set(git_config_backend *cfg, const char *name, const char
336336
/*
337337
* Internal function that actually gets the value in string form
338338
*/
339-
static int config_file_get(git_config_backend *cfg, const char *key, git_config_entry **out)
339+
static int config_file_get(
340+
git_config_backend *cfg,
341+
const char *key,
342+
git_config_backend_entry **out)
340343
{
341344
config_file_backend *h = GIT_CONTAINER_OF(cfg, config_file_backend, parent);
342345
git_config_list *config_list = NULL;
@@ -407,7 +410,7 @@ static int config_file_delete(git_config_backend *cfg, const char *name)
407410
goto out;
408411
}
409412

410-
if ((error = config_file_write(b, name, entry->base.name, NULL, NULL)) < 0)
413+
if ((error = config_file_write(b, name, entry->base.entry.name, NULL, NULL)) < 0)
411414
goto out;
412415

413416
out:
@@ -795,22 +798,22 @@ static int read_on_variable(
795798
entry = git__calloc(1, sizeof(git_config_list_entry));
796799
GIT_ERROR_CHECK_ALLOC(entry);
797800

798-
entry->base.name = git_str_detach(&buf);
799-
GIT_ERROR_CHECK_ALLOC(entry->base.name);
801+
entry->base.entry.name = git_str_detach(&buf);
802+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.name);
800803

801804
if (var_value) {
802-
entry->base.value = git__strdup(var_value);
803-
GIT_ERROR_CHECK_ALLOC(entry->base.value);
805+
entry->base.entry.value = git__strdup(var_value);
806+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.value);
804807
}
805808

806-
entry->base.backend_type = git_config_list_add_string(parse_data->config_list, CONFIG_FILE_TYPE);
807-
GIT_ERROR_CHECK_ALLOC(entry->base.backend_type);
809+
entry->base.entry.backend_type = git_config_list_add_string(parse_data->config_list, CONFIG_FILE_TYPE);
810+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.backend_type);
808811

809-
entry->base.origin_path = git_config_list_add_string(parse_data->config_list, parse_data->file->path);
810-
GIT_ERROR_CHECK_ALLOC(entry->base.origin_path);
812+
entry->base.entry.origin_path = git_config_list_add_string(parse_data->config_list, parse_data->file->path);
813+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.origin_path);
811814

812-
entry->base.level = parse_data->level;
813-
entry->base.include_depth = parse_data->depth;
815+
entry->base.entry.level = parse_data->level;
816+
entry->base.entry.include_depth = parse_data->depth;
814817
entry->base.free = git_config_list_entry_free;
815818
entry->config_list = parse_data->config_list;
816819

@@ -820,11 +823,11 @@ static int read_on_variable(
820823
result = 0;
821824

822825
/* Add or append the new config option */
823-
if (!git__strcmp(entry->base.name, "include.path"))
824-
result = parse_include(parse_data, entry->base.value);
825-
else if (!git__prefixcmp(entry->base.name, "includeif.") &&
826-
!git__suffixcmp(entry->base.name, ".path"))
827-
result = parse_conditional_include(parse_data, entry->base.name, entry->base.value);
826+
if (!git__strcmp(entry->base.entry.name, "include.path"))
827+
result = parse_include(parse_data, entry->base.entry.value);
828+
else if (!git__prefixcmp(entry->base.entry.name, "includeif.") &&
829+
!git__suffixcmp(entry->base.entry.name, ".path"))
830+
result = parse_conditional_include(parse_data, entry->base.entry.name, entry->base.entry.value);
828831

829832
return result;
830833
}

‎src/libgit2/config_list.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,24 @@ int git_config_list_dup_entry(git_config_list *config_list, const git_config_ent
6464
duplicated = git__calloc(1, sizeof(git_config_list_entry));
6565
GIT_ERROR_CHECK_ALLOC(duplicated);
6666

67-
duplicated->base.name = git__strdup(entry->name);
68-
GIT_ERROR_CHECK_ALLOC(duplicated->base.name);
67+
duplicated->base.entry.name = git__strdup(entry->name);
68+
GIT_ERROR_CHECK_ALLOC(duplicated->base.entry.name);
6969

7070
if (entry->value) {
71-
duplicated->base.value = git__strdup(entry->value);
72-
GIT_ERROR_CHECK_ALLOC(duplicated->base.value);
71+
duplicated->base.entry.value = git__strdup(entry->value);
72+
GIT_ERROR_CHECK_ALLOC(duplicated->base.entry.value);
7373
}
7474

75-
duplicated->base.backend_type = git_config_list_add_string(config_list, entry->backend_type);
76-
GIT_ERROR_CHECK_ALLOC(duplicated->base.backend_type);
75+
duplicated->base.entry.backend_type = git_config_list_add_string(config_list, entry->backend_type);
76+
GIT_ERROR_CHECK_ALLOC(duplicated->base.entry.backend_type);
7777

7878
if (entry->origin_path) {
79-
duplicated->base.origin_path = git_config_list_add_string(config_list, entry->origin_path);
80-
GIT_ERROR_CHECK_ALLOC(duplicated->base.origin_path);
79+
duplicated->base.entry.origin_path = git_config_list_add_string(config_list, entry->origin_path);
80+
GIT_ERROR_CHECK_ALLOC(duplicated->base.entry.origin_path);
8181
}
8282

83-
duplicated->base.level = entry->level;
84-
duplicated->base.include_depth = entry->include_depth;
83+
duplicated->base.entry.level = entry->level;
84+
duplicated->base.entry.include_depth = entry->include_depth;
8585
duplicated->base.free = git_config_list_entry_free;
8686
duplicated->config_list = config_list;
8787

@@ -90,8 +90,8 @@ int git_config_list_dup_entry(git_config_list *config_list, const git_config_ent
9090

9191
out:
9292
if (error && duplicated) {
93-
git__free((char *) duplicated->base.name);
94-
git__free((char *) duplicated->base.value);
93+
git__free((char *) duplicated->base.entry.name);
94+
git__free((char *) duplicated->base.entry.value);
9595
git__free(duplicated);
9696
}
9797
return error;
@@ -107,7 +107,7 @@ int git_config_list_dup(git_config_list **out, git_config_list *config_list)
107107
goto out;
108108

109109
for (head = config_list->entries; head; head = head->next)
110-
if ((git_config_list_dup_entry(result, &head->entry->base)) < 0)
110+
if ((git_config_list_dup_entry(result, &head->entry->base.entry)) < 0)
111111
goto out;
112112

113113
*out = result;
@@ -135,15 +135,15 @@ static void config_list_free(git_config_list *config_list)
135135
git_strmap_free(config_list->strings);
136136

137137
git_strmap_foreach_value(config_list->map, head, {
138-
git__free((char *) head->entry->base.name);
138+
git__free((char *) head->entry->base.entry.name);
139139
git__free(head);
140140
});
141141
git_strmap_free(config_list->map);
142142

143143
entry_list = config_list->entries;
144144
while (entry_list != NULL) {
145145
next = entry_list->next;
146-
git__free((char *) entry_list->entry->base.value);
146+
git__free((char *) entry_list->entry->base.entry.value);
147147
git__free(entry_list->entry);
148148
git__free(entry_list);
149149
entry_list = next;
@@ -163,19 +163,19 @@ int git_config_list_append(git_config_list *config_list, git_config_list_entry *
163163
config_entry_list *list_head;
164164
config_entry_map_head *map_head;
165165

166-
if ((map_head = git_strmap_get(config_list->map, entry->base.name)) != NULL) {
166+
if ((map_head = git_strmap_get(config_list->map, entry->base.entry.name)) != NULL) {
167167
map_head->multivar = true;
168168
/*
169169
* This is a micro-optimization for configuration files
170170
* with a lot of same keys. As for multivars the entry's
171171
* key will be the same for all list, we can just free
172172
* all except the first entry's name and just re-use it.
173173
*/
174-
git__free((char *) entry->base.name);
175-
entry->base.name = map_head->entry->base.name;
174+
git__free((char *) entry->base.entry.name);
175+
entry->base.entry.name = map_head->entry->base.entry.name;
176176
} else {
177177
map_head = git__calloc(1, sizeof(*map_head));
178-
if ((git_strmap_set(config_list->map, entry->base.name, map_head)) < 0)
178+
if ((git_strmap_set(config_list->map, entry->base.entry.name, map_head)) < 0)
179179
return -1;
180180
}
181181
map_head->entry = entry;
@@ -216,7 +216,7 @@ int git_config_list_get_unique(git_config_list_entry **out, git_config_list *con
216216
return -1;
217217
}
218218

219-
if (entry->entry->base.include_depth) {
219+
if (entry->entry->base.entry.include_depth) {
220220
git_error_set(GIT_ERROR_CONFIG, "entry is not unique due to being included");
221221
return -1;
222222
}
@@ -233,7 +233,7 @@ static void config_iterator_free(git_config_iterator *iter)
233233
}
234234

235235
static int config_iterator_next(
236-
git_config_entry **entry,
236+
git_config_backend_entry **entry,
237237
git_config_iterator *iter)
238238
{
239239
config_list_iterator *it = (config_list_iterator *) iter;
@@ -265,7 +265,7 @@ int git_config_list_iterator_new(git_config_iterator **out, git_config_list *con
265265
}
266266

267267
/* release the map containing the entry as an equivalent to freeing it */
268-
void git_config_list_entry_free(git_config_entry *e)
268+
void git_config_list_entry_free(git_config_backend_entry *e)
269269
{
270270
git_config_list_entry *entry = (git_config_list_entry *)e;
271271
git_config_list_free(entry->config_list);

‎src/libgit2/config_list.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
typedef struct git_config_list git_config_list;
1414

1515
typedef struct {
16-
git_config_entry base;
16+
git_config_backend_entry base;
1717
git_config_list *config_list;
1818
} git_config_list_entry;
1919

@@ -29,4 +29,4 @@ int git_config_list_get_unique(git_config_list_entry **out, git_config_list *lis
2929
int git_config_list_iterator_new(git_config_iterator **out, git_config_list *list);
3030
const char *git_config_list_add_string(git_config_list *list, const char *str);
3131

32-
void git_config_list_entry_free(git_config_entry *entry);
32+
void git_config_list_entry_free(git_config_backend_entry *entry);

‎src/libgit2/config_mem.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ static int read_variable_cb(
7777

7878
entry = git__calloc(1, sizeof(git_config_list_entry));
7979
GIT_ERROR_CHECK_ALLOC(entry);
80-
entry->base.name = git_str_detach(&buf);
81-
entry->base.value = var_value ? git__strdup(var_value) : NULL;
82-
entry->base.level = parse_data->level;
83-
entry->base.include_depth = 0;
84-
entry->base.backend_type = parse_data->backend_type;
85-
entry->base.origin_path = parse_data->origin_path;
80+
entry->base.entry.name = git_str_detach(&buf);
81+
entry->base.entry.value = var_value ? git__strdup(var_value) : NULL;
82+
entry->base.entry.level = parse_data->level;
83+
entry->base.entry.include_depth = 0;
84+
entry->base.entry.backend_type = parse_data->backend_type;
85+
entry->base.entry.origin_path = parse_data->origin_path;
8686
entry->base.free = git_config_list_entry_free;
8787
entry->config_list = parse_data->config_list;
8888

@@ -151,18 +151,18 @@ static int parse_values(
151151
entry = git__calloc(1, sizeof(git_config_list_entry));
152152
GIT_ERROR_CHECK_ALLOC(entry);
153153

154-
entry->base.name = git__strndup(memory_backend->values[i], name_len);
155-
GIT_ERROR_CHECK_ALLOC(entry->base.name);
154+
entry->base.entry.name = git__strndup(memory_backend->values[i], name_len);
155+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.name);
156156

157157
if (eql) {
158-
entry->base.value = git__strdup(eql + 1);
159-
GIT_ERROR_CHECK_ALLOC(entry->base.value);
158+
entry->base.entry.value = git__strdup(eql + 1);
159+
GIT_ERROR_CHECK_ALLOC(entry->base.entry.value);
160160
}
161161

162-
entry->base.level = level;
163-
entry->base.include_depth = 0;
164-
entry->base.backend_type = backend_type;
165-
entry->base.origin_path = origin_path;
162+
entry->base.entry.level = level;
163+
entry->base.entry.include_depth = 0;
164+
entry->base.entry.backend_type = backend_type;
165+
entry->base.entry.origin_path = origin_path;
166166
entry->base.free = git_config_list_entry_free;
167167
entry->config_list = memory_backend->config_list;
168168

@@ -190,7 +190,7 @@ static int config_memory_open(git_config_backend *backend, git_config_level_t le
190190
return 0;
191191
}
192192

193-
static int config_memory_get(git_config_backend *backend, const char *key, git_config_entry **out)
193+
static int config_memory_get(git_config_backend < 49CA span class="pl-c1">*backend, const char *key, git_config_backend_entry **out)
194194
{
195195
config_memory_backend *memory_backend = (config_memory_backend *) backend;
196196
git_config_list_entry *entry;

0 commit comments

Comments
 (0)
0