Mercurial > hg > ucx / changeset

--- a/src/kv_list.c	Thu Sep 11 20:17:43 2025 +0200
+++ b/src/kv_list.c	Fri Sep 12 16:56:04 2025 +0200
@@ -96,6 +96,10 @@
     }
 }
 
+static CxHashKey *cx_kv_list_loc_key(cx_kv_list *list, void *node_data) {
+    return (CxHashKey*)((char*)node_data + list->list.base.collection.elem_size);
+}
+
 static void cx_kvl_deallocate(struct cx_list_s *list) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
     // patch the destructors
@@ -150,10 +154,20 @@
 ) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
     // patch the destructors
+    // we also have to do that when targetbuf is NULL,
+    // because we do not want wrong destructors to be called when we remove keys from the map
     cx_kv_list_update_destructors(kv_list);
-    // TODO: always use the target buffer to get the element first,
-    //       then obtain the key, remove it from the map,
-    //       and finally call any destructors manually
+    // iterate through the elements first and remove their keys from the map
+    CxIterator iter = kv_list->list_methods->iterator(list, index, false);
+    for (size_t i = 0; i < num && cxIteratorValid(iter); i++) {
+        void *node_data = cxIteratorCurrent(iter);
+        CxHashKey *key = cx_kv_list_loc_key(kv_list, node_data);
+        // when the hash is zero, there is no key assigned to that element
+        if (key->hash != 0) {
+            kv_list->map_methods->remove(&kv_list->map->map_base.base, *key, NULL);
+        }
+        cxIteratorNext(iter);
+    }
     return kv_list->list_methods->remove(list, index, num, targetbuf);
 }
 
@@ -236,6 +250,13 @@
         &kv_list->list.base, kv_list->list.base.collection.size,
         kv_list->list.base.collection.store_pointer ? &value : value);
     if (node_data == NULL) return NULL; // LCOV_EXCL_LINE
+    // if the hash has not yet been computed, do it now
+    if (key.hash == 0) {
+        cx_hash_murmur(&key);
+    }
+    // copy the key to the node data
+    CxHashKey *key_ptr = cx_kv_list_loc_key(kv_list, node_data);
+    *key_ptr = key;
     // then insert the key into the map, referring to the node data
     return kv_list->map_methods->put(map, key, node_data);
 }
@@ -402,9 +423,16 @@
 
 int cx_kv_list_set_key(CxList *list, size_t index, CxHashKey key) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
-    char *node_data = kv_list->list_methods->at(list, index);
-    char *loc_key = node_data + list->collection.elem_size;
-    memcpy(loc_key, &key, sizeof(key));
+    void *node_data = kv_list->list_methods->at(list, index);
+    if (node_data == NULL) {
+        return 1;
+    }
+    // if the hash has not yet been computed, do it now
+    if (key.hash == 0) {
+        cx_hash_murmur(&key);
+    }
+    CxHashKey *loc_key = cx_kv_list_loc_key(kv_list, node_data);
+    *loc_key = key;
 
     // TODO: what happens when we are _replacing_ an existing key?
     kv_list->map_methods->put(&kv_list->map->map_base.base, key, node_data);

--- a/tests/test_kv_list.c	Thu Sep 11 20:17:43 2025 +0200
+++ b/tests/test_kv_list.c	Fri Sep 12 16:56:04 2025 +0200
@@ -69,6 +69,153 @@
     cx_testing_allocator_destroy(&talloc);
 }
 
+CX_TEST(test_kv_list_remove) {
+    CxList *list = cxKvListCreateSimple(sizeof(int));
+    int x;
+    CX_TEST_DO {
+        CxMap *map = cxKvListAsMap(list);
+
+        x = 13;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "xyz", &x));
+        x = 37;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "abc", &x));
+        x = 47;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "efg", &x));
+        x = 90;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "hij", &x));
+
+        CX_TEST_ASSERT(cxMapSize(map) == 4);
+        CX_TEST_ASSERT(cxListSize(list) == 4);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "efg") == 47);
+        CX_TEST_ASSERT(0 == cxListRemove(list, 2));
+        CX_TEST_ASSERT(cxListSize(list) == 3);
+        CX_TEST_ASSERT(cxMapSize(map) == 3);
+        CX_TEST_ASSERT(cxMapGet(map, "efg") == NULL);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "xyz") == 13);
+        CX_TEST_ASSERT(0 == cxListRemove(list, 0));
+        CX_TEST_ASSERT(cxListSize(list) == 2);
+        CX_TEST_ASSERT(cxMapSize(map) == 2);
+        CX_TEST_ASSERT(cxMapGet(map, "xyz") == NULL);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "hij") == 90);
+        CX_TEST_ASSERT(0 == cxListRemove(list, 1));
+        CX_TEST_ASSERT(cxListSize(list) == 1);
+        CX_TEST_ASSERT(cxMapSize(map) == 1);
+        CX_TEST_ASSERT(cxMapGet(map, "hij") == NULL);
+    }
+    cxListFree(list);
+}
+
+CX_TEST(test_kv_list_remove_and_get) {
+    CxList *list = cxKvListCreateSimple(sizeof(int));
+    int x, y;
+    CX_TEST_DO {
+        CxMap *map = cxKvListAsMap(list);
+
+        x = 13;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "xyz", &x));
+        x = 37;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "abc", &x));
+        x = 47;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "efg", &x));
+        x = 90;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "hij", &x));
+
+        CX_TEST_ASSERT(cxMapSize(map) == 4);
+        CX_TEST_ASSERT(cxListSize(list) == 4);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "efg") == 47);
+        CX_TEST_ASSERT(0 == cxListRemoveAndGet(list, 2, &y));
+        CX_TEST_ASSERT(y == 47);
+        CX_TEST_ASSERT(cxListSize(list) == 3);
+        CX_TEST_ASSERT(cxMapSize(map) == 3);
+        CX_TEST_ASSERT(cxMapGet(map, "efg") == NULL);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "xyz") == 13);
+        CX_TEST_ASSERT(0 == cxListRemoveAndGet(list, 0, &y));
+        CX_TEST_ASSERT(y == 13);
+        CX_TEST_ASSERT(cxListSize(list) == 2);
+        CX_TEST_ASSERT(cxMapSize(map) == 2);
+        CX_TEST_ASSERT(cxMapGet(map, "xyz") == NULL);
+
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "hij") == 90);
+        CX_TEST_ASSERT(0 == cxListRemoveAndGet(list, 1, &y));
+        CX_TEST_ASSERT(y == 90);
+        CX_TEST_ASSERT(cxListSize(list) == 1);
+        CX_TEST_ASSERT(cxMapSize(map) == 1);
+        CX_TEST_ASSERT(cxMapGet(map, "hij") == NULL);
+    }
+    cxListFree(list);
+}
+
+CX_TEST(test_kv_list_remove_array) {
+    CxList *list = cxKvListCreateSimple(sizeof(int));
+    int x;
+    CX_TEST_DO {
+        CxMap *map = cxKvListAsMap(list);
+
+        x = 13;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "xyz", &x));
+        x = 37;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "abc", &x));
+        x = 47;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "efg", &x));
+        x = 90;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "hij", &x));
+
+        CX_TEST_ASSERT(cxMapSize(map) == 4);
+        CX_TEST_ASSERT(cxListSize(list) == 4);
+
+        CX_TEST_ASSERT(2 == cxListRemoveArray(list, 1, 2));
+        CX_TEST_ASSERT(cxMapSize(map) == 2);
+        CX_TEST_ASSERT(cxListSize(list) == 2);
+        CX_TEST_ASSERT(cxMapGet(map, "abc") == NULL);
+        CX_TEST_ASSERT(cxMapGet(map, "efg") == NULL);
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "xyz") == 13);
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "hij") == 90);
+        CX_TEST_ASSERT(cxListAt(list, 0) == cxMapGet(map, "xyz"));
+        CX_TEST_ASSERT(cxListAt(list, 1) == cxMapGet(map, "hij"));
+    }
+    cxListFree(list);
+}
+
+CX_TEST(test_kv_list_remove_array_and_get) {
+    CxList *list = cxKvListCreateSimple(sizeof(int));
+    int x;
+    CX_TEST_DO {
+        CxMap *map = cxKvListAsMap(list);
+
+        x = 13;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "xyz", &x));
+        x = 37;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "abc", &x));
+        x = 47;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "efg", &x));
+        x = 90;
+        CX_TEST_ASSERT(0 == cxMapPut(map, "hij", &x));
+
+        CX_TEST_ASSERT(cxMapSize(map) == 4);
+        CX_TEST_ASSERT(cxListSize(list) == 4);
+
+        int y[2];
+
+        CX_TEST_ASSERT(2 == cxListRemoveArrayAndGet(list, 1, 2, y));
+        CX_TEST_ASSERT(y[0] == 37);
+        CX_TEST_ASSERT(y[1] == 47);
+        CX_TEST_ASSERT(cxMapSize(map) == 2);
+        CX_TEST_ASSERT(cxListSize(list) == 2);
+        CX_TEST_ASSERT(cxMapGet(map, "abc") == NULL);
+        CX_TEST_ASSERT(cxMapGet(map, "efg") == NULL);
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "xyz") == 13);
+        CX_TEST_ASSERT(*(int*)cxMapGet(map, "hij") == 90);
+        CX_TEST_ASSERT(cxListAt(list, 0) == cxMapGet(map, "xyz"));
+        CX_TEST_ASSERT(cxListAt(list, 1) == cxMapGet(map, "hij"));
+    }
+    cxListFree(list);
+}
+
 CX_TEST(test_kv_list_map_put) {
     CxList *list = cxKvListCreateSimple(sizeof(int));
     int x;
@@ -331,6 +478,10 @@
     cx_test_register(suite, test_kv_list_map_as_list);
     cx_test_register(suite, test_kv_list_free_as_map);
     cx_test_register(suite, test_kv_list_free_as_list);
+    cx_test_register(suite, test_kv_list_remove);
+    cx_test_register(suite, test_kv_list_remove_and_get);
+    cx_test_register(suite, test_kv_list_remove_array);
+    cx_test_register(suite, test_kv_list_remove_array_and_get);
     cx_test_register(suite, test_kv_list_map_put);
     cx_test_register(suite, test_kv_list_map_put_ptr);
     cx_test_register(suite, test_kv_list_map_remove);