Skip to content

Commit 5d71d60

Browse files
pavel-kirienkopavel-kirienko
committed
update udpard_fragment_gather
1 parent 35d6248 commit 5d71d60

File tree

5 files changed

+70
-35
lines changed

5 files changed

+70
-35
lines changed

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@
33
Please read README.md for general information about LibUDPard.
44
Read `.clang-format` and `**/.clang-tidy` to understand the coding style used in this project.
55
Keep the code and comments very brief. Be sure every significant code block is preceded with a brief comment.
6+
7+
When building the code, don't hesitate to use multiple jobs to use all CPU cores.

‎libudpard/udpard.c‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -174,26 +174,34 @@ udpard_fragment_t* udpard_fragment_next(udpard_fragment_t* const frag)
174174
}
175175

176176
size_t udpard_fragment_gather(const udpard_fragment_t* any_frag,
177-
const size_t destination_size_bytes,
177+
const size_t offset,
178+
const size_t size,
178179
void* const destination)
179180
{
180-
size_t offset = 0;
181+
size_t copied = 0;
181182
if ((any_frag != NULL) && (destination != NULL)) {
182-
while (any_frag->index_offset.up != NULL) { // Locate the root.
183-
any_frag = (const udpard_fragment_t*)any_frag->index_offset.up;
184-
}
185-
udpard_fragment_t* frag = (udpard_fragment_t*)cavl2_min((udpard_tree_t*)&any_frag->index_offset);
186-
while ((frag != NULL) && (offset < destination_size_bytes)) {
187-
UDPARD_ASSERT(frag->offset == offset);
183+
udpard_fragment_t* frag = udpard_fragment_seek((udpard_fragment_t*)any_frag, offset);
184+
size_t cursor = offset;
185+
byte_t* const out = (byte_t*)destination;
186+
// Copy contiguous fragments starting at the requested offset.
187+
while ((frag != NULL) && (copied < size)) {
188+
UDPARD_ASSERT(frag->offset <= cursor);
189+
UDPARD_ASSERT((frag->offset + frag->view.size) > cursor);
188190
UDPARD_ASSERT(frag->view.data != NULL);
189-
const size_t to_copy = smaller(frag->view.size, destination_size_bytes - offset);
191+
const size_t offset_in_frag = cursor - frag->offset;
192+
const size_t available = frag->view.size - offset_in_frag;
193+
const size_t to_copy = smaller(available, size - copied);
190194
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
191-
(void)memcpy((byte_t*)destination + offset, frag->view.data, to_copy);
192-
offset += to_copy;
193-
frag = udpard_fragment_next(frag);
195+
(void)memcpy(out + copied, ((const byte_t*)frag->view.data) + offset_in_frag, to_copy);
196+
copied += to_copy;
197+
cursor += to_copy;
198+
if (copied < size) {
199+
frag = udpard_fragment_next(frag);
200+
UDPARD_ASSERT((frag == NULL) || (frag->offset == cursor));
201+
}
194202
}
195203
}
196-
return offset;
204+
return copied;
197205
}
198206

199207
// --------------------------------------------- CRC ---------------------------------------------

‎libudpard/udpard.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,14 @@ udpard_fragment_t* udpard_fragment_seek(udpard_fragment_t* any_frag, const size_
222222
/// The complexity is amortized-constant.
223223
udpard_fragment_t* udpard_fragment_next(udpard_fragment_t* const frag);
224224

225-
/// Given any fragment in a transfer, copies the entire transfer payload into the specified contiguous buffer.
226-
/// If the total size of all fragments combined exceeds the size of the destination buffer,
227-
/// copying will stop early after the buffer is filled, thus truncating the fragmented data short.
228-
/// The function has no effect and returns zero if the destination buffer or fragment pointer is NULL.
229-
/// Returns the number of bytes copied into the contiguous destination buffer.
225+
/// 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.
227+
/// Returns the number of bytes copied into the contiguous destination buffer, which equals `size` unless
228+
/// `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.
230230
size_t udpard_fragment_gather(const udpard_fragment_t* any_frag,
231-
const size_t destination_size_bytes,
231+
const size_t offset,
232+
const size_t size,
232233
void* const destination);
233234

234235
// =====================================================================================================================
@@ -489,8 +490,8 @@ void udpard_tx_free(const udpard_tx_mem_resources_t memory, udpard_tx_item_t* co
489490
/// UNORDERED Unique transfer-ID Ordering not guaranteed UDPARD_RX_REORDERING_WINDOW_UNORDERED
490491
/// STATELESS Constant time, constant memory 1-frame only, dups, no responses UDPARD_RX_REORDERING_WINDOW_STATELESS
491492
///
492-
/// If not sure, choose the ORDERED mode with a ~1 ms reordering window for all topics except for request-response
493-
/// RPC-style topics, in which case choose UNORDERED. The STATELESS mode is chiefly intended for the heartbeat topic.
493+
/// If not sure, choose UNORDERED. The ORDERED mode is a good fit for ordering-sensitive use cases like state estimators
494+
/// and control loops, but it is not suitable for P2P. The STATELESS mode is chiefly intended for the heartbeat topic.
494495
///
495496
/// ORDERED
496497
///
@@ -712,8 +713,8 @@ void udpard_rx_poll(udpard_rx_t* const self, const udpard_us_t now);
712713
/// The collision callback is invoked if a topic hash collision is detected.
713714
/// For P2P ports, the topic hash is populated with the local node's UID instead.
714715
///
715-
/// If not sure, set the reordering window to ~1 ms for most topics, but use UNORDERED for request-response topics
716-
/// and for P2P.
716+
/// If not sure which reassembly mode to choose, consider UDPARD_RX_REORDERING_WINDOW_UNORDERED as the default choice.
717+
/// For ordering-sensitive use cases, such as state estimators and control loops, use ORDERED with a short window.
717718
///
718719
/// The return value is true on success, false if any of the arguments are invalid.
719720
/// The time complexity is constant. This function does not invoke the dynamic memory manager.

‎tests/src/test_e2e.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ void on_message(udpard_rx_t* const rx, udpard_rx_port_t* const port, const udpar
8989
// Gather fragments into a contiguous buffer so we can compare the stored prefix (payload may be truncated).
9090
std::vector<uint8_t> assembled(transfer.payload_size_stored);
9191
const size_t gathered = udpard_fragment_gather(
92-
transfer.payload, transfer.payload_size_stored, (transfer.payload_size_stored > 0U) ? assembled.data() : nullptr);
92+
transfer.payload, 0, transfer.payload_size_stored, (transfer.payload_size_stored > 0U) ? assembled.data() : nullptr);
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: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void test_udpard_fragment_gather()
161161

162162
// Test 1: NULL fragment returns 0.
163163
char buf[100]; // NOLINT(*-avoid-c-arrays)
164-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(nullptr, sizeof(buf), static_cast<void*>(buf)));
164+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(nullptr, 0, sizeof(buf), static_cast<void*>(buf)));
165165

166166
// Test 2: NULL destination returns 0.
167167
udpard_fragment_t* const single = make_test_fragment(mem_frag, mem_payload, del_payload, 0, 5, "hello");
@@ -170,18 +170,23 @@ void test_udpard_fragment_gather()
170170
single->index_offset.lr[0] = nullptr;
171171
single->index_offset.lr[1] = nullptr;
172172
single->index_offset.bf = 0;
173-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(single, sizeof(buf), nullptr));
173+
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(single, 0, sizeof(buf), nullptr));
174174

175175
// Test 3: Single fragment - gather all.
176176
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
177-
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(single, sizeof(buf), static_cast<void*>(buf)));
177+
TEST_ASSERT_EQUAL_size_t(5, udpard_fragment_gather(single, 0, sizeof(buf), static_cast<void*>(buf)));
178178
TEST_ASSERT_EQUAL_MEMORY("hello", buf, 5);
179179

180180
// Test 4: Single fragment - truncation (destination smaller than fragment).
181181
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
182-
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(single, 3, static_cast<void*>(buf)));
182+
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(single, 0, 3, static_cast<void*>(buf)));
183183
TEST_ASSERT_EQUAL_MEMORY("hel", buf, 3);
184184

185+
// Test 5: Single fragment - offset into the payload.
186+
(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)));
188+
TEST_ASSERT_EQUAL_MEMORY("ll", buf, 2);
189+
185190
// Cleanup single fragment.
186191
mem_payload.free(mem_payload.user, single->origin.size, single->origin.data);
187192
mem_frag.free(mem_frag.user, sizeof(udpard_fragment_t), single);
@@ -214,31 +219,50 @@ void test_udpard_fragment_gather()
214219

215220
// Gather from root - should collect all fragments in order.
216221
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
217-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(root, sizeof(buf), static_cast<void*>(buf)));
222+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(root, 0, sizeof(buf), static_cast<void*>(buf)));
218223
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
219224

220225
// Gather from left child - should still collect all fragments (traverses to root first).
221226
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
222-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(left, sizeof(buf), static_cast<void*>(buf)));
227+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(left, 0, sizeof(buf), static_cast<void*>(buf)));
223228
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
224229

225230
// Gather from right child - should still collect all fragments.
226231
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
227-
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(right, sizeof(buf), static_cast<void*>(buf)));
232+
TEST_ASSERT_EQUAL_size_t(12, udpard_fragment_gather(right, 0, sizeof(buf), static_cast<void*>(buf)));
228233
TEST_ASSERT_EQUAL_MEMORY("ABCDEMIDWXYZ", buf, 12);
229234

230235
// Test 6: Truncation with multiple fragments - buffer smaller than total.
231236
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
232-
TEST_ASSERT_EQUAL_size_t(7, udpard_fragment_gather(root, 7, static_cast<void*>(buf)));
237+
TEST_ASSERT_EQUAL_size_t(7, udpard_fragment_gather(root, 0, 7, static_cast<void*>(buf)));
233238
TEST_ASSERT_EQUAL_MEMORY("ABCDEMI", buf, 7);
234239

235240
// Test 7: Truncation mid-fragment.
236241
(void)std::memset(static_cast<void*>(buf), 0, sizeof(buf));
237-
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(root, 3, static_cast<void*>(buf)));
242+
TEST_ASSERT_EQUAL_size_t(3, udpard_fragment_gather(root, 0, 3, static_cast<void*>(buf)));
238243
TEST_ASSERT_EQUAL_MEMORY("ABC", buf, 3);
239244

240-
// Test 8: Zero-size destination.
241-
TEST_ASSERT_EQUAL_size_t(0, udpard_fragment_gather(root, 0, static_cast<void*>(buf)));
245+
// Test 8: Offset across fragments.
246+
(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)));
248+
TEST_ASSERT_EQUAL_MEMORY("CDEMID", buf, 6);
249+
250+
// Test 9: Start on fragment boundary, span into next.
251+
(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)));
253+
TEST_ASSERT_EQUAL_MEMORY("MIDWX", buf, 5);
254+
255+
// Test 10: Start inside last fragment with request beyond stored data.
256+
(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)));
258+
TEST_ASSERT_EQUAL_MEMORY("XYZ", buf, 3);
259+
260+
// Test 11: Offset beyond available payload.
261+
(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)));
242266

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

0 commit comments

Comments
 (0)