Skip to content

Commit 96ac7b5

Browse files
pavel-kirienkopavel-kirienko
committed
update udpard_fragment_gather
1 parent c479834 commit 96ac7b5

File tree

4 files changed

+112
-73
lines changed

4 files changed

+112
-73
lines changed

‎libudpard/udpard.c‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -142,28 +142,25 @@ void udpard_fragment_free_all(udpard_fragment_t* const frag, const udpard_mem_re
142142
udpard_fragment_t* const parent = (udpard_fragment_t*)frag->index_offset.up;
143143
mem_free_payload(frag->payload_deleter, frag->origin);
144144
mem_free(fragment_mem_resource, sizeof(udpard_fragment_t), frag);
145+
// Ascend the tree.
145146
if (parent != NULL) {
146147
parent->index_offset.lr[parent->index_offset.lr[1] == (udpard_tree_t*)frag] = NULL;
147148
udpard_fragment_free_all(parent, fragment_mem_resource); // tail call hopefully
148149
}
149150
}
150151
}
151152

152-
udpard_fragment_t* udpard_fragment_seek(udpard_fragment_t* frag, const size_t offset)
153+
udpard_fragment_t* udpard_fragment_seek(const udpard_fragment_t* frag, const size_t offset)
153154
{
154155
if (frag != NULL) {
155-
// Common case: if the offset is already within the provided fragment, return it as-is.
156-
if ((frag->offset <= offset) && ((frag->offset + frag->view.size) > offset)) {
157-
return frag;
158-
}
159156
while (frag->index_offset.up != NULL) { // Only if the given node is not already the root.
160-
frag = (udpard_fragment_t*)frag->index_offset.up;
157+
frag = (const udpard_fragment_t*)frag->index_offset.up;
161158
}
162159
if (offset == 0) { // Common fast path.
163-
return (udpard_fragment_t*)cavl2_min(&frag->index_offset);
160+
return (udpard_fragment_t*)cavl2_min((udpard_tree_t*)frag);
164161
}
165162
udpard_fragment_t* const f =
166-
(udpard_fragment_t*)cavl2_predecessor(&frag->index_offset, &offset, &cavl_compare_fragment_offset);
163+
(udpard_fragment_t*)cavl2_predecessor((udpard_tree_t*)frag, &offset, &cavl_compare_fragment_offset);
167164
if ((f != NULL) && ((f->offset + f->view.size) > offset)) {
168165
UDPARD_ASSERT(f->offset <= offset);
169166
return f;
@@ -172,38 +169,50 @@ udpard_fragment_t* udpard_fragment_seek(udpard_fragment_t* frag, const size_t of
172169
return NULL;
173170
}
174171

175-
udpard_fragment_t* udpard_fragment_next(udpard_fragment_t* const frag)
172+
udpard_fragment_t* udpard_fragment_next(const udpard_fragment_t* frag)
176173
{
177-
return (frag != NULL) ? ((udpard_fragment_t*)cavl2_next_greater(&frag->index_offset)) : NULL;
174+
return (frag != NULL) ? ((udpard_fragment_t*)cavl2_next_greater((udpard_tree_t*)frag)) : NULL;
178175
}
179176

180-
size_t udpard_fragment_gather(const udpard_fragment_t* const frag,
181-
const size_t offset,
182-
const size_t size,
183-
void* const destination)
177+
size_t udpard_fragment_gather(const udpard_fragment_t** cursor,
178+
const size_t offset,
179+
const size_t size,
180+
void* const destination)
184181
{
185182
size_t copied = 0;
186-
if ((frag != NULL) && (destination != NULL)) {
187-
udpard_fragment_t* f = udpard_fragment_seek((udpard_fragment_t*)frag, offset);
188-
size_t cursor = offset;
189-
byte_t* const out = (byte_t*)destination;
190-
// Copy contiguous fragments starting at the requested offset.
191-
while ((f != NULL) && (copied < size)) {
192-
UDPARD_ASSERT(f->offset <= cursor);
193-
UDPARD_ASSERT((f->offset + f->view.size) > cursor);
194-
UDPARD_ASSERT(f->view.data != NULL);
195-
const size_t offset_in_frag = cursor - f->offset;
196-
const size_t available = f->view.size - offset_in_frag;
197-
const size_t to_copy = smaller(available, size - copied);
198-
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
199-
(void)memcpy(out + copied, ((const byte_t*)f->view.data) + offset_in_frag, to_copy);
200-
copied += to_copy;
201-
cursor += to_copy;
202-
if (copied < size) {
203-
f = udpard_fragment_next(f);
204-
UDPARD_ASSERT((f == NULL) || (f->offset == cursor));
183+
if ((cursor != NULL) && (*cursor != NULL) && (destination != NULL)) {
184+
const size_t end_offset = (*cursor)->offset + (*cursor)->view.size;
185+
const udpard_fragment_t* f = NULL;
186+
if ((offset < (*cursor)->offset) || (offset > end_offset)) {
187+
f = udpard_fragment_seek(*cursor, offset);
188+
} else if (offset == end_offset) { // Common case during sequential access.
189+
f = udpard_fragment_next(*cursor);
190+
} else {
191+
f = *cursor;
192+
}
193+
if ((f != NULL) && (size > 0U)) {
194+
const udpard_fragment_t* last = f;
195+
size_t pos = offset;
196+
byte_t* const out = (byte_t*)destination;
197+
while ((f != NULL) && (copied < size)) { // Copy contiguous fragments starting at the requested offset.
198+
UDPARD_ASSERT(f->offset <= pos);
199+
UDPARD_ASSERT(pos < (f->offset + f->view.size));
200+
UDPARD_ASSERT(f->view.data != NULL);
201+
const size_t bias = pos - f->offset;
202+
const size_t to_copy = smaller(f->view.size - bias, size - copied);
203+
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
204+
(void)memcpy(out + copied, ((const byte_t*)f->view.data) + bias, to_copy);
205+
copied += to_copy;
206+
pos += to_copy;
207+
last = f;
208+
if (copied < size) {
209+
f = udpard_fragment_next(f);
210+
UDPARD_ASSERT((f == NULL) || (f->offset == pos));
211+
}
205212
}
213+
*cursor = last; // Keep iterator non-NULL.
206214
}
215+
UDPARD_ASSERT(NULL != *cursor);
207216
}
208217
return copied;
209218
}

‎libudpard/udpard.h‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -214,23 +214,24 @@ void udpard_fragment_free_all(udpard_fragment_t* const frag, const udpard_mem_re
214214
/// This is also the idiomatic way to find the head of the fragment list when invoked with offset zero.
215215
/// This function accepts any node in the fragment tree, not necessarily the head or the root, and
216216
/// has a logarithmic complexity in the number of fragments, which makes it very efficient.
217-
udpard_fragment_t* udpard_fragment_seek(udpard_fragment_t* frag, const size_t offset);
217+
udpard_fragment_t* udpard_fragment_seek(const udpard_fragment_t* frag, const size_t offset);
218218

219219
/// Given any fragment in a transfer, returns the next fragment in strictly ascending order of offsets.
220220
/// The offset of the next fragment always equals the sum of the offset and size of the current fragment.
221221
/// Returns NULL if there is no next fragment or if the given fragment is NULL.
222222
/// The complexity is amortized-constant.
223-
udpard_fragment_t* udpard_fragment_next(udpard_fragment_t* const frag);
223+
udpard_fragment_t* udpard_fragment_next(const udpard_fragment_t* frag);
224224

225225
/// Copies `size` bytes of payload stored in a fragment tree starting from `offset` into `destination`.
226-
/// The given fragment can be arbitrary; the function will seek the required starting fragment automatically.
226+
/// The cursor pointer is an iterator updated to the last fragment touched, enabling very efficient sequential
227+
/// access without repeated searches; it is never set to NULL.
227228
/// Returns the number of bytes copied into the contiguous destination buffer, which equals `size` unless
228229
/// `offset+size` exceeds the amount of data stored in the fragments.
229-
/// The function has no effect and returns zero if the destination buffer or fragment pointer are NULL.
230-
size_t udpard_fragment_gather(const udpard_fragment_t* const frag,
231-
const size_t offset,
232-
const size_t size,
233-
void* const destination);
230+
/// The function has no effect and returns zero if the destination buffer or iterator pointer are NULL.
231+
size_t udpard_fragment_gather(const udpard_fragment_t** cursor,
232+
const size_t offset,
233+
const size_t size,
234+
void* const destination);
234235

235236
// =====================================================================================================================
236237
// ================================================= TX PIPELINE =================================================

‎tests/src/test_e2e.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ void on_message(udpard_rx_t* const rx, udpard_rx_port_t* const port, const udpar
8787
TEST_ASSERT(it != ctx->expected.end());
8888

8989
// Gather fragments into a contiguous buffer so we can compare the stored prefix (payload may be truncated).
90-
std::vector<uint8_t> assembled(transfer.payload_size_stored);
91-
const size_t gathered = udpard_fragment_gather(
92-
transfer.payload, 0, transfer.payload_size_stored, (transfer.payload_size_stored > 0U) ? assembled.data() : nullptr);
90+
std::vector<uint8_t> assembled(transfer.payload_size_stored);
91+
const udpard_fragment_t* payload_cursor = transfer.payload;
92+
const size_t gathered = udpard_fragment_gather(&payload_cursor, 0, transfer.payload_size_stored, assembled.data());
9393
TEST_ASSERT_EQUAL_size_t(transfer.payload_size_stored, gathered);
9494
TEST_ASSERT_TRUE(transfer.payload_size_stored <= it->second.payload.size());
9595
TEST_ASSERT_EQUAL_size_t(it->second.payload_size_wire, transfer.payload_size_wire);

‎tests/src/test_fragment.cpp‎

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -160,38 +160,47 @@ void test_udpard_fragment_gather()
160160
const udpard_mem_deleter_t del_payload = instrumented_allocator_make_deleter(&alloc_payload);
161161

162162
// Test 1: NULL fragment returns 0.
163-
char buf[100]; // NOLINT(*-avoid-c-arrays)
164-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(nullptr, 0, sizeof(buf), static_cast<void*>(buf)));
163+
char buf[100]; // NOLINT(*-avoid-c-arrays)
164+
const udpard_fragment_t* null_frag = nullptr;
165+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(&null_frag, 0, sizeof(buf), static_cast<void*>(buf)));
165166

166167
// Test 2: NULL destination returns 0.
167168
udpard_fragment_t* const single = make_test_fragment(mem_frag, mem_payload, del_payload, 0, 5, "hello");
168169
TEST_ASSERT_NOT_NULL(single);
169-
single->index_offset.up = nullptr;
170-
single->index_offset.lr[0] = nullptr;
171-
single->index_offset.lr[1] = nullptr;
172-
single->index_offset.bf = 0;
173-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(single, 0, sizeof(buf), nullptr));
170+
single->index_offset.up = nullptr;
171+
single->index_offset.lr[0] = nullptr;
172+
single->index_offset.lr[1] = nullptr;
173+
single->index_offset.bf = 0;
174+
const udpard_fragment_t* cursor = single;
175+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(&cursor, 0, sizeof(buf), nullptr));
176+
TEST_ASSERT_EQUAL_PTR(single, cursor);
174177

175178
// Test 3: Single fragment - gather all.
176179
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
177-
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(single, 0, sizeof(buf), static_cast<void*>(buf)));
180+
cursor = single;
181+
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(&cursor, 0, sizeof(buf), static_cast<void*>(buf)));
178182
TEST_ASSERT_EQUAL_MEMORY("hello", buf, 5);
183+
TEST_ASSERT_EQUAL_PTR(single, cursor);
179184

180185
// Test 4: Single fragment - truncation (destination smaller than fragment).
181186
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
182-
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(single, 0, 3, static_cast<void*>(buf)));
187+
cursor = single;
188+
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(&cursor, 0, 3, static_cast<void*>(buf)));
183189
TEST_ASSERT_EQUAL_MEMORY("hel", buf, 3);
190+
TEST_ASSERT_EQUAL_PTR(single, cursor);
184191

185192
// Test 5: Single fragment - offset into the payload.
186193
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
187-
TEST_ASSERT_EQUAL_size_t(2, udpard_fragment_gather(single, 2, 2, static_cast<void*>(buf)));
194+
cursor = single;
195+
TEST_ASSERT_EQUAL_size_t(2, udpard_fragment_gather(&cursor, 2, 2, static_cast<void*>(buf)));
188196
TEST_ASSERT_EQUAL_MEMORY("ll", buf, 2);
197+
TEST_ASSERT_EQUAL_PTR(single, cursor);
189198

190199
// Cleanup single fragment.
191200
mem_payload.free(mem_payload.user, single->origin.size, single->origin.data);
192201
mem_frag.free(mem_frag.user, sizeof(udpard_fragment_t), single);
193202

194-
// Test 5: Multiple fragments forming a tree.
203+
// Test 6: Multiple fragments forming a tree.
195204
// Create tree: root at offset 5 ("MID"), left at offset 0 ("ABCDE"), right at offset 8 ("WXYZ")
196205
// Total payload when gathered: "ABCDE" + "MID" + "WXYZ" = "ABCDEMIDWXYZ" (12 bytes)
197206
udpard_fragment_t* const root = make_test_fragment(mem_frag, mem_payload, del_payload, 5, 3, "MID");
@@ -219,50 +228,70 @@ void test_udpard_fragment_gather()
219228

220229
// Gather from root - should collect all fragments in order.
221230
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
222-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(root, 0, sizeof(buf), static_cast<void*>(buf)));
231+
cursor = root;
232+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(&cursor, 0, sizeof(buf), static_cast<void*>(buf)));
223233
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
234+
TEST_ASSERT_EQUAL_PTR(right, cursor);
224235

225236
// Gather from left child - should still collect all fragments (traverses to root first).
226237
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
227-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(left, 0, sizeof(buf), static_cast<void*>(buf)));
238+
cursor = left;
239+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(&cursor, 0, sizeof(buf), static_cast<void*>(buf)));
228240
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
241+
TEST_ASSERT_EQUAL_PTR(right, cursor);
229242

230243
// Gather from right child - should still collect all fragments.
231244
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
232-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(right, 0, sizeof(buf), static_cast<void*>(buf)));
245+
cursor = right;
246+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(&cursor, 0, sizeof(buf), static_cast<void*>(buf)));
233247
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
248+
TEST_ASSERT_EQUAL_PTR(right, cursor);
234249

235-
// Test 6: Truncation with multiple fragments - buffer smaller than total.
250+
// Test 7: Truncation with multiple fragments - buffer smaller than total.
236251
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
237-
TEST_ASSERT_EQUAL_size_t(7, udpard_fragment_gather(root, 0, 7, static_cast<void*>(buf)));
252+
cursor = root;
253+
TEST_ASSERT_EQUAL_size_t(7, udpard_fragment_gather(&cursor, 0, 7, static_cast<void*>(buf)));
238254
TEST_ASSERT_EQUAL_MEMORY("ABCDEMI", buf, 7);
255+
TEST_ASSERT_EQUAL_PTR(root, cursor);
239256

240-
// Test 7: Truncation mid-fragment.
257+
// Test 8: Truncation mid-fragment.
241258
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
242-
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(root, 0, 3, static_cast<void*>(buf)));
259+
cursor = root;
260+
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(&cursor, 0, 3, static_cast<void*>(buf)));
243261
TEST_ASSERT_EQUAL_MEMORY("ABC", buf, 3);
262+
TEST_ASSERT_EQUAL_PTR(left, cursor);
244263

245-
// Test 8: Offset across fragments.
264+
// Test 9: Offset across fragments.
246265
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
247-
TEST_ASSERT_EQUAL_size_t(6, udpard_fragment_gather(root, 2, 6, static_cast<void*>(buf)));
266+
cursor = root;
267+
TEST_ASSERT_EQUAL_size_t(6, udpard_fragment_gather(&cursor, 2, 6, static_cast<void*>(buf)));
248268
TEST_ASSERT_EQUAL_MEMORY("CDEMID", buf, 6);
269+
TEST_ASSERT_EQUAL_PTR(root, cursor);
249270

250-
// Test 9: Start on fragment boundary, span into next.
271+
// Test 10: Start on fragment boundary, span into next.
251272
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
252-
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(root, 5, 5, static_cast<void*>(buf)));
273+
cursor = root;
274+
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(&cursor, 5, 5, static_cast<void*>(buf)));
253275
TEST_ASSERT_EQUAL_MEMORY("MIDWX", buf, 5);
276+
TEST_ASSERT_EQUAL_PTR(right, cursor);
254277

255-
// Test 10: Start inside last fragment with request beyond stored data.
278+
// Test 11: Start inside last fragment with request beyond stored data.
256279
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
257-
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(root, 9, 10, static_cast<void*>(buf)));
280+
cursor = root;
281+
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(&cursor, 9, 10, static_cast<void*>(buf)));
258282
TEST_ASSERT_EQUAL_MEMORY("XYZ", buf, 3);
283+
TEST_ASSERT_EQUAL_PTR(right, cursor);
259284

260-
// Test 11: Offset beyond available payload.
285+
// Test 12: Offset beyond available payload.
261286
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
262-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(root, 100, sizeof(buf), static_cast<void*>(buf)));
263-
264-
// Test 12: Zero-size destination.
265-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(root, 0, 0, static_cast<void*>(buf)));
287+
cursor = root;
288+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(&cursor, 100, sizeof(buf), static_cast<void*>(buf)));
289+
TEST_ASSERT_EQUAL_PTR(root, cursor);
290+
291+
// Test 13: Zero-size destination.
292+
cursor = root;
293+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(&cursor, 0, 0, static_cast<void*>(buf)));
294+
TEST_ASSERT_EQUAL_PTR(root, cursor);
266295

267296
// Cleanup.
268297
mem_payload.free(mem_payload.user, left->origin.size, left->origin.data);

0 commit comments

Comments
 (0)