diff --git a/Bdd/Targets/FreeRtos/main.c b/Bdd/Targets/FreeRtos/main.c index 98d5466b..10aed951 100644 --- a/Bdd/Targets/FreeRtos/main.c +++ b/Bdd/Targets/FreeRtos/main.c @@ -169,8 +169,7 @@ enum BDD_TARGET_BUFFER_MESSAGES = 8 }; -static SolidSyslogCircularBufferStorage - bufferStorage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(BDD_TARGET_BUFFER_MESSAGES)]; +static uint8_t bufferRing[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(BDD_TARGET_BUFFER_MESSAGES)]; static SolidSyslogFreeRtosMutexStorage mutexStorage; /* Lifecycle mutex serialises SolidSyslog_Service against the rebuild path @@ -825,7 +824,7 @@ static void InteractiveTask(void* argument) * is the Service task; its Write side is whichever task calls * SolidSyslog_Log. */ bufferMutex = SolidSyslogFreeRtosMutex_Create(&mutexStorage); - buffer = SolidSyslogCircularBuffer_Create(bufferStorage, sizeof(bufferStorage), bufferMutex); + buffer = SolidSyslogCircularBuffer_Create(bufferMutex, bufferRing, sizeof(bufferRing)); /* Lifecycle mutex created up front so the Service task can take it * from its very first iteration without a NULL check. */ diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index 5d20d068..682d1404 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -66,8 +66,7 @@ static const char* const THRESHOLD_MARKER_PATH = "Bdd/output/solidsyslog_thresho static SolidSyslogWinsockTcpStreamStorage tcpStreamStorage; static SolidSyslogStreamSenderStorage tcpSenderStorage; -static SolidSyslogCircularBufferStorage - bufferStorage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(BDD_TARGET_BUFFER_MESSAGES)]; +static uint8_t bufferRing[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(BDD_TARGET_BUFFER_MESSAGES)]; static SolidSyslogWindowsMutexStorage mutexStorage; static SolidSyslogWindowsAtomicCounterStorage counterStorage; static volatile bool shutdownFlag; @@ -304,7 +303,7 @@ int BddTargetWindows_Run(int argc, char* argv[]) struct SolidSyslogStore* store = CreateStore(&options); struct SolidSyslogMutex* mutex = SolidSyslogWindowsMutex_Create(&mutexStorage); - struct SolidSyslogBuffer* buffer = SolidSyslogCircularBuffer_Create(bufferStorage, sizeof(bufferStorage), mutex); + struct SolidSyslogBuffer* buffer = SolidSyslogCircularBuffer_Create(mutex, bufferRing, sizeof(bufferRing)); struct SolidSyslogAtomicCounter* counter = SolidSyslogWindowsAtomicCounter_Create(&counterStorage); struct SolidSyslogMetaSdConfig metaConfig = { .Counter = counter, diff --git a/CLAUDE.md b/CLAUDE.md index b8fb7aff..2f922a70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -338,7 +338,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogBufferDefinition.h` | Buffer implementors (extension point) | `SolidSyslogBuffer` vtable struct | | `SolidSyslogNullBuffer.h` | System setup code (single-task, no buffering) | `SolidSyslogNullBuffer_Create`, `_Destroy` | | `SolidSyslogPosixMessageQueueBuffer.h` | System setup code using POSIX message queue buffer | `SolidSyslogPosixMessageQueueBuffer_Create`, `_Destroy` | -| `SolidSyslogCircularBuffer.h` | System setup code using an in-memory ring buffer (bare-metal / RTOS / Windows) | `SolidSyslogCircularBufferStorage`, `SOLIDSYSLOG_CIRCULARBUFFER_STORAGE_SIZE(maxMessages)` (friendly: N max-sized messages), `SOLIDSYSLOG_CIRCULARBUFFER_STORAGE_SIZE_BYTES(ringBytes)` (raw byte capacity), `SolidSyslogCircularBuffer_Create(storage, sizeof(storage), mutex)`, `_Destroy(buffer)` (uint16-length-prefixed records, drop-newest on overflow, no-split wrap, mutex-injected synchronization) | +| `SolidSyslogCircularBuffer.h` | System setup code using an in-memory ring buffer (bare-metal / RTOS / Windows) | `SOLIDSYSLOG_CIRCULAR_BUFFER_HEADER_BYTES`, `SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(maxMessages)` (friendly: N max-sized messages → ring bytes), `SolidSyslogCircularBuffer_Create(mutex, ring, ringBytes)`, `_Destroy(buffer)` (uint16-length-prefixed records, drop-newest on overflow, no-split wrap, mutex-injected synchronization). Instance struct lives in a library-internal static pool (E11); caller supplies the ring memory only. | | `SolidSyslogMutex.h` | Any code holding a mutex handle | `SolidSyslogMutex_Lock`, `_Unlock` | | `SolidSyslogMutexDefinition.h` | Mutex implementors (extension point) | `SolidSyslogMutex` vtable struct (`Lock`, `Unlock`) | | `SolidSyslogNullMutex.h` | System setup code (single-task, no synchronization) | `SolidSyslogNullMutex_Create`, `_Destroy` | diff --git a/CMakeLists.txt b/CMakeLists.txt index 86784b0d..59513f2d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -61,6 +61,7 @@ if(ENABLE_CPPCHECK) "--enable=warning,style,performance,portability" "--error-exitcode=1" "--suppress=missingIncludeSystem" + "--suppress=useStlAlgorithm" "--inline-suppr" "--std=c++17" ) @@ -104,6 +105,23 @@ if(SOLIDSYSLOG_OPENSSL) endif() endif() +# Allocation strategy (E11). Currently only "static" ships — internal static +# pools allocate instance bookkeeping for stateful classes. A future epic +# may add "dynamic" (heap-backed). Selecting "dynamic" now errors hard so +# downstream builds fail fast rather than silently linking against missing +# TUs. +set(SOLIDSYSLOG_ALLOCATION_STRATEGY "static" CACHE STRING + "Allocation strategy for instance bookkeeping: static (internal pools) or dynamic (heap, not yet shipped)") +set_property(CACHE SOLIDSYSLOG_ALLOCATION_STRATEGY PROPERTY STRINGS static dynamic) +if(SOLIDSYSLOG_ALLOCATION_STRATEGY STREQUAL "dynamic") + message(FATAL_ERROR + "SOLIDSYSLOG_ALLOCATION_STRATEGY=dynamic is reserved for a future epic; " + "no dynamic TUs ship yet. Set SOLIDSYSLOG_ALLOCATION_STRATEGY=static (the default).") +elseif(NOT SOLIDSYSLOG_ALLOCATION_STRATEGY STREQUAL "static") + message(FATAL_ERROR + "SOLIDSYSLOG_ALLOCATION_STRATEGY must be 'static' (got: '${SOLIDSYSLOG_ALLOCATION_STRATEGY}').") +endif() + # Build-time tunables (E21: Port-Time Configurability). Integrators override # library defaults by setting SOLIDSYSLOG_USER_TUNABLES_FILE to the absolute # path of their own header containing #define lines for whichever tunables diff --git a/Core/Interface/SolidSyslogCircularBuffer.h b/Core/Interface/SolidSyslogCircularBuffer.h index 0b5fcd99..e221a45f 100644 --- a/Core/Interface/SolidSyslogCircularBuffer.h +++ b/Core/Interface/SolidSyslogCircularBuffer.h @@ -13,29 +13,19 @@ EXTERN_C_BEGIN struct SolidSyslogBuffer; struct SolidSyslogMutex; - typedef size_t SolidSyslogCircularBufferStorage; - enum { - SOLIDSYSLOG_CIRCULAR_BUFFER_OVERHEAD = 7, SOLIDSYSLOG_CIRCULAR_BUFFER_HEADER_BYTES = sizeof(uint16_t) }; -/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- cannot compute array size as a constexpr in C */ -#define SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE_BYTES(ringBytes) \ - (SOLIDSYSLOG_CIRCULAR_BUFFER_OVERHEAD + \ - (((ringBytes) + sizeof(SolidSyslogCircularBufferStorage) - 1U) / sizeof(SolidSyslogCircularBufferStorage))) - -/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- cannot compute array size as a constexpr in C */ -#define SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(maxMessages) \ - SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE_BYTES( \ - (size_t) (maxMessages) * (SOLIDSYSLOG_MAX_MESSAGE_SIZE + SOLIDSYSLOG_CIRCULAR_BUFFER_HEADER_BYTES) \ - ) +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- C array-size const-expr; not a function-like expression */ +#define SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(maxMessages) \ + ((maxMessages) * (SOLIDSYSLOG_MAX_MESSAGE_SIZE + SOLIDSYSLOG_CIRCULAR_BUFFER_HEADER_BYTES)) struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( - SolidSyslogCircularBufferStorage * storage, - size_t storageBytes, - struct SolidSyslogMutex* mutex + struct SolidSyslogMutex * mutex, + uint8_t* ring, + size_t ringBytes ); void SolidSyslogCircularBuffer_Destroy(struct SolidSyslogBuffer * base); diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index dad2c709..930efa3e 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -28,4 +28,26 @@ #error "SOLIDSYSLOG_MAX_MESSAGE_SIZE must be >= 64" #endif +/* + * Number of SolidSyslogCircularBuffer instances the library's internal + * static pool can simultaneously hold. Each instance is a small + * bookkeeping struct (vtable, mutex pointer, ring pointer, head/tail/wrap) + * — roughly 64 bytes on a 64-bit target, 32 on a 32-bit target. The + * caller's ring memory is separate (passed to _Create). + * + * Most integrators only ever create one CircularBuffer per process; + * default 1. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if the integrator + * needs multiple concurrent buffer instances. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE < 1 +#error "SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE must be >= 1" +#endif + #endif /* SOLIDSYSLOG_TUNABLES_DEFAULTS_H */ diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 554df2a5..abb990e6 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -8,6 +8,7 @@ set(SOURCES SolidSyslogMetaSd.c SolidSyslogNullBuffer.c SolidSyslogCircularBuffer.c + SolidSyslogCircularBufferStatic.c SolidSyslogMutex.c SolidSyslogNullMutex.c SolidSyslogSender.c diff --git a/Core/Source/SolidSyslogCircularBuffer.c b/Core/Source/SolidSyslogCircularBuffer.c index d679f1b2..d1fd1d99 100644 --- a/Core/Source/SolidSyslogCircularBuffer.c +++ b/Core/Source/SolidSyslogCircularBuffer.c @@ -6,7 +6,7 @@ #include #include "SolidSyslogBufferDefinition.h" -#include "SolidSyslogMacros.h" +#include "SolidSyslogCircularBufferPrivate.h" #include "SolidSyslogMutex.h" #include "SolidSyslogTunables.h" @@ -15,28 +15,9 @@ enum HEADER_BYTES = SOLIDSYSLOG_CIRCULAR_BUFFER_HEADER_BYTES }; -struct SolidSyslogCircularBuffer -{ - struct SolidSyslogBuffer Base; - struct SolidSyslogMutex* Mutex; - size_t Capacity; - size_t Head; - size_t Tail; - size_t WrapPoint; - uint8_t Storage[]; -}; - -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogCircularBuffer) == - (SOLIDSYSLOG_CIRCULAR_BUFFER_OVERHEAD * sizeof(SolidSyslogCircularBufferStorage)), - "SOLIDSYSLOG_CIRCULAR_BUFFER_OVERHEAD does not match struct layout" -); - static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead); static void CircularBuffer_Write(struct SolidSyslogBuffer* base, const void* data, size_t size); -static inline struct SolidSyslogCircularBuffer* CircularBuffer_SelfFromStorage(SolidSyslogCircularBufferStorage* storage -); static inline struct SolidSyslogCircularBuffer* CircularBuffer_SelfFromBase(struct SolidSyslogBuffer* base); static inline bool CircularBuffer_IsEmpty(const struct SolidSyslogCircularBuffer* self); @@ -51,44 +32,35 @@ static inline void CircularBuffer_StoreRecord(struct SolidSyslogCircularBuffer* static inline void CircularBuffer_LoadRecord(struct SolidSyslogCircularBuffer* self, void* data, size_t* bytesRead); static inline size_t CircularBuffer_PeekRecordSize(const struct SolidSyslogCircularBuffer* self); -struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( - SolidSyslogCircularBufferStorage* storage, - size_t storageBytes, - struct SolidSyslogMutex* mutex +void CircularBuffer_Initialise( + struct SolidSyslogBuffer* base, + struct SolidSyslogMutex* mutex, + uint8_t* ring, + size_t ringBytes ) { - struct SolidSyslogCircularBuffer* self = CircularBuffer_SelfFromStorage(storage); + struct SolidSyslogCircularBuffer* self = CircularBuffer_SelfFromBase(base); self->Base.Read = CircularBuffer_Read; self->Base.Write = CircularBuffer_Write; self->Mutex = mutex; - self->Capacity = storageBytes - sizeof(struct SolidSyslogCircularBuffer); + self->Ring = ring; + self->Capacity = ringBytes; CircularBuffer_ResetToStart(self); - return &self->Base; } -static inline struct SolidSyslogCircularBuffer* CircularBuffer_SelfFromStorage(SolidSyslogCircularBufferStorage* storage -) -{ - return (struct SolidSyslogCircularBuffer*) storage; -} - -void SolidSyslogCircularBuffer_Destroy(struct SolidSyslogBuffer* base) +void CircularBuffer_Cleanup(struct SolidSyslogBuffer* base) { struct SolidSyslogCircularBuffer* self = CircularBuffer_SelfFromBase(base); self->Base.Read = NULL; self->Base.Write = NULL; self->Mutex = NULL; + self->Ring = NULL; self->Capacity = 0; self->Head = 0; self->Tail = 0; self->WrapPoint = 0; } -static inline struct SolidSyslogCircularBuffer* CircularBuffer_SelfFromBase(struct SolidSyslogBuffer* base) -{ - return (struct SolidSyslogCircularBuffer*) base; -} - static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) { struct SolidSyslogCircularBuffer* self = CircularBuffer_SelfFromBase(base); @@ -114,6 +86,11 @@ static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, void* data, size return delivered; } +static inline struct SolidSyslogCircularBuffer* CircularBuffer_SelfFromBase(struct SolidSyslogBuffer* base) +{ + return (struct SolidSyslogCircularBuffer*) base; +} + static inline bool CircularBuffer_IsEmpty(const struct SolidSyslogCircularBuffer* self) { return self->Head == self->Tail; @@ -133,13 +110,13 @@ static inline void CircularBuffer_ConsumeWrapMarker(struct SolidSyslogCircularBu static inline size_t CircularBuffer_PeekRecordSize(const struct SolidSyslogCircularBuffer* self) { /* Little-endian read of the 2-byte length header out of the uint8_t ring. */ - return ((size_t) self->Storage[self->Head]) | (((size_t) self->Storage[self->Head + 1U]) << 8U); + return ((size_t) self->Ring[self->Head]) | (((size_t) self->Ring[self->Head + 1U]) << 8U); } static inline void CircularBuffer_LoadRecord(struct SolidSyslogCircularBuffer* self, void* data, size_t* bytesRead) { size_t recordSize = CircularBuffer_PeekRecordSize(self); - (void) memcpy(data, &self->Storage[self->Head + HEADER_BYTES], recordSize); + (void) memcpy(data, &self->Ring[self->Head + HEADER_BYTES], recordSize); self->Head += HEADER_BYTES + recordSize; *bytesRead = recordSize; } @@ -209,8 +186,8 @@ static inline void CircularBuffer_WrapTail(struct SolidSyslogCircularBuffer* sel static inline void CircularBuffer_StoreRecord(struct SolidSyslogCircularBuffer* self, const void* data, size_t size) { /* Little-endian write of the 2-byte length header into the uint8_t ring. */ - self->Storage[self->Tail] = (uint8_t) (size & 0xFFU); - self->Storage[self->Tail + 1U] = (uint8_t) ((size >> 8U) & 0xFFU); - (void) memcpy(&self->Storage[self->Tail + HEADER_BYTES], data, size); + self->Ring[self->Tail] = (uint8_t) (size & 0xFFU); + self->Ring[self->Tail + 1U] = (uint8_t) ((size >> 8U) & 0xFFU); + (void) memcpy(&self->Ring[self->Tail + HEADER_BYTES], data, size); self->Tail += HEADER_BYTES + size; } diff --git a/Core/Source/SolidSyslogCircularBufferPrivate.h b/Core/Source/SolidSyslogCircularBufferPrivate.h new file mode 100644 index 00000000..e4b59b6f --- /dev/null +++ b/Core/Source/SolidSyslogCircularBufferPrivate.h @@ -0,0 +1,30 @@ +#ifndef SOLIDSYSLOGCIRCULARBUFFERPRIVATE_H +#define SOLIDSYSLOGCIRCULARBUFFERPRIVATE_H + +#include +#include + +#include "SolidSyslogBufferDefinition.h" + +struct SolidSyslogMutex; + +struct SolidSyslogCircularBuffer +{ + struct SolidSyslogBuffer Base; + struct SolidSyslogMutex* Mutex; + uint8_t* Ring; + size_t Capacity; + size_t Head; + size_t Tail; + size_t WrapPoint; +}; + +void CircularBuffer_Initialise( + struct SolidSyslogBuffer* base, + struct SolidSyslogMutex* mutex, + uint8_t* ring, + size_t ringBytes +); +void CircularBuffer_Cleanup(struct SolidSyslogBuffer* base); + +#endif /* SOLIDSYSLOGCIRCULARBUFFERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogCircularBufferStatic.c b/Core/Source/SolidSyslogCircularBufferStatic.c new file mode 100644 index 00000000..836e274a --- /dev/null +++ b/Core/Source/SolidSyslogCircularBufferStatic.c @@ -0,0 +1,182 @@ +#include "SolidSyslogCircularBuffer.h" + +#include +#include +#include + +#include "SolidSyslogBufferDefinition.h" +#include "SolidSyslogCircularBufferPrivate.h" +#include "SolidSyslogConfigLock.h" +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogMutex; + +struct Slot +{ + struct SolidSyslogCircularBuffer Object; + bool InUse; +}; + +static bool Fallback_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead); +static void Fallback_Write(struct SolidSyslogBuffer* base, const void* data, size_t size); +static struct SolidSyslogBuffer* CircularBuffer_AcquireFirstFree(void); +static struct SolidSyslogBuffer* CircularBuffer_AcquireIfFree(size_t poolIndex); +static inline bool CircularBuffer_PoolItemIsFree(size_t poolIndex); +static inline bool CircularBuffer_PoolItemIsInUse(size_t poolIndex); +static inline struct SolidSyslogBuffer* CircularBuffer_Acquire(size_t poolIndex); +static inline void CircularBuffer_MarkInUse(size_t poolIndex); +static inline struct SolidSyslogBuffer* CircularBuffer_HandleFromIndex(size_t poolIndex); +static inline bool CircularBuffer_HandleIsValid(const struct SolidSyslogBuffer* handle); +static size_t CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); +static inline bool CircularBuffer_PoolIndexIsValid(size_t poolIndex); +static bool CircularBuffer_FreeIfInUse(size_t poolIndex); +static inline void CircularBuffer_MarkFree(size_t poolIndex); + +static struct Slot Pool[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE]; +static struct SolidSyslogBuffer Fallback = {Fallback_Write, Fallback_Read}; + +struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( + struct SolidSyslogMutex* mutex, + uint8_t* ring, + size_t ringBytes +) +{ + struct SolidSyslogBuffer* handle = CircularBuffer_AcquireFirstFree(); + if (CircularBuffer_HandleIsValid(handle)) + { + CircularBuffer_Initialise(handle, mutex, ring, ringBytes); + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_POOL_EXHAUSTED); + } + return handle; +} + +static struct SolidSyslogBuffer* CircularBuffer_AcquireFirstFree(void) +{ + struct SolidSyslogBuffer* handle = &Fallback; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE; poolIndex++) + { + handle = CircularBuffer_AcquireIfFree(poolIndex); + if (CircularBuffer_HandleIsValid(handle)) + { + break; + } + } + return handle; +} + +static struct SolidSyslogBuffer* CircularBuffer_AcquireIfFree(size_t poolIndex) +{ + struct SolidSyslogBuffer* handle = &Fallback; + SolidSyslog_LockConfig(); + if (CircularBuffer_PoolItemIsFree(poolIndex)) + { + handle = CircularBuffer_Acquire(poolIndex); + } + SolidSyslog_UnlockConfig(); + return handle; +} + +static inline bool CircularBuffer_PoolItemIsFree(size_t poolIndex) +{ + return !CircularBuffer_PoolItemIsInUse(poolIndex); +} + +static inline bool CircularBuffer_PoolItemIsInUse(size_t poolIndex) +{ + return Pool[poolIndex].InUse; +} + +static inline struct SolidSyslogBuffer* CircularBuffer_Acquire(size_t poolIndex) +{ + CircularBuffer_MarkInUse(poolIndex); + return CircularBuffer_HandleFromIndex(poolIndex); +} + +static inline void CircularBuffer_MarkInUse(size_t poolIndex) +{ + Pool[poolIndex].InUse = true; +} + +static inline struct SolidSyslogBuffer* CircularBuffer_HandleFromIndex(size_t poolIndex) +{ + return &Pool[poolIndex].Object.Base; +} + +static inline bool CircularBuffer_HandleIsValid(const struct SolidSyslogBuffer* handle) +{ + return handle != &Fallback; +} + +void SolidSyslogCircularBuffer_Destroy(struct SolidSyslogBuffer* base) +{ + size_t poolIndex = CircularBuffer_IndexFromHandle(base); + bool released = false; + if (CircularBuffer_PoolIndexIsValid(poolIndex)) + { + released = CircularBuffer_FreeIfInUse(poolIndex); + } + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY); + } +} + +static size_t CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base) +{ + size_t result = SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE; poolIndex++) + { + if (base == CircularBuffer_HandleFromIndex(poolIndex)) + { + result = poolIndex; + break; + } + } + return result; +} + +static inline bool CircularBuffer_PoolIndexIsValid(size_t poolIndex) +{ + return poolIndex < SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE; +} + +static bool CircularBuffer_FreeIfInUse(size_t poolIndex) +{ + bool released = false; + SolidSyslog_LockConfig(); + if (CircularBuffer_PoolItemIsInUse(poolIndex)) + { + CircularBuffer_Cleanup(CircularBuffer_HandleFromIndex(poolIndex)); + CircularBuffer_MarkFree(poolIndex); + released = true; + } + SolidSyslog_UnlockConfig(); + return released; +} + +static inline void CircularBuffer_MarkFree(size_t poolIndex) +{ + Pool[poolIndex].InUse = false; +} + +static bool Fallback_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) +{ + (void) base; + (void) data; + (void) maxSize; + *bytesRead = 0; + return false; +} + +static void Fallback_Write(struct SolidSyslogBuffer* base, const void* data, size_t size) +{ + (void) base; + (void) data; + (void) size; +} diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 89e61227..e2529b20 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -17,5 +17,9 @@ #define SOLIDSYSLOG_ERROR_MSG_UDPSENDER_CREATE_NULL_DATAGRAM "SolidSyslogUdpSender_Create config.Datagram is NULL" #define SOLIDSYSLOG_ERROR_MSG_UDPSENDER_CREATE_NULL_ENDPOINT "SolidSyslogUdpSender_Create config.Endpoint is NULL" #define SOLIDSYSLOG_ERROR_MSG_UDPSENDER_SEND_NULL_BUFFER "SolidSyslogUdpSender_Send called with NULL buffer" +#define SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_POOL_EXHAUSTED \ + "SolidSyslogCircularBuffer_Create pool exhausted; returning fallback buffer" +#define SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY \ + "SolidSyslogCircularBuffer_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/DEVLOG.md b/DEVLOG.md index b512448b..339e649d 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,155 @@ # Dev Log +## 2026-05-17 — S11.01 commits 2–N: CircularBuffer pool migration (E11 pilot, #392) + +The body of S11.01 — every commit after the `SolidSyslog_SetConfigLock` +seam landed in #393. Split across 28 commits on +`feat/s11-01-circular-buffer-pool` so each step can be reviewed +independently; the PR ships them squashed. + +### Locked-in shape (this is the reference for every other E11 class) + +- **Three-TU split per class.** `SolidSyslogCircularBuffer.c` holds the + vtable + ring logic + private `_Initialise` / `_Cleanup`. + `SolidSyslogCircularBufferPrivate.h` (TU-internal) declares the + struct + private signatures. `SolidSyslogCircularBufferStatic.c` + owns the pool, the public `_Create` / `_Destroy`, the + fallback-handle plumbing, and all slot-walk synchronisation. +- **Tunable** `SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE` (default 1U) + in `SolidSyslogTunablesDefaults.h` with `#ifndef` override and a + floor `#error`. +- **LockConfig pair injection** wraps every slot probe (in both + Acquire and Free walks). `Initialise` runs *outside* the lock so a + future expensive `_Initialise` (FatFs open, mbedTLS setup) doesn't + block interrupts when the integrator maps `SetConfigLock` to + `taskENTER_CRITICAL`. `Cleanup` stays *inside* its per-iteration + lock — releasing the lock around it would let a concurrent Create + grab the slot and race Initialise vs Cleanup. +- **Fallback singleton** of type `struct SolidSyslogBuffer` with a + no-op vtable. Pool exhaustion returns `&Fallback` (the integrator's + Log/Service calls become no-ops, never NULL deref). `_Create` + reports `SOLIDSYSLOG_SEVERITY_ERROR` once on the transition; the + integrator hooks `SolidSyslog_SetErrorHandler` to see it. +- **`_Destroy` is uniform**: any handle not currently issued by + Create — unknown stranger, pool-issued but already destroyed, + Fallback — reports `SOLIDSYSLOG_SEVERITY_WARNING`. The earlier + attempt to special-case Fallback as silent collapsed; the simpler + rule survives. +- **Symmetric `Initialise` / `Cleanup` API.** Both take + `struct SolidSyslogBuffer* base` and recover the concrete type via + `SelfFromBase` internally. The 11.3 cast lives in exactly one TU + (already deviated). `_Static.c` never casts. +- **Scan-then-release in `_Destroy`.** The address-match walk is + lock-free (pool addresses are file-scope statics, never change), + and the lock only wraps the InUse-check + Cleanup + MarkFree. + Lock-count drops to **0** on unknown handles, **1** on matched + handles, regardless of pool size. + +### Reference shape of `*Static.c` + +`_Create` and `_Destroy` each read as four lines of pure intent +against named helpers. The helper tree splits cleanly: + +- **Composition**: `AcquireFirstFree`, `AcquireIfFree`, + `IndexFromHandle`, `FreeIfInUse`, `Acquire` +- **Predicates** (subject-IS-adjective form): + `PoolItemIsFree` (chains through `PoolItemIsInUse`), + `PoolItemIsInUse`, `HandleIsValid`, `PoolIndexIsValid` +- **Atomic field ops**: `MarkInUse`, `MarkFree`, + `HandleFromIndex`, `PoolItemIsInUse` + +The `.InUse` flag is read in exactly one place (`PoolItemIsInUse`), +set-true in one place (`MarkInUse`), set-false in one place +(`MarkFree`). `&Fallback` is referenced in two places (the static +initialiser and `HandleIsValid`). `&Pool[poolIndex].Object.Base` is +computed in exactly one place (`HandleFromIndex`). + +### Tests + +The Pool TEST_GROUP pins: +- pool-exhaustion returns a distinct Fallback handle + (`FillingPoolThenOverflowReturnsDistinctFallback`) +- pool-exhaustion reports ERROR exactly once +- the Fallback vtable methods are no-ops +- `_Create` locks the right number of times under empty / full pool +- `_Destroy` locks exactly once on a pool-issued handle, zero times + on an unknown handle +- unknown and stale handles report WARNING with the right message + +100% line / 100% function / 100% branch coverage on +`SolidSyslogCircularBufferStatic.c` (68/68, 16/16, 18/18). + +The basic CircularBuffer TEST_GROUPs were also restructured: a shared +`CircularBufferFixture` TEST_BASE holds `buffer`, `readData`, +`readSize`, and the `Write(...)` / `Read()` helpers; a +`CHECK_LAST_READ_RECORD` macro replaces the +`LONGS_EQUAL + MEMCMP_EQUAL` pair at 8+ sites. Test bodies now read +top-to-bottom as setup → act → check with no `SolidSyslogBuffer_Write +(buffer, ..., sizeof(...), &readSize)` boilerplate. + +### Notable course-correction mid-flight + +Mid-pass I had extracted three static-inline helpers +(`TryClaim` / `Release` / `SlotOwnsBase`) into `*Static.c`. David +flagged that the file got harder to read, not easier, and asked for +the extractions to be reverted to a flat-inline shape. We then +re-extracted from a clean baseline, one named helper at a time, +each commit prompted and reviewed. The resulting file is shorter +and reads top-down at one level of abstraction per function. The +pattern memory `project_e11_three_tu_split.md` already records the +three-TU split; the helper-extraction lesson is preserved in +`feedback_no_premature_generalisation.md`. + +### Gate posture + +- cppcheck-misra: **87** (baseline 88 on main, improved by 1 via + `_Destroy` becoming single-exit and then by the scan-then-release + split removing the locked walk's compound condition). +- No new `cppcheck-suppress` / `NOLINT` additions on the branch. + The line-pinned 11.3 suppression for `CircularBuffer.c`'s + `SelfFromBase` moved twice (cast shifted 1 line each time + Initialise / Cleanup signature changed); both shifts updated the + existing entry in `misra_suppressions.txt`, never added new ones. +- All other gates green: debug, clang-debug, sanitize, tidy, + cppcheck, coverage, clang-format, integration-linux-openssl + (host tests only — CI covers Windows / BDD / FreeRTOS). + +### Decisions locked in for E11 + +- `CircularBuffer_AcquireIfFree` shape (single `poolIndex` argument, + returns handle-or-Fallback) is the reference for per-slot probes + in every E11 class. +- `IndexFromHandle` + `FreeIfInUse` shape in `_Destroy` is the + reference for any class whose handles can be released by the + integrator. +- "Initialise outside lock, Cleanup inside lock" asymmetry is + intentional and applies to every E11 class. +- Pool size 1 by default; integrators bump via + `SOLIDSYSLOG_USER_TUNABLES_FILE`. + +### Deferred + +- Branch-coverage gate is *not* turned on at CI level — the local + run had to re-capture with `--rc lcov_branch_coverage=1` to + surface the 18/18 figure. Switching the coverage preset to capture + branches by default is a separate housekeeping story. +- A `tunable-override-debug` preset that bumps pool size > 1 — would + pin the per-iteration locking observably in Create's + `CreateLocksOncePerSlotProbedWhenPoolIsFull` test. Useful but not + blocking; tests assert in terms of `SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE` + so they still hold at any pool size. + +### Open questions + +- Whether `PoolItemIsInUse` should be exposed in `*Private.h` for + any future test that wants to probe state from outside `*Static.c`. + Not needed yet; the file-scope visibility is correct as is. +- Whether the symmetric `HandleIsInvalid` / `PoolItemIsInUse` / + `MarkFree` helpers should be lifted into a shared header for + reuse across E11 classes, or whether each class re-implements them + privately. Defer until the second E11 class lands and we see what + actually duplicates. + ## 2026-05-17 — S11.01 commit 1: `SolidSyslog_SetConfigLock` injection point (#392) First commit of S11.01 (the E11 pilot). Lands the global lock/unlock diff --git a/README.md b/README.md index 1ab7da22..be2e0949 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Public headers are split by audience (Interface Segregation Principle): - **`SolidSyslogConfigLock.h`** — optional setup-time lock injection (`SolidSyslog_SetConfigLock(lockFn, unlockFn)`); wraps library-internal pool slot walks so multi-task setup is safe. Defaults are no-ops, so single-task systems can ignore it. Integrators on RTOS / multi-core wire `taskENTER_CRITICAL`, a static `pthread_mutex_t`, `EnterCriticalSection`, or a spinlock pair - **`SolidSyslogSenderDefinition.h`** / **`SolidSyslogBufferDefinition.h`** — extension points for custom senders and buffers - **`SolidSyslogNullBuffer.h`** — direct-send buffer for single-task systems -- **`SolidSyslogCircularBuffer.h`** — portable ring buffer with caller-allocated storage and an injected `SolidSyslogMutex` (`SolidSyslogPosixMutex` / `SolidSyslogWindowsMutex` / `SolidSyslogFreeRtosMutex` / `SolidSyslogNullMutex` / your own); the cross-platform threaded buffer +- **`SolidSyslogCircularBuffer.h`** — portable ring buffer with caller-allocated ring memory and an injected `SolidSyslogMutex` (`SolidSyslogPosixMutex` / `SolidSyslogWindowsMutex` / `SolidSyslogFreeRtosMutex` / `SolidSyslogNullMutex` / your own); the cross-platform threaded buffer. Instance bookkeeping lives in a library-internal static pool (E11) - **`SolidSyslogPosixMessageQueueBuffer.h`** — thread-safe POSIX message queue buffer - **`SolidSyslogUdpSender.h`** — UDP transport (RFC 5426) - **`SolidSyslogStreamSender.h`** — octet-framed syslog (RFC 6587) over any Stream. Note: RFC 6587 diff --git a/SKILL.md b/SKILL.md index b4e19a4b..457bbbe3 100644 --- a/SKILL.md +++ b/SKILL.md @@ -70,7 +70,7 @@ Test progression follows ZOMBIES order. - Dependency injection for transport, buffering, clock, hostname, allocator - Buffer abstraction decouples formatting from sending: `SolidSyslog_Log` writes to buffer, `SolidSyslog_Service` reads from buffer and sends. Implementations: NullBuffer (direct send, - single-task), CircularBuffer (portable ring with caller-allocated storage and an injected + single-task), CircularBuffer (portable ring with caller-allocated ring memory and an injected `SolidSyslogMutex` — Posix/Windows/Null mutex shipped, integrators can plug their own RTOS primitive), PosixMessageQueueBuffer (POSIX message queue, used by the Linux Threaded example) - Null object pattern throughout (NullBuffer is the buffer null object) diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 8873a248..c4db0c67 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -223,7 +223,7 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) * reproducer; CircularBuffer is FIFO so all messages are retained * until Service drains them (unlike BufferFake which only keeps * the last one). */ - SolidSyslogCircularBufferStorage bufferStorage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(16)] = {}; + uint8_t bufferRing[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(16)] = {}; SolidSyslogBlockStoreStorage storeStorage = {}; struct SolidSyslogStore* store = nullptr; struct SolidSyslogMutex* mutex = nullptr; @@ -234,7 +234,7 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) { setupBlockDeviceAndPolicy(); mutex = SolidSyslogNullMutex_Create(); - buffer = SolidSyslogCircularBuffer_Create(bufferStorage, sizeof(bufferStorage), mutex); + buffer = SolidSyslogCircularBuffer_Create(mutex, bufferRing, sizeof(bufferRing)); SenderSpy_Init(spy); } diff --git a/Tests/SolidSyslogCircularBufferTest.cpp b/Tests/SolidSyslogCircularBufferTest.cpp index a35967e6..7730aaf9 100644 --- a/Tests/SolidSyslogCircularBufferTest.cpp +++ b/Tests/SolidSyslogCircularBufferTest.cpp @@ -1,37 +1,79 @@ #include +#include #include "CppUTest/TestHarness.h" + +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" #include "MutexFake.h" #include "SolidSyslogBuffer.h" +#include "SolidSyslogBufferDefinition.h" #include "SolidSyslogCircularBuffer.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogNullMutex.h" +#include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +#include "TestUtils.h" + +using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings ONCE/NEVER into scope for CALLED_FAKE + +// Assertion macros at file scope so failures report the test's own +// __FILE__/__LINE__ rather than the helper's. +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macros preserve __FILE__/__LINE__ at the call site + +// Asserts the last record read equals `expected` bytes of length `size`. +// Depends on `readData` and `readSize` from CircularBufferFixture being in scope. +#define CHECK_LAST_READ_RECORD(expected, size) \ + do \ + { \ + LONGS_EQUAL((size), readSize); \ + MEMCMP_EQUAL((expected), readData, (size)); \ + } while (0) + +// Asserts buf is a non-null handle that is not one of the slots in pool. +// Used to pin the pool-exhaustion Fallback contract: every legitimate +// _Create returns either a pool slot or the Fallback singleton, never NULL. +#define CHECK_IS_FALLBACK(buf, pool) \ + do \ + { \ + CHECK_TEXT((buf) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((buf) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) enum { - TEST_MAX_MESSAGES = 1 + TEST_MAX_MESSAGES = 1, + SMALL_RING_BYTES = 32 }; +// Shared fixture for any TEST_GROUP_BASE that drives one CircularBuffer through +// Write/Read. Each derived group supplies its own ring storage and its own +// setup/teardown (the mutex source differs per group). Test bodies reference +// `buffer`, `readData`, `readSize`, `Write(...)`, `Read()` directly because +// they are inherited members. // clang-format off -TEST_GROUP(SolidSyslogCircularBuffer) +TEST_BASE(CircularBufferFixture) { - SolidSyslogCircularBufferStorage storage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(TEST_MAX_MESSAGES)]; struct SolidSyslogBuffer* buffer = nullptr; - char readData[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; - // cppcheck-suppress variableScope -- member of TEST_GROUP; scope managed by CppUTest macro - size_t readSize; + char readData[SOLIDSYSLOG_MAX_MESSAGE_SIZE] = {}; + size_t readSize = 0; - void setup() override + // Write a null-terminated C string. Size is strlen() -- intended for + // literal-string tests. For binary payloads use the two-arg overload below. + void Write(const char* text) const { - // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - buffer = SolidSyslogCircularBuffer_Create(storage, sizeof(storage), SolidSyslogNullMutex_Create()); - readSize = 0; + SolidSyslogBuffer_Write(buffer, text, strlen(text)); } - void teardown() override + void Write(const void* data, size_t size) const { - SolidSyslogCircularBuffer_Destroy(buffer); - SolidSyslogNullMutex_Destroy(); + SolidSyslogBuffer_Write(buffer, data, size); } bool Read() @@ -42,87 +84,105 @@ TEST_GROUP(SolidSyslogCircularBuffer) // clang-format on -TEST(SolidSyslogCircularBuffer, CreateDestroyDoesNotCrash) +// clang-format off +TEST_GROUP_BASE(SolidSyslogCircularBuffer, CircularBufferFixture) { -} + uint8_t ring[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(TEST_MAX_MESSAGES)]; -TEST(SolidSyslogCircularBuffer, HandleEqualsStorageAddress) + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); + } + + void teardown() override + { + SolidSyslogCircularBuffer_Destroy(buffer); + SolidSyslogNullMutex_Destroy(); + } +}; + +// clang-format on + +TEST(SolidSyslogCircularBuffer, CreateDestroyDoesNotCrash) { - POINTERS_EQUAL(&storage, buffer); } TEST(SolidSyslogCircularBuffer, ReadFromEmptyReturnsFalse) { - CHECK_FALSE(SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize)); + CHECK_FALSE(Read()); } TEST(SolidSyslogCircularBuffer, WriteThenReadReturnsTrue) { - SolidSyslogBuffer_Write(buffer, "hello", 5); - CHECK_TRUE(SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize)); + Write("hello"); + + CHECK_TRUE(Read()); } TEST(SolidSyslogCircularBuffer, ReadReturnsWrittenSize) { - SolidSyslogBuffer_Write(buffer, "hello", 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); + Write("hello"); + + Read(); + LONGS_EQUAL(5, readSize); } TEST(SolidSyslogCircularBuffer, ReadReturnsWrittenData) { - SolidSyslogBuffer_Write(buffer, "hello", 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); + Write("hello"); + + Read(); + MEMCMP_EQUAL("hello", readData, 5); } TEST(SolidSyslogCircularBuffer, SecondReadAfterSingleWriteReturnsFalse) { - SolidSyslogBuffer_Write(buffer, "hello", 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - CHECK_FALSE(SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize)); + Write("hello"); + Read(); + + CHECK_FALSE(Read()); } TEST(SolidSyslogCircularBuffer, TwoWritesReadInOrder) { - SolidSyslogBuffer_Write(buffer, "first", 5); - SolidSyslogBuffer_Write(buffer, "second", 6); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - MEMCMP_EQUAL("first", readData, 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - MEMCMP_EQUAL("second", readData, 6); + Write("first"); + Write("second"); + + Read(); + CHECK_LAST_READ_RECORD("first", 5); + Read(); + CHECK_LAST_READ_RECORD("second", 6); } TEST(SolidSyslogCircularBuffer, ThreeWritesReadInOrder) { - SolidSyslogBuffer_Write(buffer, "alpha", 5); - SolidSyslogBuffer_Write(buffer, "bravo", 5); - SolidSyslogBuffer_Write(buffer, "charlie", 7); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - MEMCMP_EQUAL("alpha", readData, 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - MEMCMP_EQUAL("bravo", readData, 5); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - MEMCMP_EQUAL("charlie", readData, 7); + Write("alpha"); + Write("bravo"); + Write("charlie"); + + Read(); + CHECK_LAST_READ_RECORD("alpha", 5); + Read(); + CHECK_LAST_READ_RECORD("bravo", 5); + Read(); + CHECK_LAST_READ_RECORD("charlie", 7); } TEST(SolidSyslogCircularBuffer, WrapsAroundEndOfStorage) { - enum - { - CYCLES = 400, - PAYLOAD_SIZE = 10 - }; - - char payload[PAYLOAD_SIZE]; - memset(payload, 'X', PAYLOAD_SIZE); + constexpr size_t payloadSize = 10; + constexpr int cycles = 400; + char payload[payloadSize]; + memset(payload, 'X', payloadSize); - for (int i = 0; i < CYCLES; i++) + for (int i = 0; i < cycles; i++) { - SolidSyslogBuffer_Write(buffer, payload, PAYLOAD_SIZE); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - LONGS_EQUAL(PAYLOAD_SIZE, readSize); - MEMCMP_EQUAL(payload, readData, PAYLOAD_SIZE); + Write(payload, payloadSize); + Read(); + CHECK_LAST_READ_RECORD(payload, payloadSize); } } @@ -130,57 +190,48 @@ TEST(SolidSyslogCircularBuffer, OverflowingWriteIsDropped) { char filler[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; memset(filler, 'A', sizeof(filler)); - char overflow[100]; - memset(overflow, 'B', sizeof(overflow)); + char extra[100]; + memset(extra, 'B', sizeof(extra)); - SolidSyslogBuffer_Write(buffer, filler, sizeof(filler)); - SolidSyslogBuffer_Write(buffer, overflow, sizeof(overflow)); + Write(filler, sizeof(filler)); + Write(extra, sizeof(extra)); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - LONGS_EQUAL(sizeof(filler), readSize); - MEMCMP_EQUAL(filler, readData, sizeof(filler)); - CHECK_FALSE(SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize)); + Read(); + CHECK_LAST_READ_RECORD(filler, sizeof(filler)); + CHECK_FALSE(Read()); } TEST(SolidSyslogCircularBuffer, WriteAfterDrainAcceptsRecordTooLargeForRemainingTailSpace) { - SolidSyslogBuffer_Write(buffer, "x", 1); - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); + Write("x"); + Read(); char big[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; memset(big, 'B', sizeof(big)); - SolidSyslogBuffer_Write(buffer, big, sizeof(big)); + Write(big, sizeof(big)); - CHECK_TRUE(SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize)); - LONGS_EQUAL(sizeof(big), readSize); - MEMCMP_EQUAL(big, readData, sizeof(big)); + CHECK_TRUE(Read()); + CHECK_LAST_READ_RECORD(big, sizeof(big)); } TEST(SolidSyslogCircularBuffer, WriteExceedingMaxMessageSizeIsDropped) { char tooBig[SOLIDSYSLOG_MAX_MESSAGE_SIZE + 1]; memset(tooBig, 'X', sizeof(tooBig)); - SolidSyslogBuffer_Write(buffer, tooBig, sizeof(tooBig)); + Write(tooBig, sizeof(tooBig)); - char bigDest[SOLIDSYSLOG_MAX_MESSAGE_SIZE + 1]; - size_t got = 0; - CHECK_FALSE(SolidSyslogBuffer_Read(buffer, bigDest, sizeof(bigDest), &got)); + CHECK_FALSE(Read()); } // clang-format off -TEST_GROUP(SolidSyslogCircularBufferMutex) +TEST_GROUP_BASE(SolidSyslogCircularBufferMutex, CircularBufferFixture) { - SolidSyslogCircularBufferStorage storage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE(TEST_MAX_MESSAGES)]; - struct SolidSyslogBuffer* buffer = nullptr; - char readData[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; - // cppcheck-suppress variableScope -- member of TEST_GROUP; scope managed by CppUTest macro - size_t readSize; + uint8_t ring[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(TEST_MAX_MESSAGES)]; void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - buffer = SolidSyslogCircularBuffer_Create(storage, sizeof(storage), MutexFake_Create()); - readSize = 0; + buffer = SolidSyslogCircularBuffer_Create(MutexFake_Create(), ring, sizeof(ring)); } void teardown() override @@ -188,46 +239,33 @@ TEST_GROUP(SolidSyslogCircularBufferMutex) SolidSyslogCircularBuffer_Destroy(buffer); MutexFake_Destroy(); } - - bool Read() - { - return SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - } }; // clang-format on TEST(SolidSyslogCircularBufferMutex, WriteCallsLockThenUnlockOnce) { - SolidSyslogBuffer_Write(buffer, "hello", 5); + Write("hello"); + STRCMP_EQUAL("LU", MutexFake_Sequence()); } TEST(SolidSyslogCircularBufferMutex, ReadCallsLockThenUnlockOnce) { - SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); + Read(); + STRCMP_EQUAL("LU", MutexFake_Sequence()); } -enum -{ - SMALL_RING_BYTES = 32 -}; - // clang-format off -TEST_GROUP(SolidSyslogCircularBufferSmallRing) +TEST_GROUP_BASE(SolidSyslogCircularBufferSmallRing, CircularBufferFixture) { - SolidSyslogCircularBufferStorage storage[SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE_BYTES(SMALL_RING_BYTES)]; - struct SolidSyslogBuffer* buffer = nullptr; - char readData[SMALL_RING_BYTES]; - // cppcheck-suppress variableScope -- member of TEST_GROUP; scope managed by CppUTest macro - size_t readSize; + uint8_t ring[SMALL_RING_BYTES]; void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - buffer = SolidSyslogCircularBuffer_Create(storage, sizeof(storage), SolidSyslogNullMutex_Create()); - readSize = 0; + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); } void teardown() override @@ -235,11 +273,6 @@ TEST_GROUP(SolidSyslogCircularBufferSmallRing) SolidSyslogCircularBuffer_Destroy(buffer); SolidSyslogNullMutex_Destroy(); } - - bool Read() - { - return SolidSyslogBuffer_Read(buffer, readData, sizeof(readData), &readSize); - } }; // clang-format on @@ -253,10 +286,10 @@ TEST(SolidSyslogCircularBufferSmallRing, WrapWriteFillingExactlyToHeadDoesNotCol { char rec[12]; memset(rec, 'A', sizeof(rec)); - SolidSyslogBuffer_Write(buffer, rec, sizeof(rec)); - SolidSyslogBuffer_Write(buffer, rec, sizeof(rec)); + Write(rec, sizeof(rec)); + Write(rec, sizeof(rec)); Read(); - SolidSyslogBuffer_Write(buffer, rec, sizeof(rec)); + Write(rec, sizeof(rec)); CHECK_TRUE(Read()); LONGS_EQUAL(sizeof(rec), readSize); @@ -278,22 +311,21 @@ TEST(SolidSyslogCircularBufferSmallRing, WriteInWrappedStateFillingExactlyToHead memset(d, 'd', sizeof(d)); memset(e, 'e', sizeof(e)); - SolidSyslogBuffer_Write(buffer, a, sizeof(a)); - SolidSyslogBuffer_Write(buffer, b, sizeof(b)); + Write(a, sizeof(a)); + Write(b, sizeof(b)); Read(); - SolidSyslogBuffer_Write(buffer, c, sizeof(c)); + Write(c, sizeof(c)); Read(); - SolidSyslogBuffer_Write(buffer, d, sizeof(d)); - SolidSyslogBuffer_Write(buffer, e, sizeof(e)); + Write(d, sizeof(d)); + Write(e, sizeof(e)); CHECK_TRUE(Read()); - LONGS_EQUAL(sizeof(c), readSize); - MEMCMP_EQUAL(c, readData, sizeof(c)); + CHECK_LAST_READ_RECORD(c, sizeof(c)); } TEST(SolidSyslogCircularBufferSmallRing, ReadIntoSmallerBufferReturnsFalseAndLeavesRecordQueued) { - SolidSyslogBuffer_Write(buffer, "hello", 5); + Write("hello"); char dest[5]; size_t got = 0; @@ -301,8 +333,7 @@ TEST(SolidSyslogCircularBufferSmallRing, ReadIntoSmallerBufferReturnsFalseAndLea LONGS_EQUAL(0, got); CHECK_TRUE(Read()); - LONGS_EQUAL(5, readSize); - MEMCMP_EQUAL("hello", readData, 5); + CHECK_LAST_READ_RECORD("hello", 5); } // Exercises the ConsumeWrapMarker branch of Read: write A, B, drain A, write @@ -319,17 +350,168 @@ TEST(SolidSyslogCircularBufferSmallRing, WrappedBufferReadsAllRecordsInOrder) memset(c, 'c', sizeof(c)); memset(d, 'd', sizeof(d)); - SolidSyslogBuffer_Write(buffer, a, sizeof(a)); - SolidSyslogBuffer_Write(buffer, b, sizeof(b)); + Write(a, sizeof(a)); + Write(b, sizeof(b)); Read(); - SolidSyslogBuffer_Write(buffer, c, sizeof(c)); - SolidSyslogBuffer_Write(buffer, d, sizeof(d)); + Write(c, sizeof(c)); + Write(d, sizeof(d)); CHECK_TRUE(Read()); - MEMCMP_EQUAL(b, readData, sizeof(b)); + CHECK_LAST_READ_RECORD(b, sizeof(b)); CHECK_TRUE(Read()); - MEMCMP_EQUAL(c, readData, sizeof(c)); + CHECK_LAST_READ_RECORD(c, sizeof(c)); CHECK_TRUE(Read()); - MEMCMP_EQUAL(d, readData, sizeof(d)); + CHECK_LAST_READ_RECORD(d, sizeof(d)); CHECK_FALSE(Read()); } + +// clang-format off +TEST_GROUP(SolidSyslogCircularBufferPool) +{ + uint8_t ring[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(1)]; + struct SolidSyslogMutex* mutex = nullptr; + struct SolidSyslogBuffer* pooled[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE] = {}; + struct SolidSyslogBuffer* overflow = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + mutex = SolidSyslogNullMutex_Create(); + } + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogCircularBuffer_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogCircularBuffer_Destroy(overflow); + } + SolidSyslogNullMutex_Destroy(); + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + struct SolidSyslogBuffer* MakeBuffer() + { + return SolidSyslogCircularBuffer_Create(mutex, ring, sizeof(ring)); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = MakeBuffer(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogCircularBufferPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = MakeBuffer(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogCircularBufferPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = MakeBuffer(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); +} + +TEST(SolidSyslogCircularBufferPool, FallbackWriteAndReadAreNoOps) +{ + FillPool(); + overflow = MakeBuffer(); + + SolidSyslogBuffer_Write(overflow, "hello", 5); + + char readBuffer[16] = {}; + size_t bytesRead = 99; + CHECK_FALSE(SolidSyslogBuffer_Read(overflow, readBuffer, sizeof(readBuffer), &bytesRead)); + LONGS_EQUAL(0, bytesRead); +} + +TEST(SolidSyslogCircularBufferPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = MakeBuffer(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogCircularBufferPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = MakeBuffer(); + + LONGS_EQUAL(SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogCircularBufferPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = MakeBuffer(); + ConfigLockFake_Install(); + + SolidSyslogCircularBuffer_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogCircularBufferPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogBuffer stranger = {}; + + SolidSyslogCircularBuffer_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogCircularBufferPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogBuffer stranger = {}; + + SolidSyslogCircularBuffer_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogCircularBufferPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = MakeBuffer(); + SolidSyslogCircularBuffer_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogCircularBuffer_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} diff --git a/Tests/SolidSyslogTest.cpp b/Tests/SolidSyslogTest.cpp index 7fa9526e..db495a50 100644 --- a/Tests/SolidSyslogTest.cpp +++ b/Tests/SolidSyslogTest.cpp @@ -1541,12 +1541,11 @@ TEST_GROUP(SolidSyslogServiceEagerDrain) void setup() override { - static SolidSyslogCircularBufferStorage bufferStorage[ - SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE_BYTES(BUFFER_BYTES)]; + static uint8_t bufferRing[BUFFER_BYTES]; fakeSender = SenderFake_Create(); circularBuffer = SolidSyslogCircularBuffer_Create( - bufferStorage, sizeof(bufferStorage), SolidSyslogNullMutex_Create()); + SolidSyslogNullMutex_Create(), bufferRing, sizeof(bufferRing)); fakeStore = StoreFake_Create(); SolidSyslogConfig serviceConfig = {}; diff --git a/docs/NAMING.md b/docs/NAMING.md index 592dfb2e..32fb4c21 100644 --- a/docs/NAMING.md +++ b/docs/NAMING.md @@ -322,31 +322,37 @@ static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, #### The storage cast: `_SelfFromStorage` -`_Create` functions take caller-supplied opaque storage and re-interpret -it as the concrete struct. The same convention applies — one named -`static inline` helper per class: +For classes still on the caller-supplied-storage pattern (BlockStore, +the Posix/Windows/FreeRTOS mutexes and streams, …), `_Create` takes +opaque storage and re-interprets it as the concrete struct. The same +convention applies — one named `static inline` helper per class: ```c -static inline struct SolidSyslogCircularBuffer* -CircularBuffer_SelfFromStorage(SolidSyslogCircularBufferStorage* storage) +static inline struct SolidSyslogBlockStore* +BlockStore_SelfFromStorage(SolidSyslogBlockStoreStorage* storage) { - return (struct SolidSyslogCircularBuffer*) storage; + return (struct SolidSyslogBlockStore*) storage; } ``` `_Create` becomes: ```c -struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( - SolidSyslogCircularBufferStorage* storage, ... +struct SolidSyslogStore* SolidSyslogBlockStore_Create( + SolidSyslogBlockStoreStorage* storage, ... ) { - struct SolidSyslogCircularBuffer* self = CircularBuffer_SelfFromStorage(storage); + struct SolidSyslogBlockStore* self = BlockStore_SelfFromStorage(storage); ... return &self->Base; } ``` +Classes migrated under E11 (currently only `SolidSyslogCircularBuffer`) +no longer use this cast — their instance struct lives in a +library-internal static pool, and `_Create` returns a slot pointer +without any storage cast. + Helpers are named per Tier 2 (`Class_Function`, `static inline`, no `SolidSyslog` prefix). Placement follows the function-ordering rule: forward-declared with the other helpers at the top of the file, diff --git a/docs/iec62443.md b/docs/iec62443.md index 5366d02c..72f58c20 100644 --- a/docs/iec62443.md +++ b/docs/iec62443.md @@ -139,7 +139,7 @@ For resource-constrained targets, SL1 or SL2 can operate with: `SolidSyslogCircularBuffer` is the portable choice for the producer / consumer seam between the application thread (calling `Log()`) and the service thread -(calling `Service()`) — caller-allocated storage, mutex injected via the +(calling `Service()`) — caller-allocated ring memory, mutex injected via the `SolidSyslogMutex` vtable, no POSIX dependency. Wrap your RTOS mutex primitive in the vtable and you're done. `SolidSyslogPosixMessageQueueBuffer` remains available where a POSIX `mqueue` is preferred for kernel-level back-pressure diff --git a/docs/misra-deviations.md b/docs/misra-deviations.md index 086197f0..9fe51d10 100644 --- a/docs/misra-deviations.md +++ b/docs/misra-deviations.md @@ -160,11 +160,29 @@ project constraints: | Pass-by-value structs | Doubles the parameter footprint of every `_Create`; breaks the vtable indirection that decouples Core from Platform. | `Address.h` (Strict tier, opaque `SolidSyslogAddress`) and every -caller-storage class (BlockStore, CircularBuffer, Formatter, the +caller-storage class (BlockStore, Formatter, the FreeRTOS/Posix/Windows mutexes and streams, …) hit the same three rules for the same structural reason — one deviation document covers all of them. +Note that `SolidSyslogCircularBuffer` moved off this pattern under +E11 (S11.01): the instance struct now lives in a library-internal +static pool, and the caller supplies only the ring memory as +plain `uint8_t* ring, size_t ringBytes`. No `void*` storage cast on +the instance; the ring pointer is held untyped inside the impl +struct. + +The 11.3 suppression listed against `Core/Source/SolidSyslogCircularBuffer.c` +post-E11 is therefore narrower than the rest of the entries above: it +covers only the **vtable downcast** inside `CircularBuffer_SelfFromBase` +(`struct SolidSyslogBuffer*` → `struct SolidSyslogCircularBuffer*`), the +standard OO-in-C "interface pointer back to derived implementation" cast +that every vtable method needs. The same downcast survives in every +caller-storage class listed above as a separate concern from the +caller-storage void* cast; this deviation covers both shapes under one +heading because both are MISRA 11.3 firings driven by the same +"interface decoupled from concrete type" design. + ### Risk and mitigation - **Type safety** — Each `_Create` is the only place in the library @@ -325,9 +343,8 @@ Project owner — David Cozens. Recorded under ### Deviation -`struct SolidSyslogFormatter` and `struct SolidSyslogCircularBuffer` -each end with a flexible array member that holds the caller-supplied -backing storage: +`struct SolidSyslogFormatter` ends with a flexible array member that +holds the caller-supplied backing storage: ```c struct SolidSyslogFormatter @@ -339,14 +356,17 @@ struct SolidSyslogFormatter ### Scope -Two classes only: +One class only: - `Core/Source/SolidSyslogFormatter.c` -- `Core/Source/SolidSyslogCircularBuffer.c` + +`SolidSyslogCircularBuffer` used to share this shape but moved off +it under E11 (S11.01) — its instance struct now holds an external +ring pointer rather than a trailing FAM. ### Rationale -These two classes implement the variable-size variant of the +The Formatter implements the variable-size variant of the caller-supplied-storage pattern (D.002). The integrator declares a storage buffer of arbitrary size (with a minimum enforced by `_Static_assert`), and the class lives inside that storage — @@ -792,18 +812,17 @@ Project owner — David Cozens. Recorded under ### Deviation -`Core/Interface/SolidSyslogCircularBuffer.h` declares two function-like -macros — `SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE_BYTES` and -`SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE` — that integrator code uses -to size caller-supplied storage. cppcheck-misra runs only over the -Strict tier (`Core/Source/`) and Pragmatic tier (`Platform/*/Source/`); +`Core/Interface/SolidSyslogCircularBuffer.h` declares one function-like +macro — `SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES` — that integrator code +uses to size caller-supplied ring memory. cppcheck-misra runs only over +the Strict tier (`Core/Source/`) and Pragmatic tier (`Platform/*/Source/`); the actual consumers live under `Tests/` (Consistency-only tier) and `Bdd/Targets/` (Out of scope) and are therefore invisible to the checker. ### Scope -`Core/Interface/SolidSyslogCircularBuffer.h` — two macro definitions. +`Core/Interface/SolidSyslogCircularBuffer.h` — one macro definition. A future per-component sweep may surface similar findings on other public API macros (per the tier model, MISRA enforcement does not cross @@ -817,17 +836,17 @@ The macros *are* used — by integrators in `Tests/` and `Bdd/Targets/`. Verified by `grep` over the tree: ``` -Tests/SolidSyslogCircularBufferTest.cpp — both macros -Tests/SolidSyslogTest.cpp — _BYTES form -Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp — _SIZE form -Bdd/Targets/Windows/BddTargetWindows.c — _SIZE form -Bdd/Targets/FreeRtos/main.c — _SIZE form +Tests/SolidSyslogCircularBufferTest.cpp — RING_BYTES +Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp — RING_BYTES +Bdd/Targets/Windows/BddTargetWindows.c — RING_BYTES +Bdd/Targets/FreeRtos/main.c — RING_BYTES ``` -The S10.05 audit's verdict ("Two declared macros that are never used. -Delete them.") was incorrect — the macros are part of the public API; -deleting them would force every integrator to compute storage size by -hand. +The macro is part of the public API; integrators use it to size +the caller-supplied ring buffer in bytes, derived from a maximum +message count (e.g. `uint8_t ring[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(4)]` +allocates enough bytes for four full-size messages plus their +length headers). The alternatives all regress: diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 21255891..863a18ed 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -24,8 +24,7 @@ misra-c2012-11.2:Platform/Windows/Source/SolidSyslogAddressInternal.h:23 misra-c2012-11.3:Core/Interface/SolidSyslogFormatter.h:30 misra-c2012-11.3:Core/Source/SolidSyslogBlockStore.c:82 misra-c2012-11.3:Core/Source/SolidSyslogBlockStore.c:165 -misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:72 -misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:89 +misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:91 misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:118 misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:146 misra-c2012-11.3:Core/Source/SolidSyslogFormatter.c:93 @@ -86,7 +85,7 @@ misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:350 # See docs/misra-deviations.md#d003 misra-c2012-5.7:Core/Interface/SolidSyslogAddress.h:10 misra-c2012-5.7:Core/Interface/SolidSyslogBlockStore.h:52 -misra-c2012-5.7:Core/Interface/SolidSyslogCircularBuffer.h:19 +misra-c2012-5.7:Core/Interface/SolidSyslogCircularBuffer.h:17 misra-c2012-5.7:Core/Interface/SolidSyslogEndpoint.h:11 misra-c2012-5.7:Core/Interface/SolidSyslogFileBlockDevice.h:14 misra-c2012-5.7:Core/Interface/SolidSyslogFormatter.h:14 @@ -97,10 +96,10 @@ misra-c2012-5.7:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-5.7:Core/Interface/SolidSyslogUdpPayload.h:15 misra-c2012-5.7:Core/Source/BlockSequence.c:6 misra-c2012-5.7:Core/Source/BlockSequence.c:92 +misra-c2012-5.7:Core/Source/SolidSyslogCircularBuffer.c:14 misra-c2012-5.7:Core/Source/RecordStore.c:9 misra-c2012-5.7:Core/Source/RecordStore.h:14 misra-c2012-5.7:Core/Source/SolidSyslog.c:26 -misra-c2012-5.7:Core/Source/SolidSyslogCircularBuffer.c:14 misra-c2012-5.7:Core/Source/SolidSyslogCrc16.c:12 misra-c2012-5.7:Core/Source/SolidSyslogCrc16Policy.c:10 misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:14 @@ -149,7 +148,6 @@ misra-c2012-18.4:Core/Source/RecordStore.c:47 # D.005 — Rule 18.7: flexible array members # See docs/misra-deviations.md#d005 -misra-c2012-18.7:Core/Source/SolidSyslogCircularBuffer.c:26 misra-c2012-18.7:Core/Source/SolidSyslogFormatter.c:12 # D.006 — Rule 1.4: C11 @@ -183,7 +181,7 @@ misra-c2012-21.6:Platform/Windows/Source/SolidSyslogWindowsFile.c:8 # D.010 — Rule 2.4: anonymous enum used as named-constant container # See docs/misra-deviations.md#d010 misra-c2012-2.4:Core/Interface/SolidSyslogBlockStore.h:52 -misra-c2012-2.4:Core/Interface/SolidSyslogCircularBuffer.h:19 +misra-c2012-2.4:Core/Interface/SolidSyslogCircularBuffer.h:17 misra-c2012-2.4:Core/Interface/SolidSyslogSecurityPolicyDefinition.h:13 misra-c2012-2.4:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-2.4:Core/Source/BlockSequence.c:6 @@ -197,6 +195,5 @@ misra-c2012-20.10:Core/Source/SolidSyslogMacros.h:9 # D.012 — Rule 2.5: public API macros consumed outside cppcheck-misra scope # See docs/misra-deviations.md#d012 -misra-c2012-2.5:Core/Interface/SolidSyslogCircularBuffer.h:25 -misra-c2012-2.5:Core/Interface/SolidSyslogCircularBuffer.h:30 +misra-c2012-2.5:Core/Interface/SolidSyslogCircularBuffer.h:22 diff --git a/scripts/iwyu_filter.py b/scripts/iwyu_filter.py index 6786a3e4..60e26c49 100755 --- a/scripts/iwyu_filter.py +++ b/scripts/iwyu_filter.py @@ -2,9 +2,11 @@ """Filter IWYU output to drop categories that don't apply to this project. IWYU's analysis is enabled in CI as a ratchet against include-hygiene drift. -Three classes of 'should remove' finding are filtered out before the gate -evaluates, because each represents IWYU producing the wrong answer for this -codebase rather than a real drift: +Several classes of finding are filtered out before the gate evaluates, +because each represents IWYU producing the wrong answer for this codebase +rather than a real drift: + +REMOVALS that we keep as-is: 1. '- struct ;' — explicit forward declarations. In C, 'struct Foo* member;' inside another struct definition itself @@ -25,10 +27,18 @@ It thinks the include is unused because the symbols it provides are only invoked via macros. Removing it would break every test file. +ADDITIONS that IWYU asks for but we never write by hand: + +4. 'class TEST___Test;' — CppUTest macro-generated classes. + TEST() / TEST_GROUP() / TEST_GROUP_BASE() expand to class declarations + that IWYU sees but can't trace back to the macros that produce them, + so it asks for forward declarations no test file ever writes. Same + shape of false positive as (3) — IWYU not modelling CppUTest macros. + Reads IWYU output (typically the stdout of iwyu_tool.py) on stdin, emits filtered output on stdout. File blocks whose only findings are filtered -removals are suppressed entirely, so the report shows only actionable -findings. +findings are suppressed entirely, so the report shows only actionable +ones. Exit code: 0 — no actionable findings remain after filtering @@ -44,11 +54,19 @@ re.compile(r"^- #include \"CppUTest/TestHarness\.h\"\s*//"), ) +FILTERED_ADDITIONS = ( + re.compile(r"^class TEST_\w+_Test;\s*$"), +) + def _is_filtered_removal(line): return any(pat.match(line) for pat in FILTERED_REMOVALS) +def _is_filtered_addition(line): + return any(pat.match(line) for pat in FILTERED_ADDITIONS) + + def filter_iwyu(stream_in, stream_out): """Filter IWYU output; return count of files with actionable findings.""" lines = stream_in.readlines() @@ -107,7 +125,10 @@ def _filter_block(block): elif section == "tail": tail_lines.append(line) - add_real = [l for l in add_lines if l.strip()] + add_real = [ + l for l in add_lines + if l.strip() and not _is_filtered_addition(l) + ] remove_real = [ l for l in remove_lines if l.strip() and not _is_filtered_removal(l) @@ -117,7 +138,8 @@ def _filter_block(block): return [], False out = [header] - out.extend(add_lines) + if add_real: + out.extend(add_real) out.append("\n") out.append(header.replace(" should add these lines:", " should remove these lines:")) if remove_real: