fix incorrect memcpy() in cxMempoolTransfer() default tip

Sat, 19 Jul 2025 21:09:07 +0200

author
Mike Becker <universe@uap-core.de>
date
Sat, 19 Jul 2025 21:09:07 +0200
changeset 1336
5acc23b518aa
parent 1335
158eb29f0b27

fix incorrect memcpy() in cxMempoolTransfer()

src/mempool.c file | annotate | diff | comparison | revisions
tests/test_mempool.c file | annotate | diff | comparison | revisions
--- a/src/mempool.c	Sun Jun 15 18:40:31 2025 +0200
+++ b/src/mempool.c	Sat Jul 19 21:09:07 2025 +0200
@@ -633,13 +633,19 @@
     new_source_allocator->data = source;
 
     // transfer all the data
-    memcpy(&dest->data[dest->size], source->data, sizeof(void*)*source->size);
-    dest->size += source->size;
+    if (source->size > 0) {
+        memcpy(&dest->data[dest->size], source->data,
+            sizeof(void*)*source->size);
+        dest->size += source->size;
+    }
 
     // transfer all registered memory
-    memcpy(&dest->registered[dest->registered_size], source->registered,
-           sizeof(struct cx_mempool_foreign_memory_s) * source->size);
-    dest->registered_size += source->registered_size;
+    if (source->registered_size > 0) {
+        memcpy(&dest->registered[dest->registered_size], source->registered,
+               sizeof(struct cx_mempool_foreign_memory_s)
+               * source->registered_size);
+        dest->registered_size += source->registered_size;
+    }
 
     // register the old allocator with the new pool
     // we have to remove const-ness for this, but that's okay here
--- a/tests/test_mempool.c	Sun Jun 15 18:40:31 2025 +0200
+++ b/tests/test_mempool.c	Sat Jul 19 21:09:07 2025 +0200
@@ -399,6 +399,53 @@
     CX_TEST_DO {
         // allocate the first object
         int *c = cxMalloc(src->allocator, sizeof(int));
+        // check that the destructor functions are also transferred
+        cxMempoolSetDestructor(c, test_mempool_destructor);
+        // allocate the second object
+        c = cxMalloc(src->allocator, sizeof(int));
+        cxMempoolSetDestructor(c, test_mempool_destructor);
+
+
+        // check source pool
+        CX_TEST_ASSERT(src->size == 2);
+        CX_TEST_ASSERT(src->registered_size == 0);
+        const CxAllocator *old_allocator = src->allocator;
+        CX_TEST_ASSERT(old_allocator->data == src);
+
+        // perform transfer
+        int result = cxMempoolTransfer(src, dest);
+        CX_TEST_ASSERT(result == 0);
+
+        // check transfer
+        CX_TEST_ASSERT(src->size == 0);
+        CX_TEST_ASSERT(dest->size == 2);
+        CX_TEST_ASSERT(src->registered_size == 0);
+        CX_TEST_ASSERT(dest->registered_size == 1); // the old allocator
+        CX_TEST_ASSERT(src->allocator != old_allocator);
+        CX_TEST_ASSERT(old_allocator->data == dest);
+
+        // verify that destroying old pool does nothing
+        test_mempool_destructor_called = 0;
+        cxMempoolFree(src);
+        CX_TEST_ASSERT(test_mempool_destructor_called == 0);
+
+        // cover illegal arguments
+        result = cxMempoolTransfer(dest, dest);
+        CX_TEST_ASSERT(result != 0);
+
+        // verify that destroying new pool calls the destructors
+        // but only two times (the old allocator has a different destructor)
+        cxMempoolFree(dest);
+        CX_TEST_ASSERT(test_mempool_destructor_called == 2);
+    }
+}
+
+CX_TEST(test_mempool_transfer_with_foreign_memory) {
+    CxMempool *src = cxMempoolCreateSimple(4);
+    CxMempool *dest = cxMempoolCreateSimple(4);
+    CX_TEST_DO {
+        // allocate the first object
+        int *c = cxMalloc(src->allocator, sizeof(int));
         // allocate the second object
         c = cxMalloc(src->allocator, sizeof(int));
         // check that the destructor functions are also transferred
@@ -430,10 +477,6 @@
         cxMempoolFree(src);
         CX_TEST_ASSERT(test_mempool_destructor_called == 0);
 
-        // cover illegal arguments
-        result = cxMempoolTransfer(dest, dest);
-        CX_TEST_ASSERT(result != 0);
-
         // verify that destroying new pool calls the destructors
         // but only two times (the old allocator has a different destructor)
         cxMempoolFree(dest);
@@ -444,6 +487,48 @@
     }
 }
 
+CX_TEST(test_mempool_transfer_foreign_memory_only) {
+    CxMempool *src = cxMempoolCreateSimple(4);
+    CxMempool *dest = cxMempoolCreateSimple(4);
+    int *a = malloc(sizeof(int));
+    int *b = malloc(sizeof(int));
+    CX_TEST_DO {
+        // register foreign objects
+        cxMempoolRegister(src, a, test_mempool_destructor);
+        cxMempoolRegister(src, b, test_mempool_destructor);
+
+        // check source pool
+        CX_TEST_ASSERT(src->size == 0);
+        CX_TEST_ASSERT(src->registered_size == 2);
+        const CxAllocator *old_allocator = src->allocator;
+        CX_TEST_ASSERT(old_allocator->data == src);
+
+        // perform transfer
+        int result = cxMempoolTransfer(src, dest);
+        CX_TEST_ASSERT(result == 0);
+
+        // check transfer
+        CX_TEST_ASSERT(src->size == 0);
+        CX_TEST_ASSERT(dest->size == 0);
+        CX_TEST_ASSERT(src->registered_size == 0);
+        CX_TEST_ASSERT(dest->registered_size == 3); // 2 objects + old allocator
+        CX_TEST_ASSERT(src->allocator != old_allocator);
+        CX_TEST_ASSERT(old_allocator->data == dest);
+
+        // verify that destroying old pool does nothing
+        test_mempool_destructor_called = 0;
+        cxMempoolFree(src);
+        CX_TEST_ASSERT(test_mempool_destructor_called == 0);
+
+        // verify that destroying new pool calls the destructors
+        // but only two times (the old allocator has a different destructor)
+        cxMempoolFree(dest);
+        CX_TEST_ASSERT(test_mempool_destructor_called == 2);
+    }
+    free(a);
+    free(b);
+}
+
 CX_TEST(test_mempool_transfer_object) {
     CxMempool *src = cxMempoolCreateSimple(4);
     CxMempool *dest = cxMempoolCreateSimple(4);
@@ -518,6 +603,8 @@
     cx_test_register(suite, test_mempool_register);
     cx_test_register(suite, test_mempool_register2);
     cx_test_register(suite, test_mempool_transfer);
+    cx_test_register(suite, test_mempool_transfer_with_foreign_memory);
+    cx_test_register(suite, test_mempool_transfer_foreign_memory_only);
     cx_test_register(suite, test_mempool_transfer_object);
 
     return suite;

mercurial