Sat, 19 Jul 2025 21:09:07 +0200
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;