testing wip

Author: pavel-kirienko

libudpard/udpard.c

@@ -111,11 +111,12 @@ void udpard_fragment_free_all(udpard_fragment_t* const frag, const udpard_mem_re
         }
         // Delete this fragment
         udpard_fragment_t* const parent = (udpard_fragment_t*)frag->index_offset.up;
-        parent->index_offset.lr[parent->index_offset.lr[1] == &frag->index_offset] = NULL;
         mem_free_payload(frag->payload_deleter, frag->origin);
         mem_free(fragment_memory_resource, sizeof(udpard_fragment_t), frag);
-        // Ascend the tree, tail call
-        udpard_fragment_free_all(parent, fragment_memory_resource);
+        if (parent != NULL) {
+            parent->index_offset.lr[parent->index_offset.lr[1] == (udpard_tree_t*)frag] = NULL;
+            udpard_fragment_free_all(parent, fragment_memory_resource); // tail call
+        }
     }
 }
 
@@ -721,7 +722,7 @@ void udpard_tx_free(const udpard_tx_mem_resources_t memory, udpard_tx_item_t* co
 /// All but the transfer metadata: fields that change from frame to frame within the same transfer.
 typedef struct
 {
-    uint32_t           offset;  ///< Offset of this fragment's payload within the full transfer payload.
+    size_t             offset;  ///< Offset of this fragment's payload within the full transfer payload.
     udpard_bytes_t     payload; ///< Does not include the header, just pure payload.
     udpard_bytes_mut_t origin;  ///< The entirety of the free-able buffer passed from the application.
 } rx_frame_base_t;
@@ -794,13 +795,15 @@ static bool rx_fragment_is_needed(udpard_tree_t* const root,
                                   const size_t         extent)
 {
     const size_t end = smaller3(frag_offset + frag_size, transfer_size, extent);
-    return (frag_offset < extent) && rx_fragment_tree_has_gap_in_range(root, frag_offset, end);
+    // We need to special-case zero-size transfers because they are effectively just a single gap at offset zero.
+    return ((end == 0) && (frag_offset == 0)) ||
+           ((frag_offset < extent) && rx_fragment_tree_has_gap_in_range(root, frag_offset, end));
 }
 
 /// Finds the number of contiguous payload bytes received from offset zero after accepting a new fragment.
 /// The transfer is considered fully received when covered_prefix >= min(extent, transfer_payload_size).
 /// This should be invoked after the fragment tree accepted a new fragment at frag_offset with frag_size.
-/// The complexity is logarithmic if updated after every received frame.
+/// The complexity is amortized-logarithmic, worst case is linear in the number of frames in the transfer.
 static size_t rx_fragment_tree_update_covered_prefix(udpard_tree_t* const root,
                                                      const size_t         old_prefix,
                                                      const size_t         frag_offset,
@@ -830,8 +833,8 @@ typedef enum
 /// Takes ownership of the frame payload; either a new fragment is inserted or the payload is freed.
 static rx_fragment_tree_update_result_t rx_fragment_tree_update(udpard_tree_t** const       root,
                                                                 const udpard_mem_resource_t fragment_memory,
-                                                                const rx_frame_base_t       frame,
                                                                 const udpard_mem_deleter_t  payload_deleter,
+                                                                const rx_frame_base_t       frame,
                                                                 const size_t                transfer_payload_size,
                                                                 const size_t                extent,
                                                                 size_t* const               covered_prefix_io)
@@ -903,11 +906,13 @@ static rx_fragment_tree_update_result_t rx_fragment_tree_update(udpard_tree_t**
     mew->payload_deleter = payload_deleter;
 
     // Remove all redundant fragments before inserting the new one.
-    while ((frag != NULL) && ((frag->offset + frag->view.size) <= v_right)) {
+    // No need to repeat tree lookup, we just step through the nodes using the next_greater lookup.
+    while ((frag != NULL) && (frag->offset >= v_left) && ((frag->offset + frag->view.size) <= v_right)) {
+        udpard_fragment_t* const next = (udpard_fragment_t*)cavl2_next_greater(&frag->index_offset);
         cavl2_remove(root, &frag->index_offset);
         mem_free_payload(frag->payload_deleter, frag->origin);
         mem_free(fragment_memory, sizeof(udpard_fragment_t), frag);
-        frag = (udpard_fragment_t*)cavl2_lower_bound(*root, &v_left, &rx_cavl_compare_fragment_offset);
+        frag = next;
     }
 
     // Insert the new fragment.

tests/src/helpers.h

@@ -57,6 +57,7 @@ static inline void dummy_free(void* const user, const size_t size, void* const p
 #define INSTRUMENTED_ALLOCATOR_CANARY_SIZE 1024U
 typedef struct
 {
+    /// Each allocator has its own canary, to catch an attempt to free memory allocated by a different allocator.
     uint_least8_t canary[INSTRUMENTED_ALLOCATOR_CANARY_SIZE];
     /// The limit can be changed at any moment to control the maximum amount of memory that can be allocated.
     /// It may be set to a value less than the currently allocated amount.

tests/src/test_intrusive_rx.c

@@ -7,21 +7,35 @@
 #include "helpers.h"
 #include <unity.h>
 
-typedef struct fragment_tree_match_item_t
+static size_t tree_count(udpard_tree_t* const root) // how many make a forest?
 {
-    size_t      offset;
-    const char* data; ///< A null-terminated string; NULL at the end.
-} fragment_tree_match_item_t;
+    size_t count = 0;
+    for (udpard_tree_t* p = cavl2_min(root); p != NULL; p = cavl2_next_greater(p)) {
+        count++;
+    }
+    return count;
+}
 
-static bool fragment_tree_match(udpard_fragment_t* const head, const fragment_tree_match_item_t items[])
+static udpard_fragment_t* fragment_at(udpard_tree_t* const root, uint32_t index)
 {
-    if (head == NULL) {
-        return (items[0].data == NULL);
+    for (udpard_fragment_t* it = (udpard_fragment_t*)cavl2_min(root); it != NULL;
+         it                    = (udpard_fragment_t*)cavl2_next_greater(&it->index_offset)) {
+        if (index-- == 0U) {
+            return it;
+        }
     }
-    const bool match = (head->offset == items[0].offset) &&
-                       (strlen((const char*)head->view.data) == strlen(items[0].data)) &&
-                       (memcmp(head->view.data, items[0].data, head->view.size) == 0);
-    return match && fragment_tree_match((udpard_fragment_t*)cavl2_next_greater(&head->index_offset), &items[1]);
+    return NULL;
+}
+
+/// Allocates the payload on the heap, emulating normal transfer reception.
+/// The payload shall not contain NUL characters.
+static rx_frame_base_t make_frame_base(const udpard_mem_resource_t mem, const size_t offset, const char* const payload)
+{
+    const size_t size = (payload != NULL) ? strlen(payload) : 0U;
+    void*        data = mem.alloc(mem.user, size);
+    return (rx_frame_base_t){ .offset  = offset,
+                              .payload = { .data = data, .size = size },
+                              .origin  = { .data = data, .size = size } };
 }
 
 static void test_rx_fragment_tree_update_a(void)
@@ -34,6 +48,45 @@ static void test_rx_fragment_tree_update_a(void)
     instrumented_allocator_new(&alloc_payload);
     const udpard_mem_resource_t mem_payload = instrumented_allocator_make_resource(&alloc_payload);
     const udpard_mem_deleter_t  del_payload = instrumented_allocator_make_deleter(&alloc_payload);
+
+    // Empty payload test
+    {
+        udpard_tree_t*                   root = NULL;
+        size_t                           cov  = 0;
+        rx_fragment_tree_update_result_t res  = rx_fragment_tree_not_done;
+        //
+        res = rx_fragment_tree_update(&root, //
+                                      mem_frag,
+                                      del_payload,
+                                      make_frame_base(mem_payload, 0, ""),
+                                      0,
+                                      0,
+                                      &cov);
+        TEST_ASSERT_EQUAL(rx_fragment_tree_done, res);
+        TEST_ASSERT_EQUAL_size_t(0, cov);
+        TEST_ASSERT_NOT_NULL(root);
+        TEST_ASSERT_EQUAL(1, tree_count(root));
+        // Check the retained payload.
+        TEST_ASSERT_EQUAL_size_t(0, fragment_at(root, 0)->offset);
+        TEST_ASSERT_EQUAL_size_t(0, fragment_at(root, 0)->view.size);
+        TEST_ASSERT_NULL(fragment_at(root, 1));
+        // Check the heap.
+        TEST_ASSERT_EQUAL_size_t(1, alloc_frag.allocated_fragments);
+        TEST_ASSERT_EQUAL_size_t(0, alloc_payload.allocated_fragments); // bc payload empty
+        TEST_ASSERT_EQUAL_size_t(1, alloc_frag.count_alloc);
+        TEST_ASSERT_EQUAL_size_t(1, alloc_payload.count_alloc);
+        TEST_ASSERT_EQUAL_size_t(0, alloc_frag.count_free);
+        TEST_ASSERT_EQUAL_size_t(0, alloc_payload.count_free);
+        // Free the tree (as in freedom). The free tree is free to manifest its own destiny.
+        udpard_fragment_free_all((udpard_fragment_t*)root, mem_frag);
+        // Check the heap.
+        TEST_ASSERT_EQUAL_size_t(0, alloc_frag.allocated_fragments);
+        TEST_ASSERT_EQUAL_size_t(0, alloc_payload.allocated_fragments);
+        TEST_ASSERT_EQUAL_size_t(1, alloc_frag.count_alloc);
+        TEST_ASSERT_EQUAL_size_t(1, alloc_payload.count_alloc);
+        TEST_ASSERT_EQUAL_size_t(1, alloc_frag.count_free);
+        TEST_ASSERT_EQUAL_size_t(0, alloc_payload.count_free); // bc payload empty
+    }
 }
 
 static void test_rx_transfer_id_forward_distance(void)