From e15a13819305d8d3d5f07a10dd808fc444a16ec4 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 05:57:07 +0100 Subject: [PATCH 1/5] refactor: S11.05 migrate StreamSender onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the E11 canonical 3-TU split. Internal struct moves to SolidSyslogStreamSenderPrivate.h; vtable + Initialise/Cleanup stay in SolidSyslogStreamSender.c; pool, public Create/Destroy, IndexFromHandle, CleanupAtIndex live in SolidSyslogStreamSenderStatic.c. Pool exhaustion resolves to the shared SolidSyslogNullSender that shipped with S11.04 — no class-private fallback needed. Public API: drops the SolidSyslogStreamSenderStorage typedef + the SOLIDSYSLOG_STREAM_SENDER_SIZE enum from the header; _Create loses the storage parameter. _Destroy keeps the (struct SolidSyslogSender*) shape established by S11.04. Pool size tunable: SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE, default 1. Error messages: STREAMSENDER_POOL_EXHAUSTED (ERROR severity) and STREAMSENDER_UNKNOWN_DESTROY (WARNING severity). Pool test: new TEST_GROUP(SolidSyslogStreamSenderPool) with FillingPoolThenOverflowReturnsDistinctFallback. Generic per-probe lock and stale-handle behaviour is covered by SolidSyslogPoolAllocatorTest.cpp. NoEndpointConfiguredConnectsToPortZero rewired — the original implicitly relied on singleton semantics (second Create silently overwrote the slot). Pool semantics require explicit Destroy then reassign so the second Create draws a fresh slot. CreateReturnsHandleInsideCallerSuppliedStorage dropped — it verified the storage-cast invariant that no longer exists. Caller updates: Bdd/Targets/{Linux,Windows,FreeRtos}/main.c drop their `static SolidSyslogStreamSenderStorage` lines; the two BddTargetTlsSender_OpenSsl_*Tcp.c variants likewise. MISRA suppressions: deletes the old SelfFromStorage 11.3 entry and the header 5.7 entry (both refer to constructs that no longer exist); updates SelfFromBase 11.3 line to :150 and adds the anonymous-enum 2.4/5.7 entries at the new line. Branch vs main cppcheck-misra diff: zero new findings, one evaporated (8.9 on the deleted DEFAULT_INSTANCE / DESTROYED_INSTANCE). Validated at SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE=3 via SOLIDSYSLOG_USER_TUNABLES_FILE override — full suite green (1155 tests). 100% line coverage on both new TUs. CLAUDE.md audience-table row updated. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../BddTargetTlsSender_OpenSsl_PosixTcp.c | 3 +- .../BddTargetTlsSender_OpenSsl_WinsockTcp.c | 3 +- Bdd/Targets/FreeRtos/main.c | 3 +- Bdd/Targets/Linux/main.c | 3 +- Bdd/Targets/Windows/BddTargetWindows.c | 3 +- CLAUDE.md | 2 +- Core/Interface/SolidSyslogStreamSender.h | 17 +-- Core/Interface/SolidSyslogTunablesDefaults.h | 21 ++++ Core/Source/CMakeLists.txt | 1 + Core/Source/SolidSyslogErrorMessages.h | 4 + Core/Source/SolidSyslogStreamSender.c | 87 +++++--------- Core/Source/SolidSyslogStreamSenderPrivate.h | 21 ++++ Core/Source/SolidSyslogStreamSenderStatic.c | 72 ++++++++++++ Tests/SolidSyslogStreamSenderTest.cpp | 107 ++++++++++++++---- misra_suppressions.txt | 7 +- 15 files changed, 242 insertions(+), 112 deletions(-) create mode 100644 Core/Source/SolidSyslogStreamSenderPrivate.h create mode 100644 Core/Source/SolidSyslogStreamSenderStatic.c diff --git a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c index 7b6db1a0..648b1768 100644 --- a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c +++ b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c @@ -14,7 +14,6 @@ static SolidSyslogPosixTcpStreamStorage underlyingStreamStorage; static struct SolidSyslogStream* underlyingStream; static SolidSyslogTlsStreamStorage tlsStreamStorage; static struct SolidSyslogStream* tlsStream; -static SolidSyslogStreamSenderStorage senderStorage; static struct SolidSyslogSender* sender; struct SolidSyslogSender* BddTargetTlsSender_Create(struct SolidSyslogResolver* resolver, bool mtls) @@ -46,7 +45,7 @@ struct SolidSyslogSender* BddTargetTlsSender_Create(struct SolidSyslogResolver* senderConfig.Endpoint = mtls ? BddTargetMtlsConfig_GetEndpoint : BddTargetTlsConfig_GetEndpoint; senderConfig.EndpointVersion = mtls ? BddTargetMtlsConfig_GetEndpointVersion : BddTargetTlsConfig_GetEndpointVersion; - sender = SolidSyslogStreamSender_Create(&senderStorage, &senderConfig); + sender = SolidSyslogStreamSender_Create(&senderConfig); return sender; } diff --git a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_WinsockTcp.c b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_WinsockTcp.c index 2d58b9ed..29eb45ec 100644 --- a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_WinsockTcp.c +++ b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_WinsockTcp.c @@ -14,7 +14,6 @@ static SolidSyslogWinsockTcpStreamStorage underlyingStreamStorage; static struct SolidSyslogStream* underlyingStream; static SolidSyslogTlsStreamStorage tlsStreamStorage; static struct SolidSyslogStream* tlsStream; -static SolidSyslogStreamSenderStorage senderStorage; static struct SolidSyslogSender* sender; struct SolidSyslogSender* BddTargetTlsSender_Create(struct SolidSyslogResolver* resolver, bool mtls) @@ -46,7 +45,7 @@ struct SolidSyslogSender* BddTargetTlsSender_Create(struct SolidSyslogResolver* senderConfig.Endpoint = mtls ? BddTargetMtlsConfig_GetEndpoint : BddTargetTlsConfig_GetEndpoint; senderConfig.EndpointVersion = mtls ? BddTargetMtlsConfig_GetEndpointVersion : BddTargetTlsConfig_GetEndpointVersion; - sender = SolidSyslogStreamSender_Create(&senderStorage, &senderConfig); + sender = SolidSyslogStreamSender_Create(&senderConfig); return sender; } diff --git a/Bdd/Targets/FreeRtos/main.c b/Bdd/Targets/FreeRtos/main.c index 080f3907..d150b8f8 100644 --- a/Bdd/Targets/FreeRtos/main.c +++ b/Bdd/Targets/FreeRtos/main.c @@ -158,7 +158,6 @@ static NetworkEndPoint_t networkEndPoint; static SolidSyslogFreeRtosStaticResolverStorage resolverStorage; static SolidSyslogFreeRtosDatagramStorage datagramStorage; static SolidSyslogFreeRtosTcpStreamStorage tcpStreamStorage; -static SolidSyslogStreamSenderStorage tcpSenderStorage; /* CircularBuffer + FreeRtosMutex composition for cross-task emission. * 8 max-sized messages is comfortably above the 3-message BDD scenarios @@ -800,7 +799,7 @@ static void InteractiveTask(void* argument) .Endpoint = GetEndpoint, .EndpointVersion = GetEndpointVersion, }; - tcpSender = SolidSyslogStreamSender_Create(&tcpSenderStorage, &tcpConfig); + tcpSender = SolidSyslogStreamSender_Create(&tcpConfig); /* SwitchingSender lets `set transport ` flip the active transport * at runtime. Default to UDP so existing UDP-tagged scenarios stay green; diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 235b5a1d..f2a70d92 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -53,7 +53,6 @@ static struct SolidSyslogFile* storeFile; static struct SolidSyslogBlockDevice* storeBlockDevice; static SolidSyslogPosixTcpStreamStorage plainTcpStreamStorage; static struct SolidSyslogStream* plainTcpStream; -static SolidSyslogStreamSenderStorage plainTcpSenderStorage; static struct SolidSyslogSender* plainTcpSender; static struct SolidSyslogSender* udpSender; static struct SolidSyslogSender* switchingSender; @@ -93,7 +92,7 @@ static struct SolidSyslogSender* CreateSender(const struct BddTargetOptions* opt tcpConfig.Stream = plainTcpStream; tcpConfig.Endpoint = BddTargetTcpConfig_GetEndpoint; tcpConfig.EndpointVersion = BddTargetTcpConfig_GetEndpointVersion; - plainTcpSender = SolidSyslogStreamSender_Create(&plainTcpSenderStorage, &tcpConfig); + plainTcpSender = SolidSyslogStreamSender_Create(&tcpConfig); struct SolidSyslogSender* tlsSender = BddTargetTlsSender_Create(resolver, mtlsModeActive); diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index 463702e8..a5661ce6 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -65,7 +65,6 @@ static const char* const STORE_PATH_PREFIX = "Bdd/output/STORE"; static const char* const THRESHOLD_MARKER_PATH = "Bdd/output/solidsyslog_threshold_marker.log"; static SolidSyslogWinsockTcpStreamStorage tcpStreamStorage; -static SolidSyslogStreamSenderStorage tcpSenderStorage; static uint8_t bufferRing[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(BDD_TARGET_BUFFER_MESSAGES)]; static SolidSyslogWindowsMutexStorage mutexStorage; static SolidSyslogWindowsAtomicCounterStorage counterStorage; @@ -196,7 +195,7 @@ static struct SolidSyslogSender* CreateSender(const struct BddTargetWindowsOptio tcpConfig.Stream = plainTcpStream; tcpConfig.Endpoint = GetEndpoint; tcpConfig.EndpointVersion = GetEndpointVersion; - plainTcpSender = SolidSyslogStreamSender_Create(&tcpSenderStorage, &tcpConfig); + plainTcpSender = SolidSyslogStreamSender_Create(&tcpConfig); struct SolidSyslogSender* tlsSender = BddTargetTlsSender_Create(resolver, mtlsModeActive); diff --git a/CLAUDE.md b/CLAUDE.md index 4ace6498..5cbfb505 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -333,7 +333,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogStreamDefinition.h` | Stream implementors (extension point) | `SolidSyslogStream` vtable struct (`Open`, `Send`, `Read`, `Close`) | | `SolidSyslogFreeRtosTcpStream.h` | System setup code on FreeRTOS targets using FreeRTOS-Plus-TCP for TCP | `SolidSyslogFreeRtosTcpStreamStorage`, `SOLIDSYSLOG_FREERTOSTCPSTREAM_SIZE`, `SolidSyslogFreeRtosTcpStream_Create(storage)`, `_Destroy(stream)` (wraps `FreeRTOS_socket` / `FreeRTOS_connect` / `FreeRTOS_send` / `FreeRTOS_recv` / `FreeRTOS_closesocket`; bounded blocking connect via `SO_SNDTIMEO=200ms`, cleared post-connect so Send/Read are non-blocking) | | `SolidSyslogUdpSender.h` | System setup code using UDP transport | `SolidSyslogUdpSenderConfig` (resolver, datagram, endpoint, endpointVersion), `SolidSyslogUdpSender_Create(config)`, `_Destroy(sender)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool-exhaustion and bad-config fallback is the shared `SolidSyslogNullSender`. | -| `SolidSyslogStreamSender.h` | System setup code using octet-framed transport (RFC 6587) over any Stream — `SolidSyslogPosixTcpStream` (POSIX TCP), `SolidSyslogWinsockTcpStream` (Windows TCP), `SolidSyslogFreeRtosTcpStream` (FreeRTOS-Plus-TCP), `SolidSyslogTlsStream` (TLS; OpenSSL reference integration), or a caller-supplied Stream backend | `SolidSyslogStreamSenderConfig` (resolver, stream, endpoint, endpointVersion), `SolidSyslogStreamSender_Create`, `_Destroy` | +| `SolidSyslogStreamSender.h` | System setup code using octet-framed transport (RFC 6587) over any Stream — `SolidSyslogPosixTcpStream` (POSIX TCP), `SolidSyslogWinsockTcpStream` (Windows TCP), `SolidSyslogFreeRtosTcpStream` (FreeRTOS-Plus-TCP), `SolidSyslogTlsStream` (TLS; OpenSSL reference integration), or a caller-supplied Stream backend | `SolidSyslogStreamSenderConfig` (resolver, stream, endpoint, endpointVersion), `SolidSyslogStreamSender_Create(config)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool exhaustion resolves to the shared `SolidSyslogNullSender`. | | `SolidSyslogSwitchingSender.h` | System setup code composing multiple inner senders | `SolidSyslogSwitchingSenderConfig` (senders, senderCount, selector), `SolidSyslogSwitchingSenderSelector`, `SolidSyslogSwitchingSender_Create(config)`, `_Destroy(sender)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Out-of-range selector and pool exhaustion both resolve to the shared `SolidSyslogNullSender`. | | `SolidSyslogBuffer.h` | Library internals consuming a buffer (Service algorithm) | `SolidSyslogBuffer_Write`, `_Read` | | `SolidSyslogBufferDefinition.h` | Buffer implementors (extension point) | `SolidSyslogBuffer` vtable struct | diff --git a/Core/Interface/SolidSyslogStreamSender.h b/Core/Interface/SolidSyslogStreamSender.h index 47759700..20154b4c 100644 --- a/Core/Interface/SolidSyslogStreamSender.h +++ b/Core/Interface/SolidSyslogStreamSender.h @@ -1,8 +1,6 @@ #ifndef SOLIDSYSLOG_STREAM_SENDER_H #define SOLIDSYSLOG_STREAM_SENDER_H -#include - #include "SolidSyslogEndpoint.h" #include "ExternC.h" @@ -18,20 +16,7 @@ EXTERN_C_BEGIN SolidSyslogEndpointVersionFunction EndpointVersion; /* polled cheaply on every Send for stale check */ }; - enum - { - SOLIDSYSLOG_STREAM_SENDER_SIZE = (sizeof(intptr_t) * 7U) + sizeof(uint32_t) - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_STREAM_SENDER_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogStreamSenderStorage; - - struct SolidSyslogSender* SolidSyslogStreamSender_Create( - SolidSyslogStreamSenderStorage * storage, - const struct SolidSyslogStreamSenderConfig* config - ); + struct SolidSyslogSender* SolidSyslogStreamSender_Create(const struct SolidSyslogStreamSenderConfig* config); void SolidSyslogStreamSender_Destroy(struct SolidSyslogSender * base); EXTERN_C_END diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index ec75d165..1f039b48 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -112,6 +112,27 @@ #error "SOLIDSYSLOG_SWITCHING_SENDER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogStreamSender instances the library's internal + * static pool can simultaneously hold. Each instance carries its + * config (resolver/stream/endpoint pointers) and connection state. + * + * Default 1 — almost all integrators wire a single stream-framed + * sender (TCP, TLS) into either SolidSyslogConfig directly or as one + * branch of a SwitchingSender. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE + * if more than one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE < 1 +#error "SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogMetaSd instances the library's internal * static pool can simultaneously hold. Default 1 — meta SD is typically diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 51643cec..623d2ac8 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -54,6 +54,7 @@ if(SOLIDSYSLOG_POSIX OR SOLIDSYSLOG_WINSOCK OR DEFINED ENV{FREERTOS_KERNEL_PATH} SolidSyslogUdpSender.c SolidSyslogUdpSenderStatic.c SolidSyslogStreamSender.c + SolidSyslogStreamSenderStatic.c ) endif() diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 2236492e..f82d0ab5 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -51,5 +51,9 @@ "SolidSyslogPassthroughBuffer_Create pool exhausted; returning fallback buffer" #define SOLIDSYSLOG_ERROR_MSG_PASSTHROUGHBUFFER_UNKNOWN_DESTROY \ "SolidSyslogPassthroughBuffer_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_STREAMSENDER_POOL_EXHAUSTED \ + "SolidSyslogStreamSender_Create pool exhausted; returning fallback sender" +#define SOLIDSYSLOG_ERROR_MSG_STREAMSENDER_UNKNOWN_DESTROY \ + "SolidSyslogStreamSender_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Core/Source/SolidSyslogStreamSender.c b/Core/Source/SolidSyslogStreamSender.c index 4b7458d7..9f001978 100644 --- a/Core/Source/SolidSyslogStreamSender.c +++ b/Core/Source/SolidSyslogStreamSender.c @@ -1,27 +1,18 @@ -#include "SolidSyslogStreamSender.h" - #include #include #include +#include "SolidSyslogAddress.h" #include "SolidSyslogEndpoint.h" #include "SolidSyslogFormatter.h" -#include "SolidSyslogMacros.h" -#include "SolidSyslogSenderDefinition.h" -#include "SolidSyslogAddress.h" #include "SolidSyslogResolver.h" +#include "SolidSyslogSenderDefinition.h" #include "SolidSyslogStream.h" +#include "SolidSyslogStreamSenderPrivate.h" #include "SolidSyslogTransport.h" struct SolidSyslogAddress; - -struct SolidSyslogStreamSender -{ - struct SolidSyslogSender Base; - struct SolidSyslogStreamSenderConfig Config; - bool Connected; - uint32_t LastEndpointVersion; -}; +struct SolidSyslogFormatter; enum { @@ -35,7 +26,6 @@ enum static bool StreamSender_Send(struct SolidSyslogSender* base, const void* buffer, size_t size); static void StreamSender_Disconnect(struct SolidSyslogSender* base); -static inline struct SolidSyslogStreamSender* StreamSender_SelfFromStorage(SolidSyslogStreamSenderStorage* storage); static inline struct SolidSyslogStreamSender* StreamSender_SelfFromBase(struct SolidSyslogSender* base); static inline bool StreamSender_Reconcile(struct SolidSyslogStreamSender* self); @@ -54,60 +44,32 @@ static bool StreamSender_SendBytes(struct SolidSyslogStreamSender* self, const v static void StreamSender_NilEndpoint(struct SolidSyslogEndpoint* endpoint); static uint32_t StreamSender_NilEndpointVersion(void); -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogStreamSender) <= sizeof(SolidSyslogStreamSenderStorage), - "SOLIDSYSLOG_STREAM_SENDER_SIZE is too small for struct SolidSyslogStreamSender" -); - -static const struct SolidSyslogStreamSender DEFAULT_INSTANCE = { - {StreamSender_Send, StreamSender_Disconnect}, - {NULL, NULL, StreamSender_NilEndpoint, StreamSender_NilEndpointVersion}, - false, - 0, -}; - -static const struct SolidSyslogStreamSender DESTROYED_INSTANCE = { - {NULL, NULL}, - {NULL, NULL, StreamSender_NilEndpoint, StreamSender_NilEndpointVersion}, - false, - 0, -}; - -struct SolidSyslogSender* SolidSyslogStreamSender_Create( - SolidSyslogStreamSenderStorage* storage, - const struct SolidSyslogStreamSenderConfig* config -) +void StreamSender_Initialise(struct SolidSyslogSender* base, const struct SolidSyslogStreamSenderConfig* config) { - struct SolidSyslogStreamSender* self = StreamSender_SelfFromStorage(storage); - *self = DEFAULT_INSTANCE; + struct SolidSyslogStreamSender* self = StreamSender_SelfFromBase(base); + self->Base.Send = StreamSender_Send; + self->Base.Disconnect = StreamSender_Disconnect; self->Config.Resolver = config->Resolver; self->Config.Stream = config->Stream; - if (config->Endpoint != NULL) - { - self->Config.Endpoint = config->Endpoint; - } - if (config->EndpointVersion != NULL) - { - self->Config.EndpointVersion = config->EndpointVersion; - } - return &self->Base; -} - -static inline struct SolidSyslogStreamSender* StreamSender_SelfFromStorage(SolidSyslogStreamSenderStorage* storage) -{ - return (struct SolidSyslogStreamSender*) storage; + self->Config.Endpoint = (config->Endpoint != NULL) ? config->Endpoint : StreamSender_NilEndpoint; + self->Config.EndpointVersion = + (config->EndpointVersion != NULL) ? config->EndpointVersion : StreamSender_NilEndpointVersion; + self->Connected = false; + self->LastEndpointVersion = 0; } -void SolidSyslogStreamSender_Destroy(struct SolidSyslogSender* base) +void StreamSender_Cleanup(struct SolidSyslogSender* base) { struct SolidSyslogStreamSender* self = StreamSender_SelfFromBase(base); StreamSender_Disconnect(base); - *self = DESTROYED_INSTANCE; -} - -static inline struct SolidSyslogStreamSender* StreamSender_SelfFromBase(struct SolidSyslogSender* base) -{ - return (struct SolidSyslogStreamSender*) base; + self->Base.Send = NULL; + self->Base.Disconnect = NULL; + self->Config.Resolver = NULL; + self->Config.Stream = NULL; + self->Config.Endpoint = NULL; + self->Config.EndpointVersion = NULL; + self->Connected = false; + self->LastEndpointVersion = 0; } static bool StreamSender_Send(struct SolidSyslogSender* base, const void* buffer, size_t size) @@ -183,6 +145,11 @@ static void StreamSender_Disconnect(struct SolidSyslogSender* base) } } +static inline struct SolidSyslogStreamSender* StreamSender_SelfFromBase(struct SolidSyslogSender* base) +{ + return (struct SolidSyslogStreamSender*) base; +} + static inline void StreamSender_CloseStream(struct SolidSyslogStreamSender* self) { SolidSyslogStream_Close(self->Config.Stream); diff --git a/Core/Source/SolidSyslogStreamSenderPrivate.h b/Core/Source/SolidSyslogStreamSenderPrivate.h new file mode 100644 index 00000000..d502eb2f --- /dev/null +++ b/Core/Source/SolidSyslogStreamSenderPrivate.h @@ -0,0 +1,21 @@ +#ifndef SOLIDSYSLOGSTREAMSENDERPRIVATE_H +#define SOLIDSYSLOGSTREAMSENDERPRIVATE_H + +#include +#include + +#include "SolidSyslogSenderDefinition.h" +#include "SolidSyslogStreamSender.h" + +struct SolidSyslogStreamSender +{ + struct SolidSyslogSender Base; + struct SolidSyslogStreamSenderConfig Config; + bool Connected; + uint32_t LastEndpointVersion; +}; + +void StreamSender_Initialise(struct SolidSyslogSender* base, const struct SolidSyslogStreamSenderConfig* config); +void StreamSender_Cleanup(struct SolidSyslogSender* base); + +#endif /* SOLIDSYSLOGSTREAMSENDERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogStreamSenderStatic.c b/Core/Source/SolidSyslogStreamSenderStatic.c new file mode 100644 index 00000000..0ca395c5 --- /dev/null +++ b/Core/Source/SolidSyslogStreamSenderStatic.c @@ -0,0 +1,72 @@ +#include "SolidSyslogStreamSender.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullSender.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogStreamSenderPrivate.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogSender; + +static size_t StreamSender_IndexFromHandle(const struct SolidSyslogSender* base); +static void StreamSender_CleanupAtIndex(size_t index, void* context); + +static bool StreamSender_InUse[SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE]; +static struct SolidSyslogStreamSender StreamSender_Pool[SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE]; +static struct SolidSyslogPoolAllocator StreamSender_Allocator = { + StreamSender_InUse, + SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE +}; + +struct SolidSyslogSender* SolidSyslogStreamSender_Create(const struct SolidSyslogStreamSenderConfig* config) +{ + struct SolidSyslogSender* result = SolidSyslogNullSender_Get(); + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&StreamSender_Allocator); + if (SolidSyslogPoolAllocator_IndexIsValid(&StreamSender_Allocator, index)) + { + StreamSender_Initialise(&StreamSender_Pool[index].Base, config); + result = &StreamSender_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_STREAMSENDER_POOL_EXHAUSTED); + } + return result; +} + +void SolidSyslogStreamSender_Destroy(struct SolidSyslogSender* base) +{ + size_t index = StreamSender_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&StreamSender_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&StreamSender_Allocator, index, StreamSender_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_STREAMSENDER_UNKNOWN_DESTROY); + } +} + +static size_t StreamSender_IndexFromHandle(const struct SolidSyslogSender* base) +{ + size_t result = SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE; poolIndex++) + { + if (base == &StreamSender_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void StreamSender_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + StreamSender_Cleanup(&StreamSender_Pool[index].Base); +} diff --git a/Tests/SolidSyslogStreamSenderTest.cpp b/Tests/SolidSyslogStreamSenderTest.cpp index 3251b96d..ab794338 100644 --- a/Tests/SolidSyslogStreamSenderTest.cpp +++ b/Tests/SolidSyslogStreamSenderTest.cpp @@ -10,6 +10,7 @@ #include "SolidSyslogPosixTcpStream.h" #include "SolidSyslogSender.h" #include "SolidSyslogStreamSender.h" +#include "SolidSyslogTunables.h" #include "SocketFake.h" #include "TestUtils.h" #include "CppUTest/TestHarness.h" @@ -89,7 +90,6 @@ TEST_GROUP(SolidSyslogStreamSender) SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; - SolidSyslogStreamSenderStorage senderStorage{}; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros struct SolidSyslogSender* sender = nullptr; @@ -104,7 +104,7 @@ TEST_GROUP(SolidSyslogStreamSender) stream = SolidSyslogPosixTcpStream_Create(&streamStorage); config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle - sender = SolidSyslogStreamSender_Create(&senderStorage, &config); + sender = SolidSyslogStreamSender_Create(&config); } void teardown() override @@ -127,15 +127,6 @@ TEST(SolidSyslogStreamSender, CreateReturnsNonNull) CHECK_TRUE(sender != nullptr); } -TEST(SolidSyslogStreamSender, CreateReturnsHandleInsideCallerSuppliedStorage) -{ - SolidSyslogStreamSenderStorage localStorage{}; - struct SolidSyslogStreamSenderConfig localConfig = {resolver, stream, TestEndpoint, TestEndpointVersion}; - struct SolidSyslogSender* localSender = SolidSyslogStreamSender_Create(&localStorage, &localConfig); - POINTERS_EQUAL(&localStorage, localSender); - SolidSyslogStreamSender_Destroy(localSender); -} - TEST(SolidSyslogStreamSender, CreateDoesNotOpenSocket) { CALLED_FAKE(SocketFake_Socket, NEVER); @@ -162,7 +153,6 @@ TEST_GROUP(SolidSyslogStreamSenderDestroy) SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; - SolidSyslogStreamSenderStorage senderStorage{}; void setup() override { @@ -184,7 +174,7 @@ TEST_GROUP(SolidSyslogStreamSenderDestroy) void CreateAndDestroy() { - struct SolidSyslogSender* localSender = SolidSyslogStreamSender_Create(&senderStorage, &config); + struct SolidSyslogSender* localSender = SolidSyslogStreamSender_Create(&config); SolidSyslogStreamSender_Destroy(localSender); } }; @@ -199,7 +189,7 @@ TEST(SolidSyslogStreamSenderDestroy, DestroyWithoutSendDoesNotClose) TEST(SolidSyslogStreamSenderDestroy, DestroyAfterSendClosesSocket) { - struct SolidSyslogSender* sender = SolidSyslogStreamSender_Create(&senderStorage, &config); + struct SolidSyslogSender* sender = SolidSyslogStreamSender_Create(&config); SolidSyslogSender_Send(sender, "x", 1); SolidSyslogStreamSender_Destroy(sender); CALLED_FAKE(SocketFake_Close, ONCE); @@ -208,7 +198,7 @@ TEST(SolidSyslogStreamSenderDestroy, DestroyAfterSendClosesSocket) TEST(SolidSyslogStreamSenderDestroy, DestroyAfterDisconnectDoesNotDoubleClose) { - struct SolidSyslogSender* sender = SolidSyslogStreamSender_Create(&senderStorage, &config); + struct SolidSyslogSender* sender = SolidSyslogStreamSender_Create(&config); SolidSyslogSender_Send(sender, "x", 1); SolidSyslogSender_Disconnect(sender); SolidSyslogStreamSender_Destroy(sender); @@ -338,7 +328,6 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) int (*getPortFn)(void) = GetPort; // NOLINT(modernize-redundant-void-arg) -- C idiom SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; - SolidSyslogStreamSenderStorage senderStorage{}; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros struct SolidSyslogSender* sender = nullptr; @@ -368,7 +357,7 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) stream = SolidSyslogPosixTcpStream_Create(&streamStorage); struct SolidSyslogStreamSenderConfig config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle - sender = SolidSyslogStreamSender_Create(&senderStorage, &config); + sender = SolidSyslogStreamSender_Create(&config); } void Send() const @@ -446,7 +435,6 @@ TEST_GROUP(SolidSyslogStreamSenderFailure) SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; - SolidSyslogStreamSenderStorage senderStorage{}; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros struct SolidSyslogSender* sender = nullptr; @@ -461,7 +449,7 @@ TEST_GROUP(SolidSyslogStreamSenderFailure) stream = SolidSyslogPosixTcpStream_Create(&streamStorage); config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle - sender = SolidSyslogStreamSender_Create(&senderStorage, &config); + sender = SolidSyslogStreamSender_Create(&config); } void teardown() override @@ -628,10 +616,87 @@ TEST(SolidSyslogStreamSenderFailure, SendRecoversAfterTransientResolveFailure) TEST(SolidSyslogStreamSenderFailure, NoEndpointConfiguredConnectsToPortZero) { + /* Drop the setup-built sender so the pool slot is free for the no-endpoint variant — + * with pool semantics a second live Create on a SIZE=1 pool would otherwise overflow + * to NullSender. Reassigning to `sender` lets teardown release the no-endpoint sender. */ SolidSyslogStreamSender_Destroy(sender); struct SolidSyslogStreamSenderConfig configNoEndpoint = {resolver, stream, nullptr, nullptr}; - struct SolidSyslogSender* senderNoEndpoint = SolidSyslogStreamSender_Create(&senderStorage, &configNoEndpoint); - SolidSyslogSender_Send(senderNoEndpoint, TEST_MESSAGE, TEST_MESSAGE_LEN); + sender = SolidSyslogStreamSender_Create(&configNoEndpoint); + SolidSyslogSender_Send(sender, TEST_MESSAGE, TEST_MESSAGE_LEN); CALLED_FAKE(SocketFake_Connect, ONCE); LONGS_EQUAL(0, SocketFake_LastConnectPort()); } + +// Pool tests — prove SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE caps live instances +// and overflow falls back to the shared SolidSyslogNullSender. Generic +// pool mechanics (lock counts, per-probe locking, stale-handle warning) +// are covered by SolidSyslogPoolAllocatorTest.cpp. + +// clang-format off +TEST_GROUP(SolidSyslogStreamSenderPool) +{ + struct SolidSyslogResolver* resolver = nullptr; + SolidSyslogPosixTcpStreamStorage streamStorage{}; + struct SolidSyslogStream* stream = nullptr; + struct SolidSyslogStreamSenderConfig config; + struct SolidSyslogSender* pooled[SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE] = {}; + struct SolidSyslogSender* overflow = nullptr; + + void setup() override + { + SocketFake_Reset(); + endpointGetHost = GetHost; + endpointVersion = 0; + endpointGetPort = GetPort; + resolver = SolidSyslogGetAddrInfoResolver_Create(); + stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + // cppcheck-suppress unreadVariable -- read by MakeSender; cppcheck does not model CppUTest macros + config = {resolver, stream, TestEndpoint, TestEndpointVersion}; + } + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogStreamSender_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogStreamSender_Destroy(overflow); + } + SolidSyslogPosixTcpStream_Destroy(stream); + SolidSyslogGetAddrInfoResolver_Destroy(); + } + + struct SolidSyslogSender* MakeSender() + { + return SolidSyslogStreamSender_Create(&config); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = MakeSender(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogStreamSenderPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = MakeSender(); + + CHECK_TEXT(overflow != nullptr, "Fallback handle was nullptr"); + for (auto* slot : pooled) + { + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); + CHECK_TEXT(overflow != slot, "Fallback handle collided with a pool slot"); + } +} diff --git a/misra_suppressions.txt b/misra_suppressions.txt index bda6faeb..f7ce5f64 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -31,8 +31,7 @@ misra-c2012-11.3:Core/Source/SolidSyslogFormatter.c:93 misra-c2012-11.3:Core/Source/SolidSyslogMetaSd.c:83 misra-c2012-11.3:Core/Source/SolidSyslogPassthroughBuffer.c:54 misra-c2012-11.3:Core/Source/SolidSyslogOriginSd.c:84 -misra-c2012-11.3:Core/Source/SolidSyslogStreamSender.c:98 -misra-c2012-11.3:Core/Source/SolidSyslogStreamSender.c:110 +misra-c2012-11.3:Core/Source/SolidSyslogStreamSender.c:150 misra-c2012-11.3:Core/Source/SolidSyslogSwitchingSender.c:66 misra-c2012-11.3:Core/Source/SolidSyslogTimeQualitySd.c:63 misra-c2012-11.3:Core/Source/SolidSyslogUdpSender.c:149 @@ -90,7 +89,7 @@ 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 misra-c2012-5.7:Core/Interface/SolidSyslogSecurityPolicyDefinition.h:13 -misra-c2012-5.7:Core/Interface/SolidSyslogStreamSender.h:22 +misra-c2012-5.7:Core/Source/SolidSyslogStreamSender.c:18 misra-c2012-5.7:Core/Interface/SolidSyslogTimeQuality.h:12 misra-c2012-5.7:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-5.7:Core/Interface/SolidSyslogUdpPayload.h:15 @@ -187,7 +186,7 @@ misra-c2012-2.4:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-2.4:Core/Source/BlockSequence.c:6 misra-c2012-2.4:Core/Source/BlockSequence.c:92 misra-c2012-2.4:Core/Source/SolidSyslogCircularBuffer.c:14 -misra-c2012-2.4:Core/Source/SolidSyslogStreamSender.c:27 +misra-c2012-2.4:Core/Source/SolidSyslogStreamSender.c:18 # D.011 — Rule 20.10: `#` stringification in _Static_assert polyfill # See docs/misra-deviations.md#d011 From fb86ca7b9c7929981e42c6238a90b95140a2395c Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 06:31:07 +0100 Subject: [PATCH 2/5] fix: S11.05 install null-object vtable on _Cleanup for crash-safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pool slots previously NULL'd their abstract-base vtable in _Cleanup, so calling Send/Disconnect/Write/Read/Format through a stale handle after Destroy was a NULL-fn-pointer dereference. The original pre-E11 pattern (`*self = DESTROYED_INSTANCE;`) had the same hazard at the vtable site. Replace the NULL-out with a copy of the relevant null-object vtable into the slot's abstract base. Use-after-destroy now routes through the GoF null implementation — Send returns true (drop-on-floor), Disconnect/Write/Format no-op, Read returns false with bytesRead=0. Only the abstract base (the public-facing first member of every derived slot struct) needs the swap. Derived fields (Config, Connected, LastEndpointVersion, Sender, Ring, ...) are private to each TU; no public API can reach them, and the next _Initialise overwrites the whole struct on slot reuse. Per-class shape: - StreamSender/UdpSender: Disconnect first (still using the live Config.Stream/Datagram), then `*base = *SolidSyslogNullSender_Get()`. - SwitchingSender: bare swap — does not own inner-sender connections. - MetaSd/TimeQualitySd/OriginSd: bare swap to NullSd. - PassthroughBuffer/CircularBuffer: no shared GoF NullBuffer exists, so the existing class-private Fallback (lived as `static struct SolidSyslogBuffer Fallback` in *Static.c) is exposed as `_Fallback` via the Private.h header (drops `static`, gains the Class prefix). _Cleanup copies it into the slot's Base, and _Create's pool-exhausted fallback path uses the same symbol. Tests: one UseAfterDestroyIsCrashSafeViaXxxVtable test per class exercises Send/Format/Write/Read after Destroy and asserts the null-object semantics. For groups whose teardown auto-destroys the slot, the test re-creates via the same Create at the end so teardown still releases a live slot. MISRA delta (cppcheck-misra vs main): zero new findings, three evaporated (rule 8.9 on StreamSender's DEFAULT/DESTROYED_INSTANCE deletion and on PassthroughBuffer's / CircularBuffer's Fallback losing its `static` qualifier). Total findings 94 vs main's 98. Coverage 99.6% — unchanged from commit 1 (the new tests exercise the null-object paths that were already covered by existing pool-exhaustion tests; the new Cleanup bodies are shorter). All gates green: debug (1163 tests), clang-debug (1163), sanitize (1163), coverage, tidy, cppcheck, cppcheck-misra, clang-format, IWYU. Co-Authored-By: Claude Opus 4.7 (1M context) --- Core/Source/SolidSyslogCircularBuffer.c | 13 ++++--------- Core/Source/SolidSyslogCircularBufferPrivate.h | 6 ++++++ Core/Source/SolidSyslogCircularBufferStatic.c | 4 ++-- Core/Source/SolidSyslogMetaSd.c | 10 +++++----- Core/Source/SolidSyslogOriginSd.c | 9 +++++---- Core/Source/SolidSyslogPassthroughBuffer.c | 8 ++++---- .../SolidSyslogPassthroughBufferPrivate.h | 7 +++++++ .../SolidSyslogPassthroughBufferStatic.c | 4 ++-- Core/Source/SolidSyslogStreamSender.c | 15 ++++++--------- Core/Source/SolidSyslogSwitchingSender.c | 12 +++++------- Core/Source/SolidSyslogTimeQualitySd.c | 8 +++++--- Core/Source/SolidSyslogUdpSender.c | 15 ++++++--------- Tests/SolidSyslogCircularBufferTest.cpp | 15 +++++++++++++++ Tests/SolidSyslogMetaSdTest.cpp | 11 +++++++++++ Tests/SolidSyslogOriginSdTest.cpp | 11 +++++++++++ Tests/SolidSyslogPassthroughBufferTest.cpp | 18 ++++++++++++++++++ Tests/SolidSyslogStreamSenderTest.cpp | 11 +++++++++++ Tests/SolidSyslogSwitchingSenderTest.cpp | 18 ++++++++++++++++++ Tests/SolidSyslogTimeQualitySdTest.cpp | 11 +++++++++++ Tests/SolidSyslogUdpSenderTest.cpp | 11 +++++++++++ misra_suppressions.txt | 8 ++++---- 21 files changed, 167 insertions(+), 58 deletions(-) diff --git a/Core/Source/SolidSyslogCircularBuffer.c b/Core/Source/SolidSyslogCircularBuffer.c index d1fd1d99..3b0c4416 100644 --- a/Core/Source/SolidSyslogCircularBuffer.c +++ b/Core/Source/SolidSyslogCircularBuffer.c @@ -50,15 +50,10 @@ void CircularBuffer_Initialise( 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; + /* Overwrite the abstract base with the class-private Fallback vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. Derived + * fields are private to this TU; the next _Initialise overwrites them. */ + *base = CircularBuffer_Fallback; } static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) diff --git a/Core/Source/SolidSyslogCircularBufferPrivate.h b/Core/Source/SolidSyslogCircularBufferPrivate.h index e4b59b6f..4a6eed06 100644 --- a/Core/Source/SolidSyslogCircularBufferPrivate.h +++ b/Core/Source/SolidSyslogCircularBufferPrivate.h @@ -27,4 +27,10 @@ void CircularBuffer_Initialise( ); void CircularBuffer_Cleanup(struct SolidSyslogBuffer* base); +/* Class-private GoF null Buffer. Defined in SolidSyslogCircularBufferStatic.c; + * used both as the pool-exhausted fallback handle returned by _Create, and as the + * vtable swapped into a slot's Base by _Cleanup so use-after-destroy is a safe + * no-op. No shared SolidSyslogNullBuffer exists for E11. */ +extern struct SolidSyslogBuffer CircularBuffer_Fallback; + #endif /* SOLIDSYSLOGCIRCULARBUFFERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogCircularBufferStatic.c b/Core/Source/SolidSyslogCircularBufferStatic.c index 3973e2c7..9b356d54 100644 --- a/Core/Source/SolidSyslogCircularBufferStatic.c +++ b/Core/Source/SolidSyslogCircularBufferStatic.c @@ -21,7 +21,7 @@ static void CircularBuffer_CleanupAtIndex(size_t index, void* context); static bool CircularBuffer_InUse[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE]; static struct SolidSyslogCircularBuffer CircularBuffer_Pool[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE]; -static struct SolidSyslogBuffer Fallback = {Fallback_Write, Fallback_Read}; +struct SolidSyslogBuffer CircularBuffer_Fallback = {Fallback_Write, Fallback_Read}; static struct SolidSyslogPoolAllocator CircularBuffer_Allocator = { CircularBuffer_InUse, SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE @@ -34,7 +34,7 @@ struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( ) { size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&CircularBuffer_Allocator); - struct SolidSyslogBuffer* handle = &Fallback; + struct SolidSyslogBuffer* handle = &CircularBuffer_Fallback; if (SolidSyslogPoolAllocator_IndexIsValid(&CircularBuffer_Allocator, index)) { CircularBuffer_Initialise(&CircularBuffer_Pool[index].Base, mutex, ring, ringBytes); diff --git a/Core/Source/SolidSyslogMetaSd.c b/Core/Source/SolidSyslogMetaSd.c index 0030ee98..046e4d6f 100644 --- a/Core/Source/SolidSyslogMetaSd.c +++ b/Core/Source/SolidSyslogMetaSd.c @@ -5,6 +5,7 @@ #include "SolidSyslogAtomicCounter.h" #include "SolidSyslogFormatter.h" #include "SolidSyslogMetaSdPrivate.h" +#include "SolidSyslogNullSd.h" #include "SolidSyslogStructuredDataDefinition.h" struct SolidSyslogFormatter; @@ -27,11 +28,10 @@ void MetaSd_Initialise(struct SolidSyslogStructuredData* base, const struct Soli void MetaSd_Cleanup(struct SolidSyslogStructuredData* base) { - struct SolidSyslogMetaSd* self = MetaSd_SelfFromBase(base); - self->Base.Format = NULL; - self->Counter = NULL; - self->GetSysUpTime = NULL; - self->GetLanguage = NULL; + /* Overwrite the abstract base with the shared NullSd vtable so use-after-destroy + * is a safe no-op rather than a NULL-fn-pointer crash. Derived fields are private + * to this TU; the next _Initialise overwrites them. */ + *base = *SolidSyslogNullSd_Get(); } static void MetaSd_Format(struct SolidSyslogStructuredData* base, struct SolidSyslogFormatter* formatter) diff --git a/Core/Source/SolidSyslogOriginSd.c b/Core/Source/SolidSyslogOriginSd.c index 4c89a661..d44a1396 100644 --- a/Core/Source/SolidSyslogOriginSd.c +++ b/Core/Source/SolidSyslogOriginSd.c @@ -3,6 +3,7 @@ #include #include "SolidSyslogFormatter.h" +#include "SolidSyslogNullSd.h" #include "SolidSyslogOriginSdPrivate.h" #include "SolidSyslogStructuredDataDefinition.h" @@ -43,10 +44,10 @@ void OriginSd_Initialise(struct SolidSyslogStructuredData* base, const struct So void OriginSd_Cleanup(struct SolidSyslogStructuredData* base) { - struct SolidSyslogOriginSd* self = OriginSd_SelfFromBase(base); - self->Base.Format = NULL; - self->GetIpCount = NULL; - self->GetIpAt = NULL; + /* Overwrite the abstract base with the shared NullSd vtable so use-after-destroy + * is a safe no-op. Derived fields (incl. the pre-formatted static-prefix Formatter + * storage) are private to this TU; the next _Initialise rebuilds them. */ + *base = *SolidSyslogNullSd_Get(); } static void OriginSd_Format(struct SolidSyslogStructuredData* base, struct SolidSyslogFormatter* formatter) diff --git a/Core/Source/SolidSyslogPassthroughBuffer.c b/Core/Source/SolidSyslogPassthroughBuffer.c index 2f48cde0..7e739df3 100644 --- a/Core/Source/SolidSyslogPassthroughBuffer.c +++ b/Core/Source/SolidSyslogPassthroughBuffer.c @@ -22,10 +22,10 @@ void PassthroughBuffer_Initialise(struct SolidSyslogBuffer* base, struct SolidSy void PassthroughBuffer_Cleanup(struct SolidSyslogBuffer* base) { - struct SolidSyslogPassthroughBuffer* self = PassthroughBuffer_SelfFromBase(base); - self->Base.Write = NULL; - self->Base.Read = NULL; - self->Sender = NULL; + /* Overwrite the abstract base with the class-private Fallback vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. The + * Sender pointer is private to this TU; the next _Initialise overwrites it. */ + *base = PassthroughBuffer_Fallback; } static bool PassthroughBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) diff --git a/Core/Source/SolidSyslogPassthroughBufferPrivate.h b/Core/Source/SolidSyslogPassthroughBufferPrivate.h index 5cb8fdb7..07b20640 100644 --- a/Core/Source/SolidSyslogPassthroughBufferPrivate.h +++ b/Core/Source/SolidSyslogPassthroughBufferPrivate.h @@ -14,4 +14,11 @@ struct SolidSyslogPassthroughBuffer void PassthroughBuffer_Initialise(struct SolidSyslogBuffer* base, struct SolidSyslogSender* sender); void PassthroughBuffer_Cleanup(struct SolidSyslogBuffer* base); +/* Class-private GoF null Buffer. Defined in SolidSyslogPassthroughBufferStatic.c; + * used both as the pool-exhausted fallback handle returned by _Create, and as the + * vtable swapped into a slot's Base by _Cleanup so use-after-destroy is a safe + * no-op. No shared SolidSyslogNullBuffer exists for E11 — PassthroughBuffer is + * itself the closest thing to a "just forward" Buffer. */ +extern struct SolidSyslogBuffer PassthroughBuffer_Fallback; + #endif /* SOLIDSYSLOGPASSTHROUGHBUFFERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogPassthroughBufferStatic.c b/Core/Source/SolidSyslogPassthroughBufferStatic.c index eb1d3fec..8d32392e 100644 --- a/Core/Source/SolidSyslogPassthroughBufferStatic.c +++ b/Core/Source/SolidSyslogPassthroughBufferStatic.c @@ -20,7 +20,7 @@ static void PassthroughBuffer_CleanupAtIndex(size_t index, void* context); static bool PassthroughBuffer_InUse[SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE]; static struct SolidSyslogPassthroughBuffer PassthroughBuffer_Pool[SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE]; -static struct SolidSyslogBuffer Fallback = {Fallback_Write, Fallback_Read}; +struct SolidSyslogBuffer PassthroughBuffer_Fallback = {Fallback_Write, Fallback_Read}; static struct SolidSyslogPoolAllocator PassthroughBuffer_Allocator = { PassthroughBuffer_InUse, SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE @@ -29,7 +29,7 @@ static struct SolidSyslogPoolAllocator PassthroughBuffer_Allocator = { struct SolidSyslogBuffer* SolidSyslogPassthroughBuffer_Create(struct SolidSyslogSender* sender) { size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PassthroughBuffer_Allocator); - struct SolidSyslogBuffer* handle = &Fallback; + struct SolidSyslogBuffer* handle = &PassthroughBuffer_Fallback; if (SolidSyslogPoolAllocator_IndexIsValid(&PassthroughBuffer_Allocator, index)) { PassthroughBuffer_Initialise(&PassthroughBuffer_Pool[index].Base, sender); diff --git a/Core/Source/SolidSyslogStreamSender.c b/Core/Source/SolidSyslogStreamSender.c index 9f001978..b84483d4 100644 --- a/Core/Source/SolidSyslogStreamSender.c +++ b/Core/Source/SolidSyslogStreamSender.c @@ -5,6 +5,7 @@ #include "SolidSyslogAddress.h" #include "SolidSyslogEndpoint.h" #include "SolidSyslogFormatter.h" +#include "SolidSyslogNullSender.h" #include "SolidSyslogResolver.h" #include "SolidSyslogSenderDefinition.h" #include "SolidSyslogStream.h" @@ -60,16 +61,12 @@ void StreamSender_Initialise(struct SolidSyslogSender* base, const struct SolidS void StreamSender_Cleanup(struct SolidSyslogSender* base) { - struct SolidSyslogStreamSender* self = StreamSender_SelfFromBase(base); + /* Disconnect first so the live Config.Stream is still reachable; then overwrite the + * abstract base with the shared NullSender vtable so use-after-destroy is a safe + * no-op rather than a NULL-fn-pointer crash. Derived fields are private to this TU + * so the next _Initialise overwrites them; no need to wipe here. */ StreamSender_Disconnect(base); - self->Base.Send = NULL; - self->Base.Disconnect = NULL; - self->Config.Resolver = NULL; - self->Config.Stream = NULL; - self->Config.Endpoint = NULL; - self->Config.EndpointVersion = NULL; - self->Connected = false; - self->LastEndpointVersion = 0; + *base = *SolidSyslogNullSender_Get(); } static bool StreamSender_Send(struct SolidSyslogSender* base, const void* buffer, size_t size) diff --git a/Core/Source/SolidSyslogSwitchingSender.c b/Core/Source/SolidSyslogSwitchingSender.c index 5a871675..820be7af 100644 --- a/Core/Source/SolidSyslogSwitchingSender.c +++ b/Core/Source/SolidSyslogSwitchingSender.c @@ -34,13 +34,11 @@ void SwitchingSender_Initialise(struct SolidSyslogSender* base, const struct Sol void SwitchingSender_Cleanup(struct SolidSyslogSender* base) { - struct SolidSyslogSwitchingSender* self = SwitchingSender_SelfFromBase(base); - self->Base.Send = NULL; - self->Base.Disconnect = NULL; - self->Config.Senders = NULL; - self->Config.SenderCount = 0; - self->Config.Selector = NULL; - self->CurrentSender = NULL; + /* Overwrite the abstract base with the shared NullSender vtable so use-after-destroy + * is a safe no-op. SwitchingSender does not own its inner senders' connections, so + * unlike UdpSender/StreamSender there is nothing to disconnect first. Derived fields + * are private to this TU; the next _Initialise overwrites them. */ + *base = *SolidSyslogNullSender_Get(); } static bool SwitchingSender_Send(struct SolidSyslogSender* base, const void* buffer, size_t size) diff --git a/Core/Source/SolidSyslogTimeQualitySd.c b/Core/Source/SolidSyslogTimeQualitySd.c index 820b5728..7d279cbd 100644 --- a/Core/Source/SolidSyslogTimeQualitySd.c +++ b/Core/Source/SolidSyslogTimeQualitySd.c @@ -5,6 +5,7 @@ #include #include "SolidSyslogFormatter.h" +#include "SolidSyslogNullSd.h" #include "SolidSyslogStructuredDataDefinition.h" #include "SolidSyslogTimeQualitySdPrivate.h" @@ -30,9 +31,10 @@ void TimeQualitySd_Initialise(struct SolidSyslogStructuredData* base, SolidSyslo void TimeQualitySd_Cleanup(struct SolidSyslogStructuredData* base) { - struct SolidSyslogTimeQualitySd* self = TimeQualitySd_SelfFromBase(base); - self->Base.Format = NULL; - self->GetTimeQuality = NULL; + /* Overwrite the abstract base with the shared NullSd vtable so use-after-destroy + * is a safe no-op. Derived fields are private to this TU; the next _Initialise + * overwrites them. */ + *base = *SolidSyslogNullSd_Get(); } static void TimeQualitySd_Format(struct SolidSyslogStructuredData* base, struct SolidSyslogFormatter* formatter) diff --git a/Core/Source/SolidSyslogUdpSender.c b/Core/Source/SolidSyslogUdpSender.c index 15a68b41..c24ea00f 100644 --- a/Core/Source/SolidSyslogUdpSender.c +++ b/Core/Source/SolidSyslogUdpSender.c @@ -8,6 +8,7 @@ #include "SolidSyslogError.h" #include "SolidSyslogErrorMessages.h" #include "SolidSyslogFormatter.h" +#include "SolidSyslogNullSender.h" #include "SolidSyslogPrival.h" #include "SolidSyslogResolver.h" #include "SolidSyslogSenderDefinition.h" @@ -59,16 +60,12 @@ void UdpSender_Initialise(struct SolidSyslogSender* base, const struct SolidSysl void UdpSender_Cleanup(struct SolidSyslogSender* base) { - struct SolidSyslogUdpSender* self = UdpSender_SelfFromBase(base); + /* Disconnect first so the live Config.Datagram is still reachable; then overwrite the + * abstract base with the shared NullSender vtable so use-after-destroy is a safe + * no-op rather than a NULL-fn-pointer crash. Derived fields are private to this TU + * so the next _Initialise overwrites them; no need to wipe here. */ UdpSender_Disconnect(base); - self->Base.Send = NULL; - self->Base.Disconnect = NULL; - self->Config.Resolver = NULL; - self->Config.Datagram = NULL; - self->Config.Endpoint = NULL; - self->Config.EndpointVersion = NULL; - self->Connected = false; - self->LastEndpointVersion = 0; + *base = *SolidSyslogNullSender_Get(); } static bool UdpSender_Send(struct SolidSyslogSender* base, const void* buffer, size_t size) diff --git a/Tests/SolidSyslogCircularBufferTest.cpp b/Tests/SolidSyslogCircularBufferTest.cpp index 7730aaf9..73bd6bff 100644 --- a/Tests/SolidSyslogCircularBufferTest.cpp +++ b/Tests/SolidSyslogCircularBufferTest.cpp @@ -108,6 +108,21 @@ TEST(SolidSyslogCircularBuffer, CreateDestroyDoesNotCrash) { } +TEST(SolidSyslogCircularBuffer, UseAfterDestroyIsCrashSafeViaFallbackVtable) +{ + /* After Destroy the slot's abstract-base vtable is the class-private Fallback's, + * so Write/Read through the stale handle is a safe no-op rather than a NULL-fn- + * pointer crash. Fallback.Write swallows; Fallback.Read returns false with + * bytesRead=0. */ + SolidSyslogCircularBuffer_Destroy(buffer); + + Write("x"); + CHECK_FALSE(Read()); + LONGS_EQUAL(0, readSize); + + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); // for teardown +} + TEST(SolidSyslogCircularBuffer, ReadFromEmptyReturnsFalse) { CHECK_FALSE(Read()); diff --git a/Tests/SolidSyslogMetaSdTest.cpp b/Tests/SolidSyslogMetaSdTest.cpp index 35730ddc..686af3a7 100644 --- a/Tests/SolidSyslogMetaSdTest.cpp +++ b/Tests/SolidSyslogMetaSdTest.cpp @@ -178,6 +178,17 @@ TEST(SolidSyslogMetaSd, DestroyDoesNotCrash) // Covered by teardown — this test documents the intent } +TEST(SolidSyslogMetaSd, UseAfterDestroyIsCrashSafeViaNullSdVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSd's, so + * Format through the stale handle is a safe no-op rather than a NULL-fn-pointer + * crash. NullSd.Format writes nothing into the Formatter. */ + SolidSyslogMetaSd_Destroy(sd); + SolidSyslogStructuredData_Format(sd, formatter); + LONGS_EQUAL(0, SolidSyslogFormatter_Length(formatter)); + sd = SolidSyslogMetaSd_Create(&config); // for teardown +} + TEST(SolidSyslogMetaSd, FormatIncludesSysUpTimeFromCallback) { useSysUpTime(12345); diff --git a/Tests/SolidSyslogOriginSdTest.cpp b/Tests/SolidSyslogOriginSdTest.cpp index 23f845f4..94fd5067 100644 --- a/Tests/SolidSyslogOriginSdTest.cpp +++ b/Tests/SolidSyslogOriginSdTest.cpp @@ -147,6 +147,17 @@ TEST(SolidSyslogOriginSd, CreateReturnsNonNull) CHECK(sd != nullptr); } +TEST(SolidSyslogOriginSd, UseAfterDestroyIsCrashSafeViaNullSdVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSd's, so + * Format through the stale handle is a safe no-op rather than a NULL-fn-pointer + * crash. NullSd.Format writes nothing into the Formatter. */ + SolidSyslogOriginSd_Destroy(sd); + SolidSyslogStructuredData_Format(sd, formatter); + LONGS_EQUAL(0, SolidSyslogFormatter_Length(formatter)); + sd = SolidSyslogOriginSd_Create(&config); // for teardown +} + TEST(SolidSyslogOriginSd, FormatContainsSoftwareName) { format(); diff --git a/Tests/SolidSyslogPassthroughBufferTest.cpp b/Tests/SolidSyslogPassthroughBufferTest.cpp index 2e4844f9..cec09007 100644 --- a/Tests/SolidSyslogPassthroughBufferTest.cpp +++ b/Tests/SolidSyslogPassthroughBufferTest.cpp @@ -82,6 +82,24 @@ TEST(SolidSyslogPassthroughBuffer, ReadReturnsNothingToSend) CHECK_FALSE(sent); } +TEST(SolidSyslogPassthroughBuffer, UseAfterDestroyIsCrashSafeViaFallbackVtable) +{ + /* After Destroy the slot's abstract-base vtable is the class-private Fallback's, + * so Write/Read through the stale handle is a safe no-op rather than a NULL-fn- + * pointer crash. Fallback.Write swallows; Fallback.Read returns false with + * bytesRead=0. */ + SolidSyslogPassthroughBuffer_Destroy(buffer); + + SolidSyslogBuffer_Write(buffer, "x", 1); + char data[16] = {}; + size_t bytesRead = 99; + CHECK_FALSE(SolidSyslogBuffer_Read(buffer, data, sizeof(data), &bytesRead)); + LONGS_EQUAL(0, bytesRead); + CALLED_FAKE_ON(SenderFake_Send, fakeSender, NEVER); + + buffer = SolidSyslogPassthroughBuffer_Create(fakeSender); // for teardown +} + IGNORE_TEST(SolidSyslogPassthroughBuffer, HappyPathOnly) { diff --git a/Tests/SolidSyslogStreamSenderTest.cpp b/Tests/SolidSyslogStreamSenderTest.cpp index ab794338..2d5c26ad 100644 --- a/Tests/SolidSyslogStreamSenderTest.cpp +++ b/Tests/SolidSyslogStreamSenderTest.cpp @@ -205,6 +205,17 @@ TEST(SolidSyslogStreamSenderDestroy, DestroyAfterDisconnectDoesNotDoubleClose) CALLED_FAKE(SocketFake_Close, ONCE); } +TEST(SolidSyslogStreamSenderDestroy, UseAfterDestroyIsCrashSafeViaNullSenderVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSender's, so + * calling Send/Disconnect through the stale handle is a safe no-op rather than a + * NULL-fn-pointer crash. NullSender.Send returns true (drop-on-floor). */ + struct SolidSyslogSender* sender = SolidSyslogStreamSender_Create(&config); + SolidSyslogStreamSender_Destroy(sender); + CHECK_TRUE(SolidSyslogSender_Send(sender, TEST_MESSAGE, TEST_MESSAGE_LEN)); + SolidSyslogSender_Disconnect(sender); +} + TEST(SolidSyslogStreamSender, SendConnectsOnFirstCall) { Send(); diff --git a/Tests/SolidSyslogSwitchingSenderTest.cpp b/Tests/SolidSyslogSwitchingSenderTest.cpp index 16beec5a..cad01918 100644 --- a/Tests/SolidSyslogSwitchingSenderTest.cpp +++ b/Tests/SolidSyslogSwitchingSenderTest.cpp @@ -119,6 +119,24 @@ TEST(SolidSyslogSwitchingSender, DestroyDoesNotDisconnectInnerSenders) CreateSwitchingSender(2); } +TEST(SolidSyslogSwitchingSender, UseAfterDestroyIsCrashSafeViaNullSenderVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSender's, so + * calling Send/Disconnect through the stale handle is a safe no-op rather than a + * NULL-fn-pointer crash. NullSender.Send returns true (drop-on-floor). */ + struct SolidSyslogSender* destroyed = sender; + SolidSyslogSwitchingSender_Destroy(destroyed); + sender = nullptr; + + CHECK_TRUE(SolidSyslogSender_Send(destroyed, "x", 1)); + SolidSyslogSender_Disconnect(destroyed); + CALLED_FAKE_ON(SenderFake_Send, innerA, NEVER); + CALLED_FAKE_ON(SenderFake_Disconnect, innerA, NEVER); + + // Re-create so teardown's Destroy(sender) targets a live handle. + CreateSwitchingSender(2); +} + TEST(SolidSyslogSwitchingSender, SendDelegatesToSenderAtSelectedIndex) { Send("x", 1); diff --git a/Tests/SolidSyslogTimeQualitySdTest.cpp b/Tests/SolidSyslogTimeQualitySdTest.cpp index f5ffec4f..bddb8bcd 100644 --- a/Tests/SolidSyslogTimeQualitySdTest.cpp +++ b/Tests/SolidSyslogTimeQualitySdTest.cpp @@ -128,6 +128,17 @@ TEST(SolidSyslogTimeQualitySd, CallbackIsInvokedOnEachFormat) STRCMP_EQUAL("[timeQuality tzKnown=\"1\" isSynced=\"0\"]", SolidSyslogFormatter_AsFormattedBuffer(formatter)); } +TEST(SolidSyslogTimeQualitySd, UseAfterDestroyIsCrashSafeViaNullSdVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSd's, so + * Format through the stale handle is a safe no-op rather than a NULL-fn-pointer + * crash. NullSd.Format writes nothing into the Formatter. */ + SolidSyslogTimeQualitySd_Destroy(sd); + SolidSyslogStructuredData_Format(sd, formatter); + LONGS_EQUAL(0, SolidSyslogFormatter_Length(formatter)); + sd = SolidSyslogTimeQualitySd_Create(StubGetTimeQuality); // for teardown +} + TEST(SolidSyslogTimeQualitySd, FormatAdvancesFormatterLength) { LONGS_EQUAL(0, SolidSyslogFormatter_Length(formatter)); diff --git a/Tests/SolidSyslogUdpSenderTest.cpp b/Tests/SolidSyslogUdpSenderTest.cpp index b5ad7d4b..438db5d2 100644 --- a/Tests/SolidSyslogUdpSenderTest.cpp +++ b/Tests/SolidSyslogUdpSenderTest.cpp @@ -384,6 +384,17 @@ TEST(SolidSyslogUdpSenderDestroy, DestroyAfterDisconnectDoesNotDoubleClose) CALLED_FAKE(SocketFake_Close, ONCE); } +TEST(SolidSyslogUdpSenderDestroy, UseAfterDestroyIsCrashSafeViaNullSenderVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullSender's, so + * calling Send/Disconnect through the stale handle is a safe no-op rather than a + * NULL-fn-pointer crash. NullSender.Send returns true (drop-on-floor). */ + struct SolidSyslogSender* destroyed = SolidSyslogUdpSender_Create(&config); + SolidSyslogUdpSender_Destroy(destroyed); + CHECK_TRUE(SolidSyslogSender_Send(destroyed, "x", 1)); + SolidSyslogSender_Disconnect(destroyed); +} + TEST(SolidSyslogUdpSenderDestroy, SimpleScenario) { sender = SolidSyslogUdpSender_Create(&config); diff --git a/misra_suppressions.txt b/misra_suppressions.txt index f7ce5f64..70257c7d 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -24,14 +24,14 @@ 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:91 +misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:86 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 misra-c2012-11.3:Core/Source/SolidSyslogMetaSd.c:83 misra-c2012-11.3:Core/Source/SolidSyslogPassthroughBuffer.c:54 misra-c2012-11.3:Core/Source/SolidSyslogOriginSd.c:84 -misra-c2012-11.3:Core/Source/SolidSyslogStreamSender.c:150 +misra-c2012-11.3:Core/Source/SolidSyslogStreamSender.c:147 misra-c2012-11.3:Core/Source/SolidSyslogSwitchingSender.c:66 misra-c2012-11.3:Core/Source/SolidSyslogTimeQualitySd.c:63 misra-c2012-11.3:Core/Source/SolidSyslogUdpSender.c:149 @@ -89,7 +89,7 @@ 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 misra-c2012-5.7:Core/Interface/SolidSyslogSecurityPolicyDefinition.h:13 -misra-c2012-5.7:Core/Source/SolidSyslogStreamSender.c:18 +misra-c2012-5.7:Core/Source/SolidSyslogStreamSender.c:19 misra-c2012-5.7:Core/Interface/SolidSyslogTimeQuality.h:12 misra-c2012-5.7:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-5.7:Core/Interface/SolidSyslogUdpPayload.h:15 @@ -186,7 +186,7 @@ misra-c2012-2.4:Core/Interface/SolidSyslogTransport.h:5 misra-c2012-2.4:Core/Source/BlockSequence.c:6 misra-c2012-2.4:Core/Source/BlockSequence.c:92 misra-c2012-2.4:Core/Source/SolidSyslogCircularBuffer.c:14 -misra-c2012-2.4:Core/Source/SolidSyslogStreamSender.c:18 +misra-c2012-2.4:Core/Source/SolidSyslogStreamSender.c:19 # D.011 — Rule 20.10: `#` stringification in _Static_assert polyfill # See docs/misra-deviations.md#d011 From 0a18e3a51391a245f63fd2bf627447601575655d Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 07:04:50 +0100 Subject: [PATCH 3/5] feat: S11.05 migrate FileBlockDevice onto PoolAllocator; ship NullBlockDevice + NullBuffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things land together because they are the same architectural choice: 1. FileBlockDevice migration to the canonical E11 3-TU split. - SolidSyslogFileBlockDevice.c: vtable + Initialise/Cleanup. - SolidSyslogFileBlockDevicePrivate.h: struct + Initialise/Cleanup decls. - SolidSyslogFileBlockDeviceStatic.c: pool + Create/Destroy + IndexFromHandle + CleanupAtIndex. - Public header: SolidSyslogFileBlockDeviceStorage typedef and SOLIDSYSLOG_FILE_BLOCK_DEVICE_STORAGE_SIZE enum deleted; _Create loses the storage parameter; _Destroy keeps the (struct *BlockDevice*) shape. - Pool size tunable: SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE, default 1. - Error messages: FILEBLOCKDEVICE_POOL_EXHAUSTED (ERROR) and FILEBLOCKDEVICE_UNKNOWN_DESTROY (WARNING). - Pool test: TEST_GROUP(SolidSyslogFileBlockDevicePool) with FillingPoolThenOverflowReturnsDistinctFallback. - Use-after-destroy test exercises the post-Cleanup vtable. 2. Two new shared GoF nulls — SolidSyslogNullBlockDevice and SolidSyslogNullBuffer. Both are public Get-only objects mirroring the existing NullSender / NullSd / NullStore family. - NullBlockDevice: every method returns false / 0 (disk doesn't exist). - NullBuffer: Write swallows; Read returns false with bytesRead=0. 3. PassthroughBuffer / CircularBuffer retrofit. Their class-private `Fallback` static (defined in *Static.c, exposed by *Private.h in commit 1b) is deleted; their _Cleanup now installs NullBuffer's vtable, and their pool-exhausted _Create returns NullBuffer_Get() instead. Same retrofit for FileBlockDevice landing fresh — no class-private Fallback ever exists; it uses NullBlockDevice from day one. Why these three land together: - S11.04 had explicitly deferred the shared NullBuffer because "PassthroughBuffer is itself the closest thing to a 'just forward' Buffer". Commit 1b changed the calculus: every Cleanup now needs a null-object vtable, so a duplicated class-private Fallback per class becomes copy-paste pressure. A shared null absorbs that cleanly. - FileBlockDevice arriving fresh would have introduced the same class-private Fallback pattern — and brought one MORE bare-prefixed set of Fallback_* statics colliding with the existing two on MISRA 5.9 (advisory: internal-linkage uniqueness across TUs). Reaching for a shared NullBlockDevice avoids the new collision AND lets us retrofit the existing two buffer classes in the same touch. MISRA cppcheck-misra delta vs main: 92 findings (was 98). Five evaporated (CircularBufferStatic 5.9 + 8.9, PassthroughBufferStatic 5.9 + 8.9, StreamSender 8.9 from commit 1's DEFAULT/DESTROYED_INSTANCE deletion). Two new (NullBlockDevice 8.9 + NullBuffer 8.9 — the static `instance` variable, matching the existing pattern for NullSender / NullSd / NullStore on main). Caller updates: drop SolidSyslogFileBlockDeviceStorage locals from Tests/SolidSyslog{BlockStore,BlockStoreDrainOrdering,BlockStorePosix, FileBlockDevice}Test.cpp and Bdd/Targets/{Linux,Windows,FreeRtos}/*. The _Create call sites drop the storage argument. Coverage 99.6% (2515 of 2525 lines, no uncovered lines in the four new TUs). Validated at SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE=3 via SOLIDSYSLOG_USER_TUNABLES_FILE override — full suite green. All gates green: debug (1178 tests), clang-debug, sanitize, coverage, tidy, cppcheck, cppcheck-misra, clang-format, IWYU. CLAUDE.md updates: two new audience-table rows for the shared nulls; FileBlockDevice / PassthroughBuffer / CircularBuffer rows updated to reference the new shared fallback target. Co-Authored-By: Claude Opus 4.7 (1M context) --- Bdd/Targets/FreeRtos/main.c | 3 +- Bdd/Targets/Linux/main.c | 3 +- Bdd/Targets/Windows/BddTargetWindows.c | 3 +- CLAUDE.md | 8 +- Core/Interface/SolidSyslogFileBlockDevice.h | 13 -- Core/Interface/SolidSyslogNullBlockDevice.h | 14 ++ Core/Interface/SolidSyslogNullBuffer.h | 14 ++ Core/Interface/SolidSyslogTunablesDefaults.h | 20 +++ Core/Source/CMakeLists.txt | 3 + Core/Source/SolidSyslogCircularBuffer.c | 5 +- .../Source/SolidSyslogCircularBufferPrivate.h | 6 - Core/Source/SolidSyslogCircularBufferStatic.c | 22 +-- Core/Source/SolidSyslogErrorMessages.h | 4 + Core/Source/SolidSyslogFileBlockDevice.c | 76 +++-------- .../SolidSyslogFileBlockDevicePrivate.h | 36 +++++ .../Source/SolidSyslogFileBlockDeviceStatic.c | 73 ++++++++++ Core/Source/SolidSyslogNullBlockDevice.c | 128 ++++++++++++++++++ Core/Source/SolidSyslogNullBuffer.c | 36 +++++ Core/Source/SolidSyslogPassthroughBuffer.c | 5 +- .../SolidSyslogPassthroughBufferPrivate.h | 7 - .../SolidSyslogPassthroughBufferStatic.c | 22 +-- Tests/CMakeLists.txt | 2 + ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 11 +- Tests/SolidSyslogBlockStorePosixTest.cpp | 3 +- Tests/SolidSyslogBlockStoreTest.cpp | 5 +- Tests/SolidSyslogCircularBufferTest.cpp | 9 +- Tests/SolidSyslogFileBlockDeviceTest.cpp | 93 ++++++++++++- Tests/SolidSyslogNullBlockDeviceTest.cpp | 63 +++++++++ Tests/SolidSyslogNullBufferTest.cpp | 41 ++++++ Tests/SolidSyslogPassthroughBufferTest.cpp | 9 +- misra_suppressions.txt | 13 +- 31 files changed, 578 insertions(+), 172 deletions(-) create mode 100644 Core/Interface/SolidSyslogNullBlockDevice.h create mode 100644 Core/Interface/SolidSyslogNullBuffer.h create mode 100644 Core/Source/SolidSyslogFileBlockDevicePrivate.h create mode 100644 Core/Source/SolidSyslogFileBlockDeviceStatic.c create mode 100644 Core/Source/SolidSyslogNullBlockDevice.c create mode 100644 Core/Source/SolidSyslogNullBuffer.c create mode 100644 Tests/SolidSyslogNullBlockDeviceTest.cpp create mode 100644 Tests/SolidSyslogNullBufferTest.cpp diff --git a/Bdd/Targets/FreeRtos/main.c b/Bdd/Targets/FreeRtos/main.c index d150b8f8..c7a74fd4 100644 --- a/Bdd/Targets/FreeRtos/main.c +++ b/Bdd/Targets/FreeRtos/main.c @@ -195,7 +195,6 @@ static volatile bool solidSyslogTeardown = false; static const char STORE_PATH_PREFIX[] = "STORE"; static SolidSyslogFatFsFileStorage storeFileStorage; -static SolidSyslogFileBlockDeviceStorage blockDeviceStorage; static SolidSyslogBlockStoreStorage blockStoreStorage; /* FATFS object lives in .bss because f_mount stores its address inside the @@ -590,7 +589,7 @@ static bool RebuildWithFileStore(void) * BlockSequence_Open's f_stat / f_open calls now hit a live filesystem * via disk_read / disk_write semihosting traps. */ storeFile = SolidSyslogFatFsFile_Create(&storeFileStorage); - storeBlockDevice = SolidSyslogFileBlockDevice_Create(&blockDeviceStorage, storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); struct SolidSyslogSecurityPolicy* policy = SolidSyslogCrc16Policy_Create(); struct SolidSyslogBlockStoreConfig storeConfig = { diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index f2a70d92..97100a08 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -160,8 +160,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetOptions* optio static SolidSyslogPosixFileStorage fileStorage; storeFile = SolidSyslogPosixFile_Create(&fileStorage); - static SolidSyslogFileBlockDeviceStorage blockDeviceStorage; - storeBlockDevice = SolidSyslogFileBlockDevice_Create(&blockDeviceStorage, storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); static size_t capacityThreshold; capacityThreshold = options->CapacityThreshold; diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index a5661ce6..8deb76f0 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -234,8 +234,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetWindowsOptions static SolidSyslogWindowsFileStorage fileStorage; storeFile = SolidSyslogWindowsFile_Create(&fileStorage); - static SolidSyslogFileBlockDeviceStorage blockDeviceStorage; - storeBlockDevice = SolidSyslogFileBlockDevice_Create(&blockDeviceStorage, storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); static size_t capacityThreshold; capacityThreshold = options->CapacityThreshold; diff --git a/CLAUDE.md b/CLAUDE.md index 5cbfb505..919c2ea0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -336,10 +336,11 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogStreamSender.h` | System setup code using octet-framed transport (RFC 6587) over any Stream — `SolidSyslogPosixTcpStream` (POSIX TCP), `SolidSyslogWinsockTcpStream` (Windows TCP), `SolidSyslogFreeRtosTcpStream` (FreeRTOS-Plus-TCP), `SolidSyslogTlsStream` (TLS; OpenSSL reference integration), or a caller-supplied Stream backend | `SolidSyslogStreamSenderConfig` (resolver, stream, endpoint, endpointVersion), `SolidSyslogStreamSender_Create(config)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool exhaustion resolves to the shared `SolidSyslogNullSender`. | | `SolidSyslogSwitchingSender.h` | System setup code composing multiple inner senders | `SolidSyslogSwitchingSenderConfig` (senders, senderCount, selector), `SolidSyslogSwitchingSenderSelector`, `SolidSyslogSwitchingSender_Create(config)`, `_Destroy(sender)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Out-of-range selector and pool exhaustion both resolve to the shared `SolidSyslogNullSender`. | | `SolidSyslogBuffer.h` | Library internals consuming a buffer (Service algorithm) | `SolidSyslogBuffer_Write`, `_Read` | +| `SolidSyslogNullBuffer.h` | Any code installing a no-op buffer slot (Read returns `false`/`bytesRead=0`; Write swallows the record) | `SolidSyslogNullBuffer_Get` | | `SolidSyslogBufferDefinition.h` | Buffer implementors (extension point) | `SolidSyslogBuffer` vtable struct | -| `SolidSyslogPassthroughBuffer.h` | System setup code (single-task, no buffering) | `SolidSyslogPassthroughBuffer_Create(sender)`, `_Destroy(buffer)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create` to release the slot. | +| `SolidSyslogPassthroughBuffer.h` | System setup code (single-task, no buffering) | `SolidSyslogPassthroughBuffer_Create(sender)`, `_Destroy(buffer)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create` to release the slot. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | | `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) | `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. | +| `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. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | | `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` | @@ -352,8 +353,9 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogFileDefinition.h` | File implementors (extension point) | `SolidSyslogFile` vtable struct | | `SolidSyslogFile.h` | Any code using the file abstraction | `SolidSyslogFile_Open`, `_Close`, `_IsOpen`, `_Read`, `_Write`, `_SeekTo`, `_Size`, `_Truncate` | | `SolidSyslogBlockDevice.h` | Library internals consuming a block device (BlockSequence inside BlockStore) and integrator code addressing blocks directly | `SolidSyslogBlockDevice_Acquire`, `_Dispose`, `_Exists`, `_Read`, `_Append`, `_WriteAt`, `_Size` (block-indexed I/O; Acquire makes a block ready for fresh writes, Dispose releases it) | +| `SolidSyslogNullBlockDevice.h` | Any code installing a no-op block device slot (every method returns `false` / `0` — disk doesn't exist) | `SolidSyslogNullBlockDevice_Get` | | `SolidSyslogBlockDeviceDefinition.h` | BlockDevice implementors (extension point) | `SolidSyslogBlockDevice` vtable struct (`Acquire`, `Dispose`, `Exists`, `Read`, `Append`, `WriteAt`, `Size`) | -| `SolidSyslogFileBlockDevice.h` | System setup code wiring a file-backed block device | `SolidSyslogFileBlockDeviceStorage`, `SOLIDSYSLOG_FILEBLOCKDEVICE_STORAGE_SIZE`, `SolidSyslogFileBlockDevice_Create(storage, readFile, writeFile, pathPrefix)` (sequence-numbered filenames `.log`, two cached file handles), `_Destroy(device)` | +| `SolidSyslogFileBlockDevice.h` | System setup code wiring a file-backed block device | `SolidSyslogFileBlockDevice_Create(file, pathPrefix)` (sequence-numbered filenames `.log`, one cached file handle), `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool exhaustion resolves to the shared `SolidSyslogNullBlockDevice`. | | `SolidSyslogBlockStore.h` | System setup code using a BlockDevice-backed store | `SolidSyslogBlockStoreStorage`, `SOLIDSYSLOG_BLOCKSTORE_STORAGE_SIZE`, `SolidSyslogBlockStoreConfig` (with `blockDevice`, `storeFullContext`, `getCapacityThreshold`, `onThresholdCrossed`, `thresholdContext`), `SolidSyslogBlockStore_Create(storage, config)`, `_Destroy(store)`, `SolidSyslogDiscardPolicy` (`SolidSyslogDiscardPolicy_Oldest` / `_Newest` / `_Halt`), `SolidSyslogStoreFullCallback(void* context)`, `SolidSyslogStoreThresholdFunction(void* context)` (returns threshold in bytes; 0 disables; queried each Write), `SolidSyslogStoreThresholdCallback(void* context)` (edge-triggered; PassthroughBuffer recursion gotcha — see header) | | `SolidSyslogSecurityPolicyDefinition.h` | SecurityPolicy implementors (extension point) | `SolidSyslogSecurityPolicy` vtable struct, `SOLIDSYSLOG_MAX_INTEGRITY_SIZE` | | `SolidSyslogNullSecurityPolicy.h` | System setup code (no integrity checking) | `SolidSyslogNullSecurityPolicy_Get` | diff --git a/Core/Interface/SolidSyslogFileBlockDevice.h b/Core/Interface/SolidSyslogFileBlockDevice.h index 4959d4ab..626bf1aa 100644 --- a/Core/Interface/SolidSyslogFileBlockDevice.h +++ b/Core/Interface/SolidSyslogFileBlockDevice.h @@ -1,8 +1,6 @@ #ifndef SOLIDSYSLOGFILEBLOCKDEVICE_H #define SOLIDSYSLOGFILEBLOCKDEVICE_H -#include - #include "ExternC.h" struct SolidSyslogBlockDevice; @@ -10,23 +8,12 @@ struct SolidSyslogFile; EXTERN_C_BEGIN - enum - { - SOLIDSYSLOG_FILE_BLOCK_DEVICE_STORAGE_SIZE = sizeof(intptr_t) * 16U - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_FILE_BLOCK_DEVICE_STORAGE_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogFileBlockDeviceStorage; - /* The driver caches at most one open SolidSyslogFile handle, re-pointed only when the * targeted blockIndex changes — same-block runs of Read and Append (and Append-then-WriteAt * during MarkSent) share the handle. The single-handle-per-path invariant the storage layer * depends on (E27 #345 / S27.01) is enforced by construction here: the driver physically * holds one file. */ struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create( - SolidSyslogFileBlockDeviceStorage * storage, struct SolidSyslogFile * file, const char* pathPrefix ); diff --git a/Core/Interface/SolidSyslogNullBlockDevice.h b/Core/Interface/SolidSyslogNullBlockDevice.h new file mode 100644 index 00000000..0ba222d9 --- /dev/null +++ b/Core/Interface/SolidSyslogNullBlockDevice.h @@ -0,0 +1,14 @@ +#ifndef SOLIDSYSLOGNULLBLOCKDEVICE_H +#define SOLIDSYSLOGNULLBLOCKDEVICE_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogBlockDevice; + + struct SolidSyslogBlockDevice* SolidSyslogNullBlockDevice_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLBLOCKDEVICE_H */ diff --git a/Core/Interface/SolidSyslogNullBuffer.h b/Core/Interface/SolidSyslogNullBuffer.h new file mode 100644 index 00000000..9524c54c --- /dev/null +++ b/Core/Interface/SolidSyslogNullBuffer.h @@ -0,0 +1,14 @@ +#ifndef SOLIDSYSLOGNULLBUFFER_H +#define SOLIDSYSLOGNULLBUFFER_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogBuffer; + + struct SolidSyslogBuffer* SolidSyslogNullBuffer_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLBUFFER_H */ diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 1f039b48..4faf3d88 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -133,6 +133,26 @@ #error "SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogFileBlockDevice instances the library's internal + * static pool can simultaneously hold. Each instance carries the cached + * open-file handle plus the path-prefix pointer. + * + * Default 1 — almost all integrators wire a single FileBlockDevice as + * the backing store for one BlockStore. Bump via + * SOLIDSYSLOG_USER_TUNABLES_FILE if more than one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE < 1 +#error "SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogMetaSd instances the library's internal * static pool can simultaneously hold. Default 1 — meta SD is typically diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 623d2ac8..69269cc5 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -20,6 +20,8 @@ set(SOURCES SolidSyslogStore.c SolidSyslogNullSd.c SolidSyslogNullStore.c + SolidSyslogNullBuffer.c + SolidSyslogNullBlockDevice.c SolidSyslogNullSecurityPolicy.c SolidSyslogCrc16.c SolidSyslogCrc16Policy.c @@ -35,6 +37,7 @@ set(SOURCES BlockSequence.c SolidSyslogBlockDevice.c SolidSyslogFileBlockDevice.c + SolidSyslogFileBlockDeviceStatic.c SolidSyslogUdpPayload.c ) diff --git a/Core/Source/SolidSyslogCircularBuffer.c b/Core/Source/SolidSyslogCircularBuffer.c index 3b0c4416..ef249b57 100644 --- a/Core/Source/SolidSyslogCircularBuffer.c +++ b/Core/Source/SolidSyslogCircularBuffer.c @@ -8,6 +8,7 @@ #include "SolidSyslogBufferDefinition.h" #include "SolidSyslogCircularBufferPrivate.h" #include "SolidSyslogMutex.h" +#include "SolidSyslogNullBuffer.h" #include "SolidSyslogTunables.h" enum @@ -50,10 +51,10 @@ void CircularBuffer_Initialise( void CircularBuffer_Cleanup(struct SolidSyslogBuffer* base) { - /* Overwrite the abstract base with the class-private Fallback vtable so + /* Overwrite the abstract base with the shared NullBuffer vtable so * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. Derived * fields are private to this TU; the next _Initialise overwrites them. */ - *base = CircularBuffer_Fallback; + *base = *SolidSyslogNullBuffer_Get(); } static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) diff --git a/Core/Source/SolidSyslogCircularBufferPrivate.h b/Core/Source/SolidSyslogCircularBufferPrivate.h index 4a6eed06..e4b59b6f 100644 --- a/Core/Source/SolidSyslogCircularBufferPrivate.h +++ b/Core/Source/SolidSyslogCircularBufferPrivate.h @@ -27,10 +27,4 @@ void CircularBuffer_Initialise( ); void CircularBuffer_Cleanup(struct SolidSyslogBuffer* base); -/* Class-private GoF null Buffer. Defined in SolidSyslogCircularBufferStatic.c; - * used both as the pool-exhausted fallback handle returned by _Create, and as the - * vtable swapped into a slot's Base by _Cleanup so use-after-destroy is a safe - * no-op. No shared SolidSyslogNullBuffer exists for E11. */ -extern struct SolidSyslogBuffer CircularBuffer_Fallback; - #endif /* SOLIDSYSLOGCIRCULARBUFFERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogCircularBufferStatic.c b/Core/Source/SolidSyslogCircularBufferStatic.c index 9b356d54..476bb83d 100644 --- a/Core/Source/SolidSyslogCircularBufferStatic.c +++ b/Core/Source/SolidSyslogCircularBufferStatic.c @@ -8,20 +8,18 @@ #include "SolidSyslogCircularBufferPrivate.h" #include "SolidSyslogError.h" #include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullBuffer.h" #include "SolidSyslogPoolAllocator.h" #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" struct SolidSyslogMutex; -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 size_t CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); static void CircularBuffer_CleanupAtIndex(size_t index, void* context); static bool CircularBuffer_InUse[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE]; static struct SolidSyslogCircularBuffer CircularBuffer_Pool[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE]; -struct SolidSyslogBuffer CircularBuffer_Fallback = {Fallback_Write, Fallback_Read}; static struct SolidSyslogPoolAllocator CircularBuffer_Allocator = { CircularBuffer_InUse, SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE @@ -34,7 +32,7 @@ struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create( ) { size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&CircularBuffer_Allocator); - struct SolidSyslogBuffer* handle = &CircularBuffer_Fallback; + struct SolidSyslogBuffer* handle = SolidSyslogNullBuffer_Get(); if (SolidSyslogPoolAllocator_IndexIsValid(&CircularBuffer_Allocator, index)) { CircularBuffer_Initialise(&CircularBuffer_Pool[index].Base, mutex, ring, ringBytes); @@ -78,19 +76,3 @@ static void CircularBuffer_CleanupAtIndex(size_t index, void* context) (void) context; CircularBuffer_Cleanup(&CircularBuffer_Pool[index].Base); } - -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 f82d0ab5..21cb2163 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -55,5 +55,9 @@ "SolidSyslogStreamSender_Create pool exhausted; returning fallback sender" #define SOLIDSYSLOG_ERROR_MSG_STREAMSENDER_UNKNOWN_DESTROY \ "SolidSyslogStreamSender_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_FILEBLOCKDEVICE_POOL_EXHAUSTED \ + "SolidSyslogFileBlockDevice_Create pool exhausted; returning fallback block device" +#define SOLIDSYSLOG_ERROR_MSG_FILEBLOCKDEVICE_UNKNOWN_DESTROY \ + "SolidSyslogFileBlockDevice_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Core/Source/SolidSyslogFileBlockDevice.c b/Core/Source/SolidSyslogFileBlockDevice.c index 4b5f9b71..018e709c 100644 --- a/Core/Source/SolidSyslogFileBlockDevice.c +++ b/Core/Source/SolidSyslogFileBlockDevice.c @@ -4,9 +4,11 @@ #include #include "SolidSyslogBlockDeviceDefinition.h" +#include "SolidSyslogFile.h" +#include "SolidSyslogFileBlockDevicePrivate.h" #include "SolidSyslogFormatter.h" #include "SolidSyslogMacros.h" -#include "SolidSyslogFile.h" +#include "SolidSyslogNullBlockDevice.h" struct SolidSyslogFile; @@ -33,30 +35,7 @@ static inline bool FileBlockDevice_IsValidBlockIndex(size_t blockIndex) return blockIndex <= MAX_BLOCK_INDEX; } -/* OpenHandle caches the single SolidSyslogFile the device holds. The handle is - * re-pointed only when the targeted blockIndex changes; same-block runs reuse - * it. This is the structural enforcement of the S27.01 single-handle-per-path - * invariant — by construction the device has exactly one underlying file. */ -struct OpenHandle -{ - struct SolidSyslogFile* File; - size_t BlockIndex; - bool IsOpen; -}; - -struct SolidSyslogFileBlockDevice -{ - struct SolidSyslogBlockDevice Base; - struct OpenHandle Handle; - const char* PathPrefix; -}; - -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogFileBlockDevice) <= sizeof(SolidSyslogFileBlockDeviceStorage), - "SOLIDSYSLOG_FILE_BLOCK_DEVICE_STORAGE_SIZE is too small for struct SolidSyslogFileBlockDevice" -); - -/* vtable — forward-declared because Create wires them before their definitions */ +/* vtable — forward-declared because _Initialise wires them before their definitions */ static bool FileBlockDevice_Acquire(struct SolidSyslogBlockDevice* base, size_t blockIndex); static bool FileBlockDevice_Dispose(struct SolidSyslogBlockDevice* base, size_t blockIndex); static bool FileBlockDevice_Exists(struct SolidSyslogBlockDevice* base, size_t blockIndex); @@ -86,40 +65,16 @@ static bool FileBlockDevice_WriteAt( // NOLINTEND(bugprone-easily-swappable-parameters) static size_t FileBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t blockIndex); -static inline struct SolidSyslogFileBlockDevice* FileBlockDevice_SelfFromStorage( - SolidSyslogFileBlockDeviceStorage* storage -); static inline struct SolidSyslogFileBlockDevice* FileBlockDevice_SelfFromBase(struct SolidSyslogBlockDevice* base); -static inline void FileBlockDevice_InitialiseVtable(struct SolidSyslogFileBlockDevice* self); - -/* ------------------------------------------------------------------ - * Create - * ----------------------------------------------------------------*/ +static inline void FileBlockDevice_CloseIfOpen(struct OpenHandle* handle); -struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create( - SolidSyslogFileBlockDeviceStorage* storage, +void FileBlockDevice_Initialise( + struct SolidSyslogBlockDevice* base, struct SolidSyslogFile* file, const char* pathPrefix ) { - struct SolidSyslogFileBlockDevice* self = FileBlockDevice_SelfFromStorage(storage); - - FileBlockDevice_InitialiseVtable(self); - self->Handle = (struct OpenHandle) {.File = file, .BlockIndex = 0, .IsOpen = false}; - self->PathPrefix = pathPrefix; - - return &self->Base; -} - -static inline struct SolidSyslogFileBlockDevice* FileBlockDevice_SelfFromStorage( - SolidSyslogFileBlockDeviceStorage* storage -) -{ - return (struct SolidSyslogFileBlockDevice*) storage; -} - -static inline void FileBlockDevice_InitialiseVtable(struct SolidSyslogFileBlockDevice* self) -{ + struct SolidSyslogFileBlockDevice* self = FileBlockDevice_SelfFromBase(base); self->Base.Acquire = FileBlockDevice_Acquire; self->Base.Dispose = FileBlockDevice_Dispose; self->Base.Exists = FileBlockDevice_Exists; @@ -127,18 +82,19 @@ static inline void FileBlockDevice_InitialiseVtable(struct SolidSyslogFileBlockD self->Base.Append = FileBlockDevice_Append; self->Base.WriteAt = FileBlockDevice_WriteAt; self->Base.Size = FileBlockDevice_Size; + self->Handle = (struct OpenHandle) {.File = file, .BlockIndex = 0, .IsOpen = false}; + self->PathPrefix = pathPrefix; } -/* ------------------------------------------------------------------ - * Destroy - * ----------------------------------------------------------------*/ - -static inline void FileBlockDevice_CloseIfOpen(struct OpenHandle* handle); - -void SolidSyslogFileBlockDevice_Destroy(struct SolidSyslogBlockDevice* base) +void FileBlockDevice_Cleanup(struct SolidSyslogBlockDevice* base) { + /* Close the cached file handle while we can still see it; then overwrite the + * abstract base with the shared NullBlockDevice vtable so use-after-destroy is + * a safe no-op rather than a NULL-fn-pointer crash. Derived fields are private + * to this TU; the next _Initialise overwrites them. */ struct SolidSyslogFileBlockDevice* self = FileBlockDevice_SelfFromBase(base); FileBlockDevice_CloseIfOpen(&self->Handle); + *base = *SolidSyslogNullBlockDevice_Get(); } static inline struct SolidSyslogFileBlockDevice* FileBlockDevice_SelfFromBase(struct SolidSyslogBlockDevice* base) diff --git a/Core/Source/SolidSyslogFileBlockDevicePrivate.h b/Core/Source/SolidSyslogFileBlockDevicePrivate.h new file mode 100644 index 00000000..d1e780a3 --- /dev/null +++ b/Core/Source/SolidSyslogFileBlockDevicePrivate.h @@ -0,0 +1,36 @@ +#ifndef SOLIDSYSLOGFILEBLOCKDEVICEPRIVATE_H +#define SOLIDSYSLOGFILEBLOCKDEVICEPRIVATE_H + +#include +#include + +#include "SolidSyslogBlockDeviceDefinition.h" + +struct SolidSyslogFile; + +/* OpenHandle caches the single SolidSyslogFile the device holds. The handle is + * re-pointed only when the targeted blockIndex changes; same-block runs reuse + * it. This is the structural enforcement of the S27.01 single-handle-per-path + * invariant — by construction the device has exactly one underlying file. */ +struct OpenHandle +{ + struct SolidSyslogFile* File; + size_t BlockIndex; + bool IsOpen; +}; + +struct SolidSyslogFileBlockDevice +{ + struct SolidSyslogBlockDevice Base; + struct OpenHandle Handle; + const char* PathPrefix; +}; + +void FileBlockDevice_Initialise( + struct SolidSyslogBlockDevice* base, + struct SolidSyslogFile* file, + const char* pathPrefix +); +void FileBlockDevice_Cleanup(struct SolidSyslogBlockDevice* base); + +#endif /* SOLIDSYSLOGFILEBLOCKDEVICEPRIVATE_H */ diff --git a/Core/Source/SolidSyslogFileBlockDeviceStatic.c b/Core/Source/SolidSyslogFileBlockDeviceStatic.c new file mode 100644 index 00000000..21f6a184 --- /dev/null +++ b/Core/Source/SolidSyslogFileBlockDeviceStatic.c @@ -0,0 +1,73 @@ +#include "SolidSyslogFileBlockDevice.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogFileBlockDevicePrivate.h" +#include "SolidSyslogNullBlockDevice.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogBlockDevice; +struct SolidSyslogFile; + +static size_t FileBlockDevice_IndexFromHandle(const struct SolidSyslogBlockDevice* base); +static void FileBlockDevice_CleanupAtIndex(size_t index, void* context); + +static bool FileBlockDevice_InUse[SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE]; +static struct SolidSyslogFileBlockDevice FileBlockDevice_Pool[SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE]; +static struct SolidSyslogPoolAllocator FileBlockDevice_Allocator = { + FileBlockDevice_InUse, + SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE +}; + +struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create(struct SolidSyslogFile* file, const char* pathPrefix) +{ + struct SolidSyslogBlockDevice* result = SolidSyslogNullBlockDevice_Get(); + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&FileBlockDevice_Allocator); + if (SolidSyslogPoolAllocator_IndexIsValid(&FileBlockDevice_Allocator, index)) + { + FileBlockDevice_Initialise(&FileBlockDevice_Pool[index].Base, file, pathPrefix); + result = &FileBlockDevice_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_FILEBLOCKDEVICE_POOL_EXHAUSTED); + } + return result; +} + +void SolidSyslogFileBlockDevice_Destroy(struct SolidSyslogBlockDevice* base) +{ + size_t index = FileBlockDevice_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&FileBlockDevice_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&FileBlockDevice_Allocator, index, FileBlockDevice_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_FILEBLOCKDEVICE_UNKNOWN_DESTROY); + } +} + +static size_t FileBlockDevice_IndexFromHandle(const struct SolidSyslogBlockDevice* base) +{ + size_t result = SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE; poolIndex++) + { + if (base == &FileBlockDevice_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void FileBlockDevice_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + FileBlockDevice_Cleanup(&FileBlockDevice_Pool[index].Base); +} diff --git a/Core/Source/SolidSyslogNullBlockDevice.c b/Core/Source/SolidSyslogNullBlockDevice.c new file mode 100644 index 00000000..aeb42efc --- /dev/null +++ b/Core/Source/SolidSyslogNullBlockDevice.c @@ -0,0 +1,128 @@ +#include "SolidSyslogNullBlockDevice.h" + +#include +#include + +#include "SolidSyslogBlockDeviceDefinition.h" + +static bool NullBlockDevice_Acquire(struct SolidSyslogBlockDevice* base, size_t blockIndex); +static bool NullBlockDevice_Dispose(struct SolidSyslogBlockDevice* base, size_t blockIndex); +static bool NullBlockDevice_Exists(struct SolidSyslogBlockDevice* base, size_t blockIndex); +// NOLINTBEGIN(bugprone-easily-swappable-parameters) -- vtable signature mirrored from BlockDevice contract +static bool NullBlockDevice_Read( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + size_t offset, + void* buf, + size_t count +); +static bool NullBlockDevice_WriteAt( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + size_t offset, + const void* buf, + size_t count +); +// NOLINTEND(bugprone-easily-swappable-parameters) +static bool NullBlockDevice_Append( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + const void* buf, + size_t count +); +static size_t NullBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t blockIndex); + +static struct SolidSyslogBlockDevice instance = { + .Acquire = NullBlockDevice_Acquire, + .Dispose = NullBlockDevice_Dispose, + .Exists = NullBlockDevice_Exists, + .Read = NullBlockDevice_Read, + .Append = NullBlockDevice_Append, + .WriteAt = NullBlockDevice_WriteAt, + .Size = NullBlockDevice_Size, +}; + +struct SolidSyslogBlockDevice* SolidSyslogNullBlockDevice_Get(void) +{ + return &instance; +} + +static bool NullBlockDevice_Acquire(struct SolidSyslogBlockDevice* base, size_t blockIndex) +{ + (void) base; + (void) blockIndex; + return false; +} + +static bool NullBlockDevice_Dispose(struct SolidSyslogBlockDevice* base, size_t blockIndex) +{ + (void) base; + (void) blockIndex; + return false; +} + +static bool NullBlockDevice_Exists(struct SolidSyslogBlockDevice* base, size_t blockIndex) +{ + (void) base; + (void) blockIndex; + return false; +} + +// NOLINTBEGIN(bugprone-easily-swappable-parameters) -- vtable signature mirrored from BlockDevice contract +static bool NullBlockDevice_Read( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + size_t offset, + void* buf, + size_t count +) +{ + (void) base; + (void) blockIndex; + (void) offset; + (void) buf; + (void) count; + return false; +} + +// NOLINTEND(bugprone-easily-swappable-parameters) + +static bool NullBlockDevice_Append( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + const void* buf, + size_t count +) +{ + (void) base; + (void) blockIndex; + (void) buf; + (void) count; + return false; +} + +// NOLINTBEGIN(bugprone-easily-swappable-parameters) -- vtable signature mirrored from BlockDevice contract +static bool NullBlockDevice_WriteAt( + struct SolidSyslogBlockDevice* base, + size_t blockIndex, + size_t offset, + const void* buf, + size_t count +) +{ + (void) base; + (void) blockIndex; + (void) offset; + (void) buf; + (void) count; + return false; +} + +// NOLINTEND(bugprone-easily-swappable-parameters) + +static size_t NullBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t blockIndex) +{ + (void) base; + (void) blockIndex; + return 0; +} diff --git a/Core/Source/SolidSyslogNullBuffer.c b/Core/Source/SolidSyslogNullBuffer.c new file mode 100644 index 00000000..fbae1dd1 --- /dev/null +++ b/Core/Source/SolidSyslogNullBuffer.c @@ -0,0 +1,36 @@ +#include "SolidSyslogNullBuffer.h" + +#include +#include + +#include "SolidSyslogBufferDefinition.h" + +static bool NullBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead); +static void NullBuffer_Write(struct SolidSyslogBuffer* base, const void* data, size_t size); + +static struct SolidSyslogBuffer instance = {.Write = NullBuffer_Write, .Read = NullBuffer_Read}; + +struct SolidSyslogBuffer* SolidSyslogNullBuffer_Get(void) +{ + return &instance; +} + +/* Read returns false ("nothing to deliver") so the Service algorithm + * sees an empty Buffer and stops draining. */ +static bool NullBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) +{ + (void) base; + (void) data; + (void) maxSize; + *bytesRead = 0; + return false; +} + +/* Write swallows the record — a misconfigured Buffer paired with a + * caller that doesn't gate Log() must not block or crash. */ +static void NullBuffer_Write(struct SolidSyslogBuffer* base, const void* data, size_t size) +{ + (void) base; + (void) data; + (void) size; +} diff --git a/Core/Source/SolidSyslogPassthroughBuffer.c b/Core/Source/SolidSyslogPassthroughBuffer.c index 7e739df3..bfec7e2b 100644 --- a/Core/Source/SolidSyslogPassthroughBuffer.c +++ b/Core/Source/SolidSyslogPassthroughBuffer.c @@ -4,6 +4,7 @@ #include #include "SolidSyslogBufferDefinition.h" +#include "SolidSyslogNullBuffer.h" #include "SolidSyslogPassthroughBufferPrivate.h" #include "SolidSyslogSender.h" @@ -22,10 +23,10 @@ void PassthroughBuffer_Initialise(struct SolidSyslogBuffer* base, struct SolidSy void PassthroughBuffer_Cleanup(struct SolidSyslogBuffer* base) { - /* Overwrite the abstract base with the class-private Fallback vtable so + /* Overwrite the abstract base with the shared NullBuffer vtable so * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. The * Sender pointer is private to this TU; the next _Initialise overwrites it. */ - *base = PassthroughBuffer_Fallback; + *base = *SolidSyslogNullBuffer_Get(); } static bool PassthroughBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) diff --git a/Core/Source/SolidSyslogPassthroughBufferPrivate.h b/Core/Source/SolidSyslogPassthroughBufferPrivate.h index 07b20640..5cb8fdb7 100644 --- a/Core/Source/SolidSyslogPassthroughBufferPrivate.h +++ b/Core/Source/SolidSyslogPassthroughBufferPrivate.h @@ -14,11 +14,4 @@ struct SolidSyslogPassthroughBuffer void PassthroughBuffer_Initialise(struct SolidSyslogBuffer* base, struct SolidSyslogSender* sender); void PassthroughBuffer_Cleanup(struct SolidSyslogBuffer* base); -/* Class-private GoF null Buffer. Defined in SolidSyslogPassthroughBufferStatic.c; - * used both as the pool-exhausted fallback handle returned by _Create, and as the - * vtable swapped into a slot's Base by _Cleanup so use-after-destroy is a safe - * no-op. No shared SolidSyslogNullBuffer exists for E11 — PassthroughBuffer is - * itself the closest thing to a "just forward" Buffer. */ -extern struct SolidSyslogBuffer PassthroughBuffer_Fallback; - #endif /* SOLIDSYSLOGPASSTHROUGHBUFFERPRIVATE_H */ diff --git a/Core/Source/SolidSyslogPassthroughBufferStatic.c b/Core/Source/SolidSyslogPassthroughBufferStatic.c index 8d32392e..6d389088 100644 --- a/Core/Source/SolidSyslogPassthroughBufferStatic.c +++ b/Core/Source/SolidSyslogPassthroughBufferStatic.c @@ -6,6 +6,7 @@ #include "SolidSyslogBufferDefinition.h" #include "SolidSyslogError.h" #include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullBuffer.h" #include "SolidSyslogPassthroughBufferPrivate.h" #include "SolidSyslogPoolAllocator.h" #include "SolidSyslogPrival.h" @@ -13,14 +14,11 @@ struct SolidSyslogSender; -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 size_t PassthroughBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); static void PassthroughBuffer_CleanupAtIndex(size_t index, void* context); static bool PassthroughBuffer_InUse[SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE]; static struct SolidSyslogPassthroughBuffer PassthroughBuffer_Pool[SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE]; -struct SolidSyslogBuffer PassthroughBuffer_Fallback = {Fallback_Write, Fallback_Read}; static struct SolidSyslogPoolAllocator PassthroughBuffer_Allocator = { PassthroughBuffer_InUse, SOLIDSYSLOG_PASSTHROUGH_BUFFER_POOL_SIZE @@ -29,7 +27,7 @@ static struct SolidSyslogPoolAllocator PassthroughBuffer_Allocator = { struct SolidSyslogBuffer* SolidSyslogPassthroughBuffer_Create(struct SolidSyslogSender* sender) { size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PassthroughBuffer_Allocator); - struct SolidSyslogBuffer* handle = &PassthroughBuffer_Fallback; + struct SolidSyslogBuffer* handle = SolidSyslogNullBuffer_Get(); if (SolidSyslogPoolAllocator_IndexIsValid(&PassthroughBuffer_Allocator, index)) { PassthroughBuffer_Initialise(&PassthroughBuffer_Pool[index].Base, sender); @@ -77,19 +75,3 @@ static void PassthroughBuffer_CleanupAtIndex(size_t index, void* context) (void) context; PassthroughBuffer_Cleanup(&PassthroughBuffer_Pool[index].Base); } - -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/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index b2fb4ab2..ad551178 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -26,6 +26,8 @@ set(TEST_SOURCES SolidSyslogPassthroughBufferTest.cpp SolidSyslogCircularBufferTest.cpp SolidSyslogPoolAllocatorTest.cpp + SolidSyslogNullBlockDeviceTest.cpp + SolidSyslogNullBufferTest.cpp SolidSyslogNullMutexTest.cpp SolidSyslogNullSdTest.cpp SolidSyslogNullSenderTest.cpp diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 7601d746..463a8f9b 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -113,16 +113,15 @@ struct DrainTestConfig // clang-format off TEST_BASE(DrainTestFixtureBase) { - struct FileFakeStorage fileStorage = {}; - struct SolidSyslogFile* file = nullptr; - SolidSyslogFileBlockDeviceStorage deviceStorage = {}; - struct SolidSyslogBlockDevice* device = nullptr; - struct SolidSyslogSecurityPolicy* policy = nullptr; + struct FileFakeStorage fileStorage = {}; + struct SolidSyslogFile* file = nullptr; + struct SolidSyslogBlockDevice* device = nullptr; + struct SolidSyslogSecurityPolicy* policy = nullptr; void setupBlockDeviceAndPolicy() { file = FileFake_Create(&fileStorage); - device = SolidSyslogFileBlockDevice_Create(&deviceStorage, file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); policy = SolidSyslogNullSecurityPolicy_Get(); } diff --git a/Tests/SolidSyslogBlockStorePosixTest.cpp b/Tests/SolidSyslogBlockStorePosixTest.cpp index e2c77530..78d9ebb1 100644 --- a/Tests/SolidSyslogBlockStorePosixTest.cpp +++ b/Tests/SolidSyslogBlockStorePosixTest.cpp @@ -41,7 +41,6 @@ TEST_GROUP(SolidSyslogBlockStorePosix) SolidSyslogPosixFileStorage fileStorage = {}; struct SolidSyslogFile* file = nullptr; - SolidSyslogFileBlockDeviceStorage deviceStorage = {}; struct SolidSyslogBlockDevice* device = nullptr; struct SolidSyslogStore* store = nullptr; char maxMsg[SOLIDSYSLOG_MAX_MESSAGE_SIZE] = {}; @@ -50,7 +49,7 @@ TEST_GROUP(SolidSyslogBlockStorePosix) { CleanStoreFiles(); file = SolidSyslogPosixFile_Create(&fileStorage); - device = SolidSyslogFileBlockDevice_Create(&deviceStorage, file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); std::memset(maxMsg, 'A', sizeof(maxMsg)); } diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index e0c95499..46217606 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -71,13 +71,12 @@ TEST_BASE(BlockDeviceTestBase) { struct FileFakeStorage storage = {}; struct SolidSyslogFile* file = nullptr; - SolidSyslogFileBlockDeviceStorage deviceStorage = {}; struct SolidSyslogBlockDevice* device = nullptr; void setupBlockDeviceFakes() { file = FileFake_Create(&storage); - device = SolidSyslogFileBlockDevice_Create(&deviceStorage, file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); } void teardownBlockDeviceFakes() const @@ -498,7 +497,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) { SolidSyslogFileBlockDevice_Destroy(device); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - device = SolidSyslogFileBlockDevice_Create(&deviceStorage, file, prefix); + device = SolidSyslogFileBlockDevice_Create(file, prefix); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros store = SolidSyslogBlockStore_Create(&storeStorage, &config); diff --git a/Tests/SolidSyslogCircularBufferTest.cpp b/Tests/SolidSyslogCircularBufferTest.cpp index 73bd6bff..8207c6b1 100644 --- a/Tests/SolidSyslogCircularBufferTest.cpp +++ b/Tests/SolidSyslogCircularBufferTest.cpp @@ -108,12 +108,11 @@ TEST(SolidSyslogCircularBuffer, CreateDestroyDoesNotCrash) { } -TEST(SolidSyslogCircularBuffer, UseAfterDestroyIsCrashSafeViaFallbackVtable) +TEST(SolidSyslogCircularBuffer, UseAfterDestroyIsCrashSafeViaNullBufferVtable) { - /* After Destroy the slot's abstract-base vtable is the class-private Fallback's, - * so Write/Read through the stale handle is a safe no-op rather than a NULL-fn- - * pointer crash. Fallback.Write swallows; Fallback.Read returns false with - * bytesRead=0. */ + /* After Destroy the slot's abstract-base vtable is the shared NullBuffer's, so + * Write/Read through the stale handle is a safe no-op rather than a NULL-fn-pointer + * crash. NullBuffer.Write swallows; NullBuffer.Read returns false with bytesRead=0. */ SolidSyslogCircularBuffer_Destroy(buffer); Write("x"); diff --git a/Tests/SolidSyslogFileBlockDeviceTest.cpp b/Tests/SolidSyslogFileBlockDeviceTest.cpp index f19cb6ca..b60de21c 100644 --- a/Tests/SolidSyslogFileBlockDeviceTest.cpp +++ b/Tests/SolidSyslogFileBlockDeviceTest.cpp @@ -2,6 +2,7 @@ #include "SolidSyslogBlockDevice.h" #include "SolidSyslogFileBlockDevice.h" #include "SolidSyslogFile.h" +#include "SolidSyslogTunables.h" #include "CppUTest/TestHarness.h" class TEST_SolidSyslogFileBlockDevice_AcquireSecondBlockPreservesFirstBlockContent_Test; @@ -32,16 +33,15 @@ static const char* const TEST_PATH_PREFIX = "/tmp/blockdev_"; // clang-format off TEST_GROUP(SolidSyslogFileBlockDevice) { - struct FileFakeStorage fileStorage = {}; - struct SolidSyslogFile* file = nullptr; - SolidSyslogFileBlockDeviceStorage deviceStorage = {}; - struct SolidSyslogBlockDevice* device = nullptr; + struct FileFakeStorage fileStorage = {}; + struct SolidSyslogFile* file = nullptr; + struct SolidSyslogBlockDevice* device = nullptr; void setup() override { file = FileFake_Create(&fileStorage); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - device = SolidSyslogFileBlockDevice_Create(&deviceStorage, file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); } void teardown() override @@ -256,3 +256,86 @@ TEST(SolidSyslogFileBlockDevice, ReadFollowedByWriteOnSameBlockDoesNotOpenTwoHan CHECK_TRUE(SolidSyslogBlockDevice_WriteAt(device, 0, 1, "X", 1)); CHECK_BLOCK_CONTAINS(0, 0, "rXc", 3); } + +TEST(SolidSyslogFileBlockDevice, UseAfterDestroyIsCrashSafeViaNullBlockDeviceVtable) +{ + /* After Destroy the slot's abstract-base vtable is the shared NullBlockDevice's, so + * vtable calls through the stale handle are safe no-ops rather than a NULL-fn-pointer + * crash. NullBlockDevice methods return false / 0. */ + SolidSyslogFileBlockDevice_Destroy(device); + + CHECK_FALSE(SolidSyslogBlockDevice_Acquire(device, 0)); + CHECK_FALSE(SolidSyslogBlockDevice_Exists(device, 0)); + CHECK_FALSE(SolidSyslogBlockDevice_Dispose(device, 0)); + char buf[3] = {}; + CHECK_FALSE(SolidSyslogBlockDevice_Read(device, 0, 0, buf, 3)); + CHECK_FALSE(SolidSyslogBlockDevice_Append(device, 0, "x", 1)); + CHECK_FALSE(SolidSyslogBlockDevice_WriteAt(device, 0, 0, "x", 1)); + LONGS_EQUAL(0, SolidSyslogBlockDevice_Size(device, 0)); + + device = nullptr; // teardown's nullptr guard skips the second Destroy +} + +// Pool tests — prove SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE caps live +// instances and overflow falls back to the class-private no-op BlockDevice. +// Generic pool mechanics (lock counts, per-probe locking, stale-handle warning) +// are covered by SolidSyslogPoolAllocatorTest.cpp. + +// clang-format off +TEST_GROUP(SolidSyslogFileBlockDevicePool) +{ + struct FileFakeStorage fileStorage = {}; + struct SolidSyslogFile* file = nullptr; + struct SolidSyslogBlockDevice* pooled[SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE] = {}; + struct SolidSyslogBlockDevice* overflow = nullptr; + + void setup() override + { + file = FileFake_Create(&fileStorage); + } + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogFileBlockDevice_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogFileBlockDevice_Destroy(overflow); + } + FileFake_Destroy(); + } + + [[nodiscard]] struct SolidSyslogBlockDevice* MakeDevice() const + { + return SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = MakeDevice(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogFileBlockDevicePool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = MakeDevice(); + + CHECK_TEXT(overflow != nullptr, "Fallback handle was nullptr"); + for (auto* slot : pooled) + { + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); + CHECK_TEXT(overflow != slot, "Fallback handle collided with a pool slot"); + } +} diff --git a/Tests/SolidSyslogNullBlockDeviceTest.cpp b/Tests/SolidSyslogNullBlockDeviceTest.cpp new file mode 100644 index 00000000..d0e5e0ee --- /dev/null +++ b/Tests/SolidSyslogNullBlockDeviceTest.cpp @@ -0,0 +1,63 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogBlockDevice.h" +#include "SolidSyslogNullBlockDevice.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullBlockDevice) +{ + struct SolidSyslogBlockDevice* device = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + device = SolidSyslogNullBlockDevice_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullBlockDevice, GetReturnsNonNull) +{ + CHECK_TRUE(device != nullptr); +} + +TEST(SolidSyslogNullBlockDevice, AcquireReturnsFalse) +{ + CHECK_FALSE(SolidSyslogBlockDevice_Acquire(device, 0)); +} + +TEST(SolidSyslogNullBlockDevice, DisposeReturnsFalse) +{ + CHECK_FALSE(SolidSyslogBlockDevice_Dispose(device, 0)); +} + +TEST(SolidSyslogNullBlockDevice, ExistsReturnsFalse) +{ + CHECK_FALSE(SolidSyslogBlockDevice_Exists(device, 0)); +} + +TEST(SolidSyslogNullBlockDevice, ReadReturnsFalse) +{ + char buf[4] = {}; + CHECK_FALSE(SolidSyslogBlockDevice_Read(device, 0, 0, buf, sizeof(buf))); +} + +TEST(SolidSyslogNullBlockDevice, AppendReturnsFalse) +{ + CHECK_FALSE(SolidSyslogBlockDevice_Append(device, 0, "x", 1)); +} + +TEST(SolidSyslogNullBlockDevice, WriteAtReturnsFalse) +{ + CHECK_FALSE(SolidSyslogBlockDevice_WriteAt(device, 0, 0, "x", 1)); +} + +TEST(SolidSyslogNullBlockDevice, SizeReturnsZero) +{ + LONGS_EQUAL(0, SolidSyslogBlockDevice_Size(device, 0)); +} + +TEST(SolidSyslogNullBlockDevice, GetIsIdempotentAndReturnsSameInstance) +{ + POINTERS_EQUAL(device, SolidSyslogNullBlockDevice_Get()); +} diff --git a/Tests/SolidSyslogNullBufferTest.cpp b/Tests/SolidSyslogNullBufferTest.cpp new file mode 100644 index 00000000..52b44c31 --- /dev/null +++ b/Tests/SolidSyslogNullBufferTest.cpp @@ -0,0 +1,41 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogBuffer.h" +#include "SolidSyslogNullBuffer.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullBuffer) +{ + struct SolidSyslogBuffer* buffer = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + buffer = SolidSyslogNullBuffer_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullBuffer, GetReturnsNonNull) +{ + CHECK_TRUE(buffer != nullptr); +} + +TEST(SolidSyslogNullBuffer, WriteSwallowsRecord) +{ + SolidSyslogBuffer_Write(buffer, "hello", 5); + // Doesn't crash; nothing to assert on the buffer state. +} + +TEST(SolidSyslogNullBuffer, ReadReturnsFalseAndZeroBytes) +{ + char data[16] = {}; + size_t bytesRead = 99; + CHECK_FALSE(SolidSyslogBuffer_Read(buffer, data, sizeof(data), &bytesRead)); + LONGS_EQUAL(0, bytesRead); +} + +TEST(SolidSyslogNullBuffer, GetIsIdempotentAndReturnsSameInstance) +{ + POINTERS_EQUAL(buffer, SolidSyslogNullBuffer_Get()); +} diff --git a/Tests/SolidSyslogPassthroughBufferTest.cpp b/Tests/SolidSyslogPassthroughBufferTest.cpp index cec09007..7f6f896a 100644 --- a/Tests/SolidSyslogPassthroughBufferTest.cpp +++ b/Tests/SolidSyslogPassthroughBufferTest.cpp @@ -82,12 +82,11 @@ TEST(SolidSyslogPassthroughBuffer, ReadReturnsNothingToSend) CHECK_FALSE(sent); } -TEST(SolidSyslogPassthroughBuffer, UseAfterDestroyIsCrashSafeViaFallbackVtable) +TEST(SolidSyslogPassthroughBuffer, UseAfterDestroyIsCrashSafeViaNullBufferVtable) { - /* After Destroy the slot's abstract-base vtable is the class-private Fallback's, - * so Write/Read through the stale handle is a safe no-op rather than a NULL-fn- - * pointer crash. Fallback.Write swallows; Fallback.Read returns false with - * bytesRead=0. */ + /* After Destroy the slot's abstract-base vtable is the shared NullBuffer's, so + * Write/Read through the stale handle is a safe no-op rather than a NULL-fn-pointer + * crash. NullBuffer.Write swallows; NullBuffer.Read returns false with bytesRead=0. */ SolidSyslogPassthroughBuffer_Destroy(buffer); SolidSyslogBuffer_Write(buffer, "x", 1); diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 70257c7d..895f9ce9 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -24,9 +24,8 @@ 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:86 -misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:118 -misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:146 +misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:87 +misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:102 misra-c2012-11.3:Core/Source/SolidSyslogFormatter.c:93 misra-c2012-11.3:Core/Source/SolidSyslogMetaSd.c:83 misra-c2012-11.3:Core/Source/SolidSyslogPassthroughBuffer.c:54 @@ -95,14 +94,14 @@ 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/SolidSyslogCircularBuffer.c:15 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/SolidSyslogCrc16.c:12 misra-c2012-5.7:Core/Source/SolidSyslogCrc16Policy.c:10 -misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:14 -misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:21 +misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:16 +misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:23 misra-c2012-5.7:Core/Source/SolidSyslogOriginSd.c:6 misra-c2012-5.7:Core/Source/SolidSyslogUdpPayload.c:5 misra-c2012-5.7:Core/Interface/SolidSyslogAtomicCounter.h:11 @@ -185,7 +184,7 @@ 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 misra-c2012-2.4:Core/Source/BlockSequence.c:92 -misra-c2012-2.4:Core/Source/SolidSyslogCircularBuffer.c:14 +misra-c2012-2.4:Core/Source/SolidSyslogCircularBuffer.c:15 misra-c2012-2.4:Core/Source/SolidSyslogStreamSender.c:19 # D.011 — Rule 20.10: `#` stringification in _Static_assert polyfill From 8446f931bda1179986086ca1a0051f211f599d79 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 07:12:39 +0100 Subject: [PATCH 4/5] docs: update DEVLOG for S11.05 part A Captures the three commits (StreamSender + crash-safety retrofit + FileBlockDevice migration + shared NullBlockDevice / NullBuffer) and flags what's deferred to part B (the BlockStore composition migration). Co-Authored-By: Claude Opus 4.7 (1M context) --- DEVLOG.md | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/DEVLOG.md b/DEVLOG.md index 6bde8a9d..9d84a57e 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,153 @@ # Dev Log +## 2026-05-19 — S11.05 part A: public storage-cast classes onto PoolAllocator + shared null objects + +This is the first half of S11.05. The story body opened by listing +"bundle vs split" as a deferred question; mid-session it became clear +that the public storage-cast migrations (StreamSender, FileBlockDevice) +plus the use-after-destroy crash-safety retrofit form a coherent unit +that's worth its own review surface, separate from the BlockStore +composition rewrite (RecordStore + BlockSequence + BlockStore — a +second PR follows in the next session). + +Three commits on the branch: + +- `e15a138` — StreamSender migration to the canonical E11 3-TU split. + Public `SolidSyslogStreamSenderStorage` typedef + `SOLIDSYSLOG_STREAM_SENDER_SIZE` + enum deleted; `_Create` loses the storage parameter. Pool tunable + + error messages + pool test added. +- `fb86ca7` — install null-object vtable on `_Cleanup` for crash-safety, + applied across eight classes (CircularBuffer + PassthroughBuffer + + UdpSender + SwitchingSender + StreamSender + MetaSd + TimeQualitySd + + OriginSd). Replaces the previous NULL-out-the-vtable shape that + would have crashed on use-after-destroy via NULL-fn-pointer deref. +- `0a18e3a` — FileBlockDevice migration to the 3-TU split + ship + `SolidSyslogNullBlockDevice` and `SolidSyslogNullBuffer` as two new + shared GoF nulls + retrofit PassthroughBuffer / CircularBuffer to + use the new shared `NullBuffer` instead of their class-private + Fallback. FileBlockDevice arrives fresh with no class-private + Fallback — it uses `NullBlockDevice_Get()` from day one. + +### Decisions + +- **The user-after-destroy crash-safety gap was caught after commit 1 + landed.** David's question: "I think we have been overwriting self + with a null-object so crash safe if used after destroy" — surfaced + that my StreamSender Cleanup (and every S11.04-migrated class on + main, and the S11.01 pilot too) NULL'd the vtable in `_Cleanup` + rather than copying a safe vtable into the slot. Use-after-destroy + would dereference a NULL function pointer. The pre-E11 + `*self = DESTROYED_INSTANCE` pattern had the same hazard. Fix + uniformly across all eight migrated classes. + +- **Overwriting only the abstract base is sufficient.** David's + refinement: external callers only ever dispatch through `Base.`, + so `*base = *SolidSyslogNullSender_Get();` (or `NullSd_Get()`, + `NullBuffer_Get()`, `NullBlockDevice_Get()`) is the entire body of + Cleanup. Derived fields (Config, Connected, Sender, Ring, …) are + TU-private and the next `_Initialise` overwrites them; no need to + wipe them on Cleanup. UdpSender and StreamSender call their own + `Disconnect` first so the live config is still reachable; the rest + is a one-liner. + +- **PassthroughBuffer / CircularBuffer keep class-private Fallback + ... initially.** Commit 1b shipped them with a class-private + `Fallback` static (declared in `Private.h`, defined in `Static.c`) + because S11.04 had explicitly decided against a shared `NullBuffer`. + That decision was made before crash-safety was a requirement; once + every Cleanup needs a null-object vtable, the class-private pattern + is copy-paste pressure that would only get worse as more Buffer-like + classes ship. + +- **FileBlockDevice landing fresh forced the issue.** Bare-prefixed + `Fallback_Read` / `Fallback_Write` in PassthroughBufferStatic.c and + CircularBufferStatic.c already collide on MISRA 5.9 (advisory: + internal-linkage uniqueness across TUs); adding a third file with + `Fallback_Acquire`, `Fallback_Dispose`, … would make it three + collisions instead of two. David's question — "is this the same + issue we just fixed by using the base null object" — pointed at the + cleaner architecture: ship `SolidSyslogNullBlockDevice` and + `SolidSyslogNullBuffer` as two new shared GoF nulls, retrofit the + three classes to use them. Commit 2 lands all of this together. + +- **NullSender.Send still returns true (drop on the floor); NullBuffer + / NullBlockDevice methods return false.** S11.04 set the precedent + for NullSender: returning true keeps Store from filling with + undeliverables when an integrator misconfigures the Sender. The + Buffer / BlockDevice semantics don't have the same retain-vs-drop + asymmetry — false is the natural "couldn't do it" signal at the + null-object boundary. + +- **Use-after-destroy tests are first-class assertions of the new + contract.** One `UseAfterDestroyIsCrashSafeVia…Vtable` test per + migrated class proves that calling the vtable through a stale + handle is a safe no-op. For test-group fixtures whose teardown + auto-destroys the slot (the SD tests, the buffer tests), the test + re-creates after the destroy-then-use so teardown still releases + a live slot. The pattern is mechanical; one test per class. + +- **NullSender / NullSd / NullStore — and now NullBlockDevice / + NullBuffer — all share the same shape.** Public Get-only API, + file-scope `static struct …Definition instance` initialised at + declaration, no Create/Destroy lifecycle. The recent flip of + NullStore / NullSecurityPolicy from `_Create`/`_Destroy` to `_Get` + in S11.04 put the four older classes onto this shape; the two new + ones inherit it. NullMutex stays on the legacy `_Create`/`_Destroy` + shape because the Buffer tests still depend on the Destroy-then- + re-Create dance for mutex setup — a follow-up could flip it but + it's not in scope here. + +- **MISRA invariant honoured at the (file, rule) granularity.** + Branch's cppcheck-misra count: 92 (was 98 on main). Five evaporated + — `CircularBufferStatic` 5.9 + 8.9 and `PassthroughBufferStatic` + 5.9 + 8.9 (the bare Fallback_* statics are gone) and `StreamSender` + 8.9 (DEFAULT_INSTANCE / DESTROYED_INSTANCE deleted by the migration). + Two new — `NullBlockDevice` 8.9 + `NullBuffer` 8.9, same shape as + the existing 8.9 findings on NullSender / NullSd / NullStore. Zero + truly-new architectural findings. + +- **Verified in `cpputest-freertos`** per + `feedback_verify_in_freertos_host_image`. All eight test binaries + green with `FREERTOS_KERNEL_PATH=/opt/freertos/kernel` set: Core + (1178 tests), FatFs (28), Cmsdk (15), FreeRtosDatagram (21), + FreeRtosMutex (5), FreeRtosStaticResolver (10), FreeRtosSysUpTime + (4), FreeRtosTcpStream (37). 1298 tests total. + +- **Pool-size override validated** per AC #9. Full suite green at + `SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE=3` and + `SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE=3` via the + `SOLIDSYSLOG_USER_TUNABLES_FILE` override mechanism. + +### Deferred to part B (next session) + +- **BlockStore composition migration.** The most architecturally + interesting part of S11.05. RecordStore and BlockSequence are + TU-internal sub-components today, embedded by value inside + `struct SolidSyslogBlockStore`. After the migration they each get + their own 3-TU split, pool, and Create/Destroy lifecycle (sized + 1:1 with the BlockStore pool — no separate tunable). BlockStore + switches its slot struct from embedded structs to pointer-into- + pool-slot — the "embedded → pointer" rewrite is the substantive + algorithmic change in S11.05. + +- **NullMutex flip to `_Get`-only.** Currently still on the + legacy `_Create`/`_Destroy` shape. Flip mirrors the S11.04 flips + of NullStore / NullSecurityPolicy. Test-side teardown patterns + need a careful read before flipping — the buffer tests' Mutex + lifecycle is the tricky bit. + +- **MetaSd / OriginSd validation symmetry.** TimeQualitySd grew + NULL-callback validation in S11.04 from a CodeRabbit nudge. + MetaSd already validates; OriginSd doesn't. Belongs in E12 not + in any sweep PR. Surfaced explicitly so it's not forgotten. + +### Open questions + +- None for part A. Pushing to CI; will revisit if CodeRabbit or the + Windows/BDD/integration jobs surface anything. + +--- + ## 2026-05-18 — S11.03: Rename NullBuffer to PassthroughBuffer (#397) Pure mechanical rename — the type is a passthrough that forwards From 84f0edfb37b9f8c0e1a804248c5dfd163c1c4a65 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 07:54:19 +0100 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20S11.05=20part=20A=20CI=20cleanup=20?= =?UTF-8?q?=E2=80=94=20IWYU=20and=20StreamSender=20pool=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part A's CI surfaced two red gates after push: - analyze-iwyu: five include adjustments. Two Static.c files were pulling in SolidSyslogBufferDefinition.h when only the pointer was used (the full definition arrives transitively via the Private.h); FileBlockDevice.c carried an unused SolidSyslogMacros.h and was missing for the uint8_t cast in _FormatBlockFilename; StreamSender.c was using SolidSyslogStreamSenderConfig without a direct include of its public header; NullBufferTest.cpp used size_t without . All fixed; iwyu now clean. - bdd-linux-syslog-ng / bdd-windows-otel: 4+4 TLS/mTLS scenarios silently dropping ("received 0 of 1 messages"). Root cause: the Linux and Windows BDD targets each wire two StreamSenders into a SwitchingSender (plain TCP at one branch, TLS at another) but SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE defaulted to 1 — the second Create returned SolidSyslogNullSender_Get() and dropped on Send. Bumped the default to 2; the previous tunable comment ("almost all integrators wire a single stream-framed sender … as one branch of a SwitchingSender") was too optimistic. A TCP-with-TLS-fallback wiring is a realistic shape and one extra pool slot (~64B/64-bit) is trivial against silent message drop. Comment rewritten to match. Also absorbs two CodeRabbit doc nits — CLAUDE.md BrE consistency on "synchronise"; DEVLOG typo "user-after-destroy" → "use-after-destroy". CodeRabbit also proposed two production-code suggestions to silence the UNKNOWN_DESTROY warning when the caller passes the Create-time null-object fallback back through _Destroy (FileBlockDevice and StreamSender). Declined — the warning is correct signal: the Create-time error already flagged a misconfigured pool, and a Destroy-time warning on the same bad setup makes the misuse *more* discoverable. Suppressing it would hide a real signal when the integrator destroys something they didn't actually own. No crash risk either way — the null-object vtable is safe to destroy. DEVLOG updated with rationale on the same. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 4 +- Core/Interface/SolidSyslogTunablesDefaults.h | 12 +++--- Core/Source/SolidSyslogCircularBufferStatic.c | 2 +- Core/Source/SolidSyslogFileBlockDevice.c | 2 +- .../SolidSyslogPassthroughBufferStatic.c | 2 +- Core/Source/SolidSyslogStreamSender.c | 1 + DEVLOG.md | 37 ++++++++++++++++++- Tests/SolidSyslogNullBufferTest.cpp | 2 + 8 files changed, 51 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 919c2ea0..90bc2f61 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -340,10 +340,10 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogBufferDefinition.h` | Buffer implementors (extension point) | `SolidSyslogBuffer` vtable struct | | `SolidSyslogPassthroughBuffer.h` | System setup code (single-task, no buffering) | `SolidSyslogPassthroughBuffer_Create(sender)`, `_Destroy(buffer)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create` to release the slot. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | | `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) | `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. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | +| `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 synchronisation). Instance struct lives in a library-internal static pool (E11); caller supplies the ring memory only. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | | `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` | +| `SolidSyslogNullMutex.h` | System setup code (single-task, no synchronisation) | `SolidSyslogNullMutex_Create`, `_Destroy` | | `SolidSyslogPosixMutex.h` | System setup code on POSIX targets needing thread-safe buffering | `SolidSyslogPosixMutexStorage`, `SOLIDSYSLOG_POSIXMUTEX_SIZE`, `SolidSyslogPosixMutex_Create(storage)`, `_Destroy(mutex)` (wraps `pthread_mutex_t`) | | `SolidSyslogWindowsMutex.h` | System setup code on Windows targets needing thread-safe buffering | `SolidSyslogWindowsMutexStorage`, `SOLIDSYSLOG_WINDOWSMUTEX_SIZE`, `SolidSyslogWindowsMutex_Create(storage)`, `_Destroy(mutex)` (wraps `CRITICAL_SECTION`) | | `SolidSyslogFreeRtosMutex.h` | System setup code on FreeRTOS targets needing thread-safe buffering | `SolidSyslogFreeRtosMutexStorage`, `SOLIDSYSLOG_FREERTOSMUTEX_SIZE`, `SolidSyslogFreeRtosMutex_Create(storage)`, `_Destroy(mutex)` (wraps `xSemaphoreCreateMutexStatic` — caller's `StaticSemaphore_t` lives inside the injected storage; requires `configSUPPORT_STATIC_ALLOCATION=1`) | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 4faf3d88..447373bb 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -117,16 +117,18 @@ * static pool can simultaneously hold. Each instance carries its * config (resolver/stream/endpoint pointers) and connection state. * - * Default 1 — almost all integrators wire a single stream-framed - * sender (TCP, TLS) into either SolidSyslogConfig directly or as one - * branch of a SwitchingSender. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE - * if more than one is genuinely needed. + * Default 2 — common multi-transport wirings combine a plain TCP + * stream sender with a TLS stream sender behind a SwitchingSender so + * a TLS failure can fall back to plain TCP (or vice-versa). A pool of + * 1 would starve the second branch and silently resolve it to the + * shared SolidSyslogNullSender. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE + * for wirings that need more. * * Floor: 1. Sub-floor values rejected at compile time. */ #ifndef SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE /* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ -#define SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE 1U +#define SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE 2U #endif #if SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE < 1 diff --git a/Core/Source/SolidSyslogCircularBufferStatic.c b/Core/Source/SolidSyslogCircularBufferStatic.c index 476bb83d..db776022 100644 --- a/Core/Source/SolidSyslogCircularBufferStatic.c +++ b/Core/Source/SolidSyslogCircularBufferStatic.c @@ -4,7 +4,6 @@ #include #include -#include "SolidSyslogBufferDefinition.h" #include "SolidSyslogCircularBufferPrivate.h" #include "SolidSyslogError.h" #include "SolidSyslogErrorMessages.h" @@ -13,6 +12,7 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogBuffer; struct SolidSyslogMutex; static size_t CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); diff --git a/Core/Source/SolidSyslogFileBlockDevice.c b/Core/Source/SolidSyslogFileBlockDevice.c index 018e709c..32454107 100644 --- a/Core/Source/SolidSyslogFileBlockDevice.c +++ b/Core/Source/SolidSyslogFileBlockDevice.c @@ -2,12 +2,12 @@ #include #include +#include #include "SolidSyslogBlockDeviceDefinition.h" #include "SolidSyslogFile.h" #include "SolidSyslogFileBlockDevicePrivate.h" #include "SolidSyslogFormatter.h" -#include "SolidSyslogMacros.h" #include "SolidSyslogNullBlockDevice.h" struct SolidSyslogFile; diff --git a/Core/Source/SolidSyslogPassthroughBufferStatic.c b/Core/Source/SolidSyslogPassthroughBufferStatic.c index 6d389088..28012517 100644 --- a/Core/Source/SolidSyslogPassthroughBufferStatic.c +++ b/Core/Source/SolidSyslogPassthroughBufferStatic.c @@ -3,7 +3,6 @@ #include #include -#include "SolidSyslogBufferDefinition.h" #include "SolidSyslogError.h" #include "SolidSyslogErrorMessages.h" #include "SolidSyslogNullBuffer.h" @@ -12,6 +11,7 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogBuffer; struct SolidSyslogSender; static size_t PassthroughBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); diff --git a/Core/Source/SolidSyslogStreamSender.c b/Core/Source/SolidSyslogStreamSender.c index b84483d4..eb985c56 100644 --- a/Core/Source/SolidSyslogStreamSender.c +++ b/Core/Source/SolidSyslogStreamSender.c @@ -9,6 +9,7 @@ #include "SolidSyslogResolver.h" #include "SolidSyslogSenderDefinition.h" #include "SolidSyslogStream.h" +#include "SolidSyslogStreamSender.h" #include "SolidSyslogStreamSenderPrivate.h" #include "SolidSyslogTransport.h" diff --git a/DEVLOG.md b/DEVLOG.md index 9d84a57e..f2129a06 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -30,7 +30,7 @@ Three commits on the branch: ### Decisions -- **The user-after-destroy crash-safety gap was caught after commit 1 +- **The use-after-destroy crash-safety gap was caught after commit 1 landed.** David's question: "I think we have been overwriting self with a null-object so crash safe if used after destroy" — surfaced that my StreamSender Cleanup (and every S11.04-migrated class on @@ -118,6 +118,41 @@ Three commits on the branch: `SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE=3` via the `SOLIDSYSLOG_USER_TUNABLES_FILE` override mechanism. +- **`SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE` default bumped to 2.** + CI on the open PR surfaced 4+4 BDD scenarios (Linux + Windows + TLS/mTLS) failing with "received 0 of 1 messages". Root cause: the + Linux and Windows BDD targets each wire two StreamSenders behind a + SwitchingSender — plain TCP for the `@tcp` scenarios, TLS for the + `@tls`/`@mtls` scenarios. With pool=1 the second Create returned the + shared NullSender and silently dropped on Send. The previous tunable + note ("almost all integrators wire a single stream-framed sender … + as one branch of a SwitchingSender") was too optimistic; a + TCP-with-TLS-fallback wiring is a realistic shape, and the cost of + one extra slot (~64B/64-bit) is trivial against the silent-drop + failure mode. Note kept rewritten to reflect this. + +- **IWYU hygiene on the new Static.c / `Get()` files.** CI flagged + five direct/transitive include mismatches introduced by Part A: two + `Static.c` files were pulling in `SolidSyslogBufferDefinition.h` + when only the pointer was used (Definition arrives transitively + via `…Private.h`); `FileBlockDevice.c` carried a stale + `SolidSyslogMacros.h` and was missing `` for a + `(uint8_t)` cast; `StreamSender.c` was using + `SolidSyslogStreamSenderConfig` without a direct include of its + public header; `NullBufferTest.cpp` used `size_t` without + ``. All fixed; iwyu now clean. + +- **CodeRabbit nits absorbed.** Two doc fixes (CLAUDE.md BrE + consistency on `synchronise`; DEVLOG typo `user-after-destroy` → + `use-after-destroy`). Two production-code suggestions to silence + the `UNKNOWN_DESTROY` warning when the caller passes the + Create-time null-object fallback back through `_Destroy` were + declined — the warning is correct signal: a bad-setup `Create` + emits ERR at detection, and a Destroy-time warning on the same bad + setup makes the misuse *more* discoverable, not less. Suppressing + it would hide a real signal in cases where the integrator destroys + something they didn't actually own. + ### Deferred to part B (next session) - **BlockStore composition migration.** The most architecturally diff --git a/Tests/SolidSyslogNullBufferTest.cpp b/Tests/SolidSyslogNullBufferTest.cpp index 52b44c31..276f58a4 100644 --- a/Tests/SolidSyslogNullBufferTest.cpp +++ b/Tests/SolidSyslogNullBufferTest.cpp @@ -1,3 +1,5 @@ +#include + #include "CppUTest/TestHarness.h" #include "SolidSyslogBuffer.h" #include "SolidSyslogNullBuffer.h"