Skip to content

Commit c9c29ab

Browse files
authored
Sshirokov/v2 single tx copy (#60)
- `udpardTxPeek` now returns mutable item - needed for payload ownership transfer. - `udpardTxPop` now accepts mutable item - eliminated private `TxItem` Also: - fixed Sonar warning: property 'sonar.cfamily.build-wrapper-output' is deprecated
1 parent 3d9f5ef commit c9c29ab

File tree

7 files changed

+514
-399
lines changed

7 files changed

+514
-399
lines changed

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ For complete usage examples, please refer to <https://github.com/OpenCyphal-Gara
7878

7979
## Revisions
8080

81+
### TODO: Add new v2
82+
☝ todo will be addressed as soon as the new API is stable.
83+
8184
### v1.0
8285

8386
Initial release.

libudpard/udpard.c

Lines changed: 82 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "udpard.h"
77
#include "_udpard_cavl.h"
8+
89
#include <string.h>
910

1011
// --------------------------------------------- BUILD CONFIGURATION ---------------------------------------------
@@ -271,55 +272,56 @@ static inline uint32_t transferCRCCompute(const size_t size, const void* const d
271272
// ================================================= TX PIPELINE =================================================
272273
// =====================================================================================================================
273274

274-
/// This is a subclass of UdpardTxItem. A pointer to this type can be cast to UdpardTxItem safely.
275-
/// This is compliant with the C99 standard; paragraph 6.7.2.1.15 says:
276-
/// A pointer to a structure object, suitably converted, points to its initial member (or if that member is a
277-
/// bit-field, then to the unit in which it resides), and vice versa. There may be unnamed padding within a
278-
/// structure object, but not at its beginning.
279-
typedef struct
280-
{
281-
struct UdpardTxItem base;
282-
enum UdpardPriority priority; ///< Do we need this exposed in the public structure? We already have DSCP there.
283-
// The MISRA violation here is hard to get rid of without having to allocate a separate memory block for the
284-
// payload, which is much more costly risk-wise.
285-
byte_t payload_buffer[]; // NOSONAR MISRA C 18.7 Flexible array member.
286-
} TxItem;
287-
288275
/// Chain of TX frames prepared for insertion into a TX queue.
289276
typedef struct
290277
{
291-
TxItem* head;
292-
TxItem* tail;
293-
size_t count;
278+
struct UdpardTxItem* head;
279+
struct UdpardTxItem* tail;
280+
size_t count;
294281
} TxChain;
295282

296-
static inline TxItem* txNewItem(const struct UdpardMemoryResource memory,
297-
const uint_least8_t dscp_value_per_priority[UDPARD_PRIORITY_MAX + 1U],
298-
const UdpardMicrosecond deadline_usec,
299-
const enum UdpardPriority priority,
300-
const struct UdpardUDPIPEndpoint endpoint,
301-
const size_t datagram_payload_size,
302-
void* const user_transfer_reference)
283+
static inline bool txValidateMemoryResources(const struct UdpardTxMemoryResources memory)
284+
{
285+
return (memory.fragment.allocate != NULL) && (memory.fragment.deallocate != NULL) &&
286+
(memory.payload.allocate != NULL) && (memory.payload.deallocate != NULL);
287+
}
288+
289+
static inline struct UdpardTxItem* txNewItem(const struct UdpardTxMemoryResources memory,
290+
const uint_least8_t dscp_value_per_priority[UDPARD_PRIORITY_MAX + 1U],
291+
const UdpardMicrosecond deadline_usec,
292+
const enum UdpardPriority priority,
293+
const struct UdpardUDPIPEndpoint endpoint,
294+
const size_t datagram_payload_size,
295+
void* const user_transfer_reference)
303296
{
304-
TxItem* const out = (TxItem*) memAlloc(memory, sizeof(TxItem) + datagram_payload_size);
297+
struct UdpardTxItem* out = memAlloc(memory.fragment, sizeof(struct UdpardTxItem));
305298
if (out != NULL)
306299
{
307300
// No tree linkage by default.
308-
out->base.base.up = NULL;
309-
out->base.base.lr[0] = NULL;
310-
out->base.base.lr[1] = NULL;
311-
out->base.base.bf = 0;
301+
out->base.up = NULL;
302+
out->base.lr[0] = NULL;
303+
out->base.lr[1] = NULL;
304+
out->base.bf = 0;
312305
// Init metadata.
313-
out->priority = priority;
314-
out->base.next_in_transfer = NULL; // Last by default.
315-
out->base.deadline_usec = deadline_usec;
306+
out->priority = priority;
307+
out->next_in_transfer = NULL; // Last by default.
308+
out->deadline_usec = deadline_usec;
316309
UDPARD_ASSERT(priority <= UDPARD_PRIORITY_MAX);
317-
out->base.dscp = dscp_value_per_priority[priority];
318-
out->base.destination = endpoint;
319-
out->base.user_transfer_reference = user_transfer_reference;
320-
// The payload points to the buffer already allocated.
321-
out->base.datagram_payload.size = datagram_payload_size;
322-
out->base.datagram_payload.data = &out->payload_buffer[0];
310+
out->dscp = dscp_value_per_priority[priority];
311+
out->destination = endpoint;
312+
out->user_transfer_reference = user_transfer_reference;
313+
314+
void* const payload_data = memAlloc(memory.payload, datagram_payload_size);
315+
if (NULL != payload_data)
316+
{
317+
out->datagram_payload.data = payload_data;
318+
out->datagram_payload.size = datagram_payload_size;
319+
}
320+
else
321+
{
322+
memFree(memory.fragment, sizeof(struct UdpardTxItem), out);
323+
out = NULL;
324+
}
323325
}
324326
return out;
325327
}
@@ -329,8 +331,8 @@ static inline TxItem* txNewItem(const struct UdpardMemoryResource memory,
329331
static inline int_fast8_t txAVLPredicate(void* const user_reference, // NOSONAR Cavl API requires pointer to non-const.
330332
const struct UdpardTreeNode* const node)
331333
{
332-
const TxItem* const target = (const TxItem*) user_reference;
333-
const TxItem* const other = (const TxItem*) (const void*) node;
334+
const struct UdpardTxItem* const target = (const struct UdpardTxItem*) user_reference;
335+
const struct UdpardTxItem* const other = (const struct UdpardTxItem*) (const void*) node;
334336
UDPARD_ASSERT((target != NULL) && (other != NULL));
335337
return (target->priority >= other->priority) ? +1 : -1;
336338
}
@@ -390,14 +392,14 @@ static inline byte_t* txSerializeHeader(byte_t* const destination_buffe
390392

391393
/// Produces a chain of Tx queue items for later insertion into the Tx queue. The tail is NULL if OOM.
392394
/// The caller is responsible for freeing the memory allocated for the chain.
393-
static inline TxChain txMakeChain(const struct UdpardMemoryResource memory,
394-
const uint_least8_t dscp_value_per_priority[UDPARD_PRIORITY_MAX + 1U],
395-
const size_t mtu,
396-
const UdpardMicrosecond deadline_usec,
397-
const TransferMetadata meta,
398-
const struct UdpardUDPIPEndpoint endpoint,
399-
const struct UdpardPayload payload,
400-
void* const user_transfer_reference)
395+
static inline TxChain txMakeChain(const struct UdpardTxMemoryResources memory,
396+
const uint_least8_t dscp_value_per_priority[UDPARD_PRIORITY_MAX + 1U],
397+
const size_t mtu,
398+
const UdpardMicrosecond deadline_usec,
399+
const TransferMetadata meta,
400+
const struct UdpardUDPIPEndpoint endpoint,
401+
const struct UdpardPayload payload,
402+
void* const user_transfer_reference)
401403
{
402404
UDPARD_ASSERT(mtu > 0);
403405
UDPARD_ASSERT((payload.data != NULL) || (payload.size == 0U));
@@ -408,13 +410,13 @@ static inline TxChain txMakeChain(const struct UdpardMemoryResource memory,
408410
size_t offset = 0U;
409411
while (offset < payload_size_with_crc)
410412
{
411-
TxItem* const item = txNewItem(memory,
412-
dscp_value_per_priority,
413-
deadline_usec,
414-
meta.priority,
415-
endpoint,
416-
smaller(payload_size_with_crc - offset, mtu) + HEADER_SIZE_BYTES,
417-
user_transfer_reference);
413+
struct UdpardTxItem* const item = txNewItem(memory,
414+
dscp_value_per_priority,
415+
deadline_usec,
416+
meta.priority,
417+
endpoint,
418+
smaller(payload_size_with_crc - offset, mtu) + HEADER_SIZE_BYTES,
419+
user_transfer_reference);
418420
if (NULL == out.head)
419421
{
420422
out.head = item;
@@ -423,15 +425,16 @@ static inline TxChain txMakeChain(const struct UdpardMemoryResource memory,
423425
{
424426
// C std, 6.7.2.1.15: A pointer to a structure object <...> points to its initial member, and vice versa.
425427
// Can't just read tqi->base because tqi may be NULL; https://github.com/OpenCyphal/libcanard/issues/203.
426-
out.tail->base.next_in_transfer = (struct UdpardTxItem*) item;
428+
out.tail->next_in_transfer = item;
427429
}
428430
out.tail = item;
429431
if (NULL == out.tail)
430432
{
431433
break;
432434
}
433-
const bool last = (payload_size_with_crc - offset) <= mtu;
434-
byte_t* write_ptr = txSerializeHeader(&item->payload_buffer[0], meta, (uint32_t) out.count, last);
435+
const bool last = (payload_size_with_crc - offset) <= mtu;
436+
byte_t* const dst_buffer = item->datagram_payload.data;
437+
byte_t* write_ptr = txSerializeHeader(dst_buffer, meta, (uint32_t) out.count, last);
435438
if (offset < payload.size)
436439
{
437440
const size_t progress = smaller(payload.size - offset, mtu);
@@ -446,7 +449,7 @@ static inline TxChain txMakeChain(const struct UdpardMemoryResource memory,
446449
{
447450
const size_t crc_offset = offset - payload.size;
448451
UDPARD_ASSERT(crc_offset < TRANSFER_CRC_SIZE_BYTES);
449-
const size_t available = item->base.datagram_payload.size - (size_t) (write_ptr - &item->payload_buffer[0]);
452+
const size_t available = item->datagram_payload.size - (size_t) (write_ptr - dst_buffer);
450453
UDPARD_ASSERT(available <= TRANSFER_CRC_SIZE_BYTES);
451454
const size_t write_size = smaller(TRANSFER_CRC_SIZE_BYTES - crc_offset, available);
452455
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
@@ -495,7 +498,7 @@ static inline int32_t txPush(struct UdpardTx* const tx,
495498
if (chain.tail != NULL)
496499
{
497500
UDPARD_ASSERT(frame_count == chain.count);
498-
struct UdpardTxItem* next = &chain.head->base;
501+
struct UdpardTxItem* next = chain.head;
499502
do
500503
{
501504
const struct UdpardTreeNode* const res =
@@ -513,11 +516,11 @@ static inline int32_t txPush(struct UdpardTx* const tx,
513516
else // The queue is large enough but we ran out of heap memory, so we have to unwind the chain.
514517
{
515518
out = -UDPARD_ERROR_MEMORY;
516-
struct UdpardTxItem* head = &chain.head->base;
519+
struct UdpardTxItem* head = chain.head;
517520
while (head != NULL)
518521
{
519522
struct UdpardTxItem* const next = head->next_in_transfer;
520-
memFree(tx->memory, sizeof(TxItem) + head->datagram_payload.size, head);
523+
udpardTxFree(tx->memory, head);
521524
head = next;
522525
}
523526
}
@@ -526,13 +529,13 @@ static inline int32_t txPush(struct UdpardTx* const tx,
526529
return out;
527530
}
528531

529-
int_fast8_t udpardTxInit(struct UdpardTx* const self,
530-
const UdpardNodeID* const local_node_id,
531-
const size_t queue_capacity,
532-
const struct UdpardMemoryResource memory)
532+
int_fast8_t udpardTxInit(struct UdpardTx* const self,
533+
const UdpardNodeID* const local_node_id,
534+
const size_t queue_capacity,
535+
const struct UdpardTxMemoryResources memory)
533536
{
534537
int_fast8_t ret = -UDPARD_ERROR_ARGUMENT;
535-
if ((NULL != self) && (NULL != local_node_id) && (memory.allocate != NULL) && (memory.deallocate != NULL))
538+
if ((NULL != self) && (NULL != local_node_id) && txValidateMemoryResources(memory))
536539
{
537540
ret = 0;
538541
memZero(sizeof(*self), self);
@@ -639,27 +642,22 @@ int32_t udpardTxRespond(struct UdpardTx* const self,
639642
return out;
640643
}
641644

642-
const struct UdpardTxItem* udpardTxPeek(const struct UdpardTx* const self)
645+
struct UdpardTxItem* udpardTxPeek(const struct UdpardTx* const self)
643646
{
644-
const struct UdpardTxItem* out = NULL;
647+
struct UdpardTxItem* out = NULL;
645648
if (self != NULL)
646649
{
647650
// Paragraph 6.7.2.1.15 of the C standard says:
648651
// A pointer to a structure object, suitably converted, points to its initial member, and vice versa.
649-
out = (const struct UdpardTxItem*) (void*) cavlFindExtremum(self->root, false);
652+
out = (struct UdpardTxItem*) (void*) cavlFindExtremum(self->root, false);
650653
}
651654
return out;
652655
}
653656

654-
struct UdpardTxItem* udpardTxPop(struct UdpardTx* const self, const struct UdpardTxItem* const item)
657+
struct UdpardTxItem* udpardTxPop(struct UdpardTx* const self, struct UdpardTxItem* const item)
655658
{
656-
struct UdpardTxItem* out = NULL;
657659
if ((self != NULL) && (item != NULL))
658660
{
659-
// Intentional violation of MISRA: casting away const qualifier. This is considered safe because the API
660-
// contract dictates that the pointer shall point to a mutable entity in RAM previously allocated by the
661-
// memory manager. It is difficult to avoid this cast in this context.
662-
out = (struct UdpardTxItem*) item; // NOSONAR casting away const qualifier.
663661
// Paragraph 6.7.2.1.15 of the C standard says:
664662
// A pointer to a structure object, suitably converted, points to its initial member, and vice versa.
665663
// Note that the highest-priority frame is always a leaf node in the AVL tree, which means that it is very
@@ -668,14 +666,19 @@ struct UdpardTxItem* udpardTxPop(struct UdpardTx* const self, const struct Udpar
668666
UDPARD_ASSERT(self->queue_size > 0U);
669667
self->queue_size--;
670668
}
671-
return out;
669+
return item;
672670
}
673671

674-
void udpardTxFree(const struct UdpardMemoryResource memory, struct UdpardTxItem* const item)
672+
void udpardTxFree(const struct UdpardTxMemoryResources memory, struct UdpardTxItem* const item)
675673
{
676674
if (item != NULL)
677675
{
678-
memFree(memory, sizeof(TxItem) + item->datagram_payload.size, item);
676+
if (item->datagram_payload.data != NULL)
677+
{
678+
memFree(memory.payload, item->datagram_payload.size, item->datagram_payload.data);
679+
}
680+
681+
memFree(memory.fragment, sizeof(struct UdpardTxItem), item);
679682
}
680683
}
681684

0 commit comments

Comments
 (0)