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.

diff --git a/contrib/prune/prune.c b/contrib/prune/prune.c
index 4f0b20856599a701558f2acbbd0f69700cc0db07..be3bf1784e6880391b8a360d1770ce1b0b978639 100644 (file)
--- 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 a6dba2daa2ad3a7321b9d2f67c331213da8314a6..c8da91a1388f7941a05bbae9d1a8addf3e56f4c6 100644 (file)
--- 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 ba9436e0949a198369acdf669d39fe83b4692d8e..ac5cb1a11439d369e5e6b6266ed005ffdca135bd 100644 (file)
--- 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");