From 4873f46bab3dffb6ec3b46348d034c4cd459aae9 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sun, 11 Dec 2022 12:27:52 -0800 Subject: [PATCH] cgraph: pass destructor as a macro parameter instead of struct member This smoothes down a number of rough edges in the generic list implementation: 1. No need to noisily cast e.g. `free` as a destructor at the initialization site. 2. The generic list struct is reduced by the size of a function pointer. This is not significant when a list is used entirely locally as the compiler can destructure this type as an optimization. But when passing objects of this type between functions, this can help. 3. Some functions like free of the generic list zeroed the structure, losing the destructor. This could be improved by retaining it, but it seemed simpler to remove the opportunity for this confusion altogether. An alternative to creating the new `DEFINE_LIST_WITH_DTOR` macro would be to implement `DEFINE_LIST` as a variadic macro, taking an optional third parameter. However the gymnastics required to achieve this in C99 (search the internet for macro argument counting) make it not worth it. --- contrib/prune/prune.c | 8 ++++---- lib/cgraph/list.h | 36 +++++++++++++++++++++++------------- lib/cgraph/test_list.c | 4 ++-- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/contrib/prune/prune.c b/contrib/prune/prune.c index 4f0b20856..be3bf1784 100644 --- a/contrib/prune/prune.c +++ b/contrib/prune/prune.c @@ -31,8 +31,8 @@ static void free_strattr(strattr_t s) { free(s.v); } -DEFINE_LIST(attrs, strattr_t) -DEFINE_LIST(nodes, char*) +DEFINE_LIST_WITH_DTOR(attrs, strattr_t, free_strattr) +DEFINE_LIST_WITH_DTOR(nodes, char*, free) static int remove_child(Agraph_t * graph, Agnode_t * node); static void help_message(const char *progname); @@ -84,8 +84,8 @@ int main(int argc, char **argv) progname++; /* character after last '/' */ } - attrs_t attr_list = {.dtor = free_strattr}; - nodes_t node_list = {.dtor = (void(*)(char*))free}; + attrs_t attr_list = {0}; + nodes_t node_list = {0}; while ((c = getopt(argc, argv, "hvn:N:")) != -1) { switch (c) { diff --git a/lib/cgraph/list.h b/lib/cgraph/list.h index a6dba2daa..c8da91a13 100644 --- a/lib/cgraph/list.h +++ b/lib/cgraph/list.h @@ -16,7 +16,15 @@ * \param name Type name to give the list container * \param type Type of the elements the list will store */ -#define DEFINE_LIST(name, type) \ +#define DEFINE_LIST(name, type) DEFINE_LIST_WITH_DTOR(name, type, NULL) + +/** \p DEFINE_LIST but with a custom element destructor + * + * \param name Type name to give the list container + * \param type Type of the elements the list will store + * \param dtor Destructor to be called on elements being released + */ +#define DEFINE_LIST_WITH_DTOR(name, type, dtor) \ \ /** list container \ * \ @@ -24,10 +32,9 @@ * accessed through the functions below. \ */ \ typedef struct { \ - type *data; /* backing storage */ \ - size_t size; /* number of elements in the list */ \ - size_t capacity; /* available storage slots */ \ - void (*dtor)(type); /* optional element destructor */ \ + type *data; /* backing storage */ \ + size_t size; /* number of elements in the list */ \ + size_t capacity; /* available storage slots */ \ } name##_t; \ \ /** create a new list \ @@ -91,8 +98,9 @@ type item) { \ assert(list != NULL); \ assert(index < list->size && "index out of bounds"); \ - if (list->dtor != NULL) { \ - list->dtor(list->data[index]); \ + void (*dtor_)(type) = (void (*)(type))(dtor); \ + if (dtor_ != NULL) { \ + dtor_(list->data[index]); \ } \ list->data[index] = item; \ } \ @@ -119,9 +127,10 @@ static inline LIST_UNUSED void name##_clear(name##_t *list) { \ assert(list != NULL); \ \ - if (list->dtor != NULL) { \ + void (*dtor_)(type) = (void (*)(type))(dtor); \ + if (dtor_ != NULL) { \ for (size_t i = 0; i < list->size; ++i) { \ - list->dtor(list->data[i]); \ + dtor_(list->data[i]); \ } \ } \ \ @@ -146,8 +155,9 @@ } else if (list->size > size) { \ /* we are shrinking the list */ \ while (list->size > size) { \ - if (list->dtor != NULL) { \ - list->dtor(list->data[list->size - 1]); \ + void (*dtor_)(type) = (void (*)(type))(dtor); \ + if (dtor_ != NULL) { \ + dtor_(list->data[list->size - 1]); \ } \ --list->size; \ } \ @@ -185,8 +195,8 @@ \ type value = list->data[list->size - 1]; \ \ - /* do not call `list->dtor` because we are transferring ownership of the \ - * removed element to the caller \ + /* do not call `dtor` because we are transferring ownership of the removed \ + * element to the caller \ */ \ --list->size; \ \ diff --git a/lib/cgraph/test_list.c b/lib/cgraph/test_list.c index ba9436e09..ac5cb1a11 100644 --- a/lib/cgraph/test_list.c +++ b/lib/cgraph/test_list.c @@ -258,12 +258,12 @@ static void test_attach_detach(void) { ints_free(&zs); } -DEFINE_LIST(strs, char *) +DEFINE_LIST_WITH_DTOR(strs, char *, free) static void test_dtor(void) { // setup a list with a non-trivial destructor - strs_t xs = {.dtor = (void (*)(char *))free}; + strs_t xs = {0}; for (size_t i = 0; i < 10; ++i) { char *hello = strdup("hello"); -- 2.50.1