Hi Josh
On 09/08/2024 23:41, Josh Steadmon wrote:
Add git_configset_alloc() and git_configset_clear_and_free() functions
so that callers can manage config_set structs on the heap. This also
allows non-C external consumers to treat config_sets as opaque structs.
Do we really need to add this code to config.c rather than handling it
in the wrapper layer in the next patch?
Looking ahead I wonder how useful it is to users of the library to
separate out allocation from initialization. A function that allocates
and initializes a configset would be more convenient and harder to
misuse. Calling release functions *_free() rather than
*_clear_and_free() would be more convenient as well. I also noticed that
the data types are not namespaced when they are exported. So perhaps we
could drop this patch and add the following to the next patch.
/*
* Namespace data types as well as functions and ensure consistent
* naming of data types and the functions that operate on them.
*/
struct libgit_configset {
struct config_set set;
};
struct libgit_configset *libgit_configset_new() {
struct libgit_configset *set = xmalloc(sizeof(*set));
git_configset_init(&set->set);
return set;
}
void libgit_configset_free(struct libgit_configset *set) {
git_configset_clear(&set->set);
free(set);
}
Best Wishes
Phillip
Co-authored-by: Calvin Wan <calvinwan@xxxxxxxxxx>
Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx>
Signed-off-by: Josh Steadmon <steadmon@xxxxxxxxxx>
---
config.c | 11 +++++++++++
config.h | 10 ++++++++++
2 files changed, 21 insertions(+)
diff --git a/config.c b/config.c
index 6421894614..444db8de79 100644
--- a/config.c
+++ b/config.c
@@ -2324,6 +2324,11 @@ static int config_set_element_cmp(const void *cmp_data UNUSED,
return strcmp(e1->key, e2->key);
}
+struct config_set *git_configset_alloc(void)
+{
+ return xmalloc(sizeof(struct config_set));
+}
+
void git_configset_init(struct config_set *set)
{
hashmap_init(&set->config_hash, config_set_element_cmp, NULL, 0);
@@ -2353,6 +2358,12 @@ void git_configset_clear(struct config_set *set)
set->list.items = NULL;
}
+void git_configset_clear_and_free(struct config_set *set)
+{
+ git_configset_clear(set);
+ free(set);
+}
+
static int config_set_callback(const char *key, const char *value,
const struct config_context *ctx,
void *cb)
diff --git a/config.h b/config.h
index 54b47dec9e..074c85a788 100644
--- a/config.h
+++ b/config.h
@@ -472,6 +472,11 @@ struct config_set {
struct configset_list list;
};
+/**
+ * Alloc a config_set
+ */
+struct config_set *git_configset_alloc(void);
+
/**
* Initializes the config_set `cs`.
*/
@@ -520,6 +525,11 @@ int git_configset_get_string_multi(struct config_set *cs, const char *key,
*/
void git_configset_clear(struct config_set *cs);
+/**
+ * Clears and frees a heap-allocated `config_set` structure.
+ */
+void git_configset_clear_and_free(struct config_set *cs);
+
/*
* These functions return 1 if not found, and 0 if found, leaving the found
* value in the 'dest' pointer.
