add unit test for cxListClear + fix destructor functions not always invoked with the correct pointer

Mon, 20 Mar 2023 19:09:08 +0100

author
Mike Becker <universe@uap-core.de>
date
Mon, 20 Mar 2023 19:09:08 +0100
changeset 666
b5dd654deb3b
parent 665
c4041b07165e
child 667
2f88a7c13a28

add unit test for cxListClear + fix destructor functions not always invoked with the correct pointer

src/array_list.c file | annotate | diff | comparison | revisions
src/cx/list.h file | annotate | diff | comparison | revisions
src/linked_list.c file | annotate | diff | comparison | revisions
src/list.c file | annotate | diff | comparison | revisions
tests/test_list.cpp file | annotate | diff | comparison | revisions
--- a/src/array_list.c	Mon Mar 20 18:05:12 2023 +0100
+++ b/src/array_list.c	Mon Mar 20 19:09:08 2023 +0100
@@ -310,15 +310,14 @@
     switch (list->content_destructor_type) {
         case CX_DESTRUCTOR_SIMPLE: {
             for (size_t i = 0; i < list->size; i++) {
-                list->simple_destructor(ptr);
+                cx_list_invoke_simple_destructor(list, ptr);
                 ptr += list->itemsize;
             }
             break;
         }
         case CX_DESTRUCTOR_ADVANCED: {
             for (size_t i = 0; i < list->size; i++) {
-                list->advanced_destructor.func(list->advanced_destructor.data,
-                                               ptr);
+                cx_list_invoke_advanced_destructor(list, ptr);
                 ptr += list->itemsize;
             }
             break;
@@ -326,6 +325,9 @@
         case CX_DESTRUCTOR_NONE:
             break; // nothing
     }
+
+    memset(arl->data, 0, list->size * list->itemsize);
+    list->size = 0;
 }
 
 static int cx_arl_swap(
--- a/src/cx/list.h	Mon Mar 20 18:05:12 2023 +0100
+++ b/src/cx/list.h	Mon Mar 20 19:09:08 2023 +0100
@@ -227,7 +227,7 @@
 typedef struct cx_list_s CxList;
 
 /**
- * Invokes the destructor function for a specific element.
+ * Invokes the configured destructor function for a specific element.
  *
  * Usually only used by list implementations. There should be no need
  * to invoke this function manually.
@@ -242,6 +242,36 @@
 );
 
 /**
+ * Invokes the simple destructor function for a specific element.
+ *
+ * Usually only used by list implementations. There should be no need
+ * to invoke this function manually.
+ *
+ * @param list the list
+ * @param elem the element
+ */
+__attribute__((__nonnull__))
+void cx_list_invoke_simple_destructor(
+        struct cx_list_s const *list,
+        void *elem
+);
+
+/**
+ * Invokes the advanced destructor function for a specific element.
+ *
+ * Usually only used by list implementations. There should be no need
+ * to invoke this function manually.
+ *
+ * @param list the list
+ * @param elem the element
+ */
+__attribute__((__nonnull__))
+void cx_list_invoke_advanced_destructor(
+        struct cx_list_s const *list,
+        void *elem
+);
+
+/**
  * Advises the list to store copies of the objects (default mode of operation).
  *
  * Retrieving objects from this list will yield pointers to the copies stored
@@ -276,7 +306,7 @@
  * @see cxListStorePointers()
  */
 __attribute__((__nonnull__))
-bool cxListIsStoringPointers(CxList *list);
+bool cxListIsStoringPointers(CxList const *list);
 
 /**
  * Adds an item to the end of the list.
--- a/src/linked_list.c	Mon Mar 20 18:05:12 2023 +0100
+++ b/src/linked_list.c	Mon Mar 20 19:09:08 2023 +0100
@@ -598,7 +598,7 @@
     switch (list->content_destructor_type) {
         case CX_DESTRUCTOR_SIMPLE: {
             while (node != NULL) {
-                list->simple_destructor(node->payload);
+                cx_list_invoke_simple_destructor(list, node->payload);
                 cx_linked_list_node *next = node->next;
                 cxFree(list->allocator, node);
                 node = next;
@@ -607,8 +607,7 @@
         }
         case CX_DESTRUCTOR_ADVANCED: {
             while (node != NULL) {
-                list->advanced_destructor.func(list->advanced_destructor.data,
-                                               node->payload);
+                cx_list_invoke_advanced_destructor(list, node->payload);
                 cx_linked_list_node *next = node->next;
                 cxFree(list->allocator, node);
                 node = next;
--- a/src/list.c	Mon Mar 20 18:05:12 2023 +0100
+++ b/src/list.c	Mon Mar 20 19:09:08 2023 +0100
@@ -191,7 +191,7 @@
     list->cl = &cx_pointer_list_class;
 }
 
-bool cxListIsStoringPointers(CxList *list) {
+bool cxListIsStoringPointers(CxList const *list) {
     return list->climpl != NULL;
 }
 
@@ -203,11 +203,11 @@
 ) {
     switch (list->content_destructor_type) {
         case CX_DESTRUCTOR_SIMPLE: {
-            list->simple_destructor(elem);
+            cx_list_invoke_simple_destructor(list, elem);
             break;
         }
         case CX_DESTRUCTOR_ADVANCED: {
-            list->advanced_destructor.func(list->advanced_destructor.data, elem);
+            cx_list_invoke_advanced_destructor(list, elem);
             break;
         }
         case CX_DESTRUCTOR_NONE:
@@ -215,11 +215,32 @@
     }
 }
 
+void cx_list_invoke_simple_destructor(
+        CxList const *list,
+        void *elem
+) {
+    if (cxListIsStoringPointers(list)) {
+        elem = *((void **) elem);
+    }
+    list->simple_destructor(elem);
+}
+
+void cx_list_invoke_advanced_destructor(
+        CxList const *list,
+        void *elem
+) {
+    if (cxListIsStoringPointers(list)) {
+        elem = *((void **) elem);
+    }
+    list->advanced_destructor.func(list->advanced_destructor.data, elem);
+}
+
 void cxListDestroy(CxList *list) {
     switch (list->content_destructor_type) {
         case CX_DESTRUCTOR_SIMPLE: {
             CxIterator iter = cxListIterator(list);
             cx_foreach(void*, elem, iter) {
+                // already correctly resolved pointer - immediately invoke dtor
                 list->simple_destructor(elem);
             }
             break;
@@ -227,6 +248,7 @@
         case CX_DESTRUCTOR_ADVANCED: {
             CxIterator iter = cxListIterator(list);
             cx_foreach(void*, elem, iter) {
+                // already correctly resolved pointer - immediately invoke dtor
                 list->advanced_destructor.func(list->advanced_destructor.data, elem);
             }
             break;
--- a/tests/test_list.cpp	Mon Mar 20 18:05:12 2023 +0100
+++ b/tests/test_list.cpp	Mon Mar 20 19:09:08 2023 +0100
@@ -38,6 +38,19 @@
 #include <unordered_set>
 #include <algorithm>
 
+struct testdatastruct {
+    int x;
+    void *ptr;
+};
+
+static void free_testdatastruct(
+        void *a,
+        void *s
+) {
+    auto al = reinterpret_cast<CxTestingAllocator *>(a);
+    cxFree(al, reinterpret_cast<testdatastruct *>(s)->ptr);
+}
+
 struct node {
     node *next = nullptr;
     node *prev = nullptr;
@@ -697,6 +710,21 @@
         EXPECT_NE(cxListRemove(list, testdata_len), 0);
     }
 
+    void verifyClear(CxList *list) {
+        // use the testing allocator for testing the destructor function
+        list->content_destructor_type = CX_DESTRUCTOR_ADVANCED;
+        list->advanced_destructor.func = free_testdatastruct;
+        list->advanced_destructor.data = &testingAllocator;
+
+        testdatastruct s[10];
+        for (auto &t: s) {
+            t.ptr = cxMalloc(&testingAllocator, 16);
+            cxListAdd(list, &t);
+        }
+
+        cxListClear(list);
+    }
+
     static void verifySwap(CxList *list) {
         ASSERT_EQ(list->size, 0);
 
@@ -997,6 +1025,20 @@
     verifyRemove(arrayListFromTestData());
 }
 
+TEST_F(LinkedList, cxListClear) {
+    verifyClear(autofree(cxLinkedListCreateSimple(sizeof(testdatastruct))));
+}
+
+TEST_F(PointerLinkedList, cxListClear) {
+    auto l = cxLinkedListCreateSimple(sizeof(testdatastruct));
+    cxListStorePointers(l);
+    verifyClear(autofree(l));
+}
+
+TEST_F(ArrayList, cxListClear) {
+    verifyClear(autofree(cxArrayListCreateSimple(sizeof(testdatastruct), 8)));
+}
+
 TEST_F(LinkedList, cxListSwap) {
     verifySwap(autofree(cxLinkedListCreate(&testingAllocator, cx_cmp_int, sizeof(int))));
 }

mercurial