From 17101b7b5d944bf7ac637d8c304fa35e4647b831 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:01:26 +0100 Subject: [PATCH 01/15] feat: S11.06 add SolidSyslogNullDatagram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared GoF null object for the Datagram vtable, used as the fallback when PosixDatagram_Create (S11.06) or FreeRtosDatagram_Create (S11.08) exhaust their respective pools or hit bad config. - Open returns true so callers do not loop on a false from the no-op. - SendTo returns SENT to drop on the floor — UdpSender treats the message as delivered and the Store does not accumulate undeliverables. Mirrors NullSender_Send's contract. - MaxPayload returns SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD so any direct caller (e.g. the OVERSIZE retry path) sees a sensible default. - Close is a no-op. Compiled under the same network-backend gate as SolidSyslogDatagram.c in Core/Source/CMakeLists.txt. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + Core/Interface/SolidSyslogNullDatagram.h | 12 +++++ Core/Source/CMakeLists.txt | 1 + Core/Source/SolidSyslogNullDatagram.c | 63 ++++++++++++++++++++++++ Tests/CMakeLists.txt | 1 + Tests/SolidSyslogNullDatagramTest.cpp | 38 ++++++++++++++ 6 files changed, 116 insertions(+) create mode 100644 Core/Interface/SolidSyslogNullDatagram.h create mode 100644 Core/Source/SolidSyslogNullDatagram.c create mode 100644 Tests/SolidSyslogNullDatagramTest.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 240652d6..44220543 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -328,6 +328,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogFreeRtosStaticResolver.h` | System setup code on FreeRTOS targets pinned to a hardcoded IPv4 destination (no DNS) | `SolidSyslogFreeRtosStaticResolverStorage`, `SOLIDSYSLOG_FREERTOSSTATICRESOLVER_SIZE`, `SolidSyslogFreeRtosStaticResolver_Create(storage, ipv4Octets)`, `_Destroy(resolver)` (ignores host and transport at Resolve time; port is taken from each call) | | `SolidSyslogDatagram.h` | Sender implementors using datagram transport | `SolidSyslogDatagram_Open`, `_SendTo`, `_Close` | | `SolidSyslogDatagramDefinition.h` | Datagram implementors (extension point) | `SolidSyslogDatagram` vtable struct (`Open`, `SendTo`, `Close`) | +| `SolidSyslogNullDatagram.h` | Any code installing a no-op datagram slot (Open/Close are no-ops, SendTo returns `SENT` to drop on the floor so the Store does not fill with undeliverables, MaxPayload returns the IPv6-safe default) | `SolidSyslogNullDatagram_Get` | | `SolidSyslogFreeRtosDatagram.h` | System setup code on FreeRTOS targets using FreeRTOS-Plus-TCP for UDP | `SolidSyslogFreeRtosDatagramStorage`, `SOLIDSYSLOG_FREERTOSDATAGRAM_SIZE`, `SolidSyslogFreeRtosDatagram_Create(storage)`, `_Destroy(datagram)` (wraps `FreeRTOS_socket` / `FreeRTOS_sendto` / `FreeRTOS_closesocket`) | | `SolidSyslogStream.h` | Sender implementors using stream transport | `SolidSyslogStream_Open`, `_Send`, `_Read`, `_Close`, `SolidSyslogSsize` | | `SolidSyslogStreamDefinition.h` | Stream implementors (extension point) | `SolidSyslogStream` vtable struct (`Open`, `Send`, `Read`, `Close`) | diff --git a/Core/Interface/SolidSyslogNullDatagram.h b/Core/Interface/SolidSyslogNullDatagram.h new file mode 100644 index 00000000..a5d128a2 --- /dev/null +++ b/Core/Interface/SolidSyslogNullDatagram.h @@ -0,0 +1,12 @@ +#ifndef SOLIDSYSLOGNULLDATAGRAM_H +#define SOLIDSYSLOGNULLDATAGRAM_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogDatagram* SolidSyslogNullDatagram_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLDATAGRAM_H */ diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 0aff1dce..e345a030 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -57,6 +57,7 @@ if(SOLIDSYSLOG_POSIX OR SOLIDSYSLOG_WINSOCK OR DEFINED ENV{FREERTOS_KERNEL_PATH} list(APPEND SOURCES SolidSyslogResolver.c SolidSyslogDatagram.c + SolidSyslogNullDatagram.c SolidSyslogUdpSender.c SolidSyslogUdpSenderStatic.c SolidSyslogStreamSender.c diff --git a/Core/Source/SolidSyslogNullDatagram.c b/Core/Source/SolidSyslogNullDatagram.c new file mode 100644 index 00000000..4e67a95b --- /dev/null +++ b/Core/Source/SolidSyslogNullDatagram.c @@ -0,0 +1,63 @@ +#include "SolidSyslogNullDatagram.h" + +#include +#include + +#include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogUdpPayload.h" + +static bool NullDatagram_Open(struct SolidSyslogDatagram* base); +static enum SolidSyslogDatagramSendResult NullDatagram_SendTo( + struct SolidSyslogDatagram* base, + const void* buffer, + size_t size, + const struct SolidSyslogAddress* addr +); +static size_t NullDatagram_MaxPayload(struct SolidSyslogDatagram* base); +static void NullDatagram_Close(struct SolidSyslogDatagram* base); + +static struct SolidSyslogDatagram instance = { + .Open = NullDatagram_Open, + .SendTo = NullDatagram_SendTo, + .MaxPayload = NullDatagram_MaxPayload, + .Close = NullDatagram_Close, +}; + +struct SolidSyslogDatagram* SolidSyslogNullDatagram_Get(void) +{ + return &instance; +} + +static bool NullDatagram_Open(struct SolidSyslogDatagram* base) +{ + (void) base; + return true; +} + +/* SendTo returns SENT so the Service algorithm treats the message as + * delivered and drops it from the Store, rather than retaining an + * undeliverable. Mirrors NullSender_Send's drop-on-the-floor contract. */ +static enum SolidSyslogDatagramSendResult NullDatagram_SendTo( + struct SolidSyslogDatagram* base, + const void* buffer, + size_t size, + const struct SolidSyslogAddress* addr +) +{ + (void) base; + (void) buffer; + (void) size; + (void) addr; + return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT; +} + +static size_t NullDatagram_MaxPayload(struct SolidSyslogDatagram* base) +{ + (void) base; + return SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD; +} + +static void NullDatagram_Close(struct SolidSyslogDatagram* base) +{ + (void) base; +} diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 84155c17..7ac93b40 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -28,6 +28,7 @@ set(TEST_SOURCES SolidSyslogPoolAllocatorTest.cpp SolidSyslogNullBlockDeviceTest.cpp SolidSyslogNullBufferTest.cpp + SolidSyslogNullDatagramTest.cpp SolidSyslogNullMutexTest.cpp SolidSyslogNullSdTest.cpp SolidSyslogNullSenderTest.cpp diff --git a/Tests/SolidSyslogNullDatagramTest.cpp b/Tests/SolidSyslogNullDatagramTest.cpp new file mode 100644 index 00000000..922489a1 --- /dev/null +++ b/Tests/SolidSyslogNullDatagramTest.cpp @@ -0,0 +1,38 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogDatagram.h" +#include "SolidSyslogNullDatagram.h" +#include "SolidSyslogUdpPayload.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullDatagram) +{ + struct SolidSyslogDatagram* datagram = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + datagram = SolidSyslogNullDatagram_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullDatagram, SendToReturnsSentToDropOnTheFloor) +{ + CHECK_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, SolidSyslogDatagram_SendTo(datagram, "x", 1, nullptr)); +} + +TEST(SolidSyslogNullDatagram, OpenReturnsTrue) +{ + CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); +} + +TEST(SolidSyslogNullDatagram, MaxPayloadReturnsIpv6SafeDefault) +{ + CHECK_EQUAL((size_t) SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD, SolidSyslogDatagram_MaxPayload(datagram)); +} + +TEST(SolidSyslogNullDatagram, CloseDoesNotCrash) +{ + SolidSyslogDatagram_Close(datagram); +} From b90e56e3f7a2e48fd690877930a8d9b842a58009 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:05:04 +0100 Subject: [PATCH 02/15] feat: S11.06 add SolidSyslogNullStream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared GoF null object for the Stream vtable, used as the fallback when PosixTcpStream_Create (S11.06), WinsockTcpStream_Create (S11.07), FreeRtosTcpStream_Create (S11.08), or TlsStream_Create (S11.09) exhaust their respective pools or hit bad config. - Open returns true so callers do not loop on a no-op fail. - Send returns true to drop on the floor — StreamSender treats the message as delivered and the Store does not accumulate undeliverables. Mirrors NullSender_Send. - Read returns 0 ("would-block, nothing available") rather than < 0, so the caller does not flag a broken connection and tear it down. - Close is a no-op. NullStream lives in the unconditional Core/Source section because SolidSyslogStream.c (its base dispatcher) is unconditional. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + Core/Interface/SolidSyslogNullStream.h | 12 ++++++ Core/Source/CMakeLists.txt | 1 + Core/Source/SolidSyslogNullStream.c | 58 ++++++++++++++++++++++++++ Tests/CMakeLists.txt | 1 + Tests/SolidSyslogNullStreamTest.cpp | 38 +++++++++++++++++ 6 files changed, 111 insertions(+) create mode 100644 Core/Interface/SolidSyslogNullStream.h create mode 100644 Core/Source/SolidSyslogNullStream.c create mode 100644 Tests/SolidSyslogNullStreamTest.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 44220543..956f4541 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -332,6 +332,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogFreeRtosDatagram.h` | System setup code on FreeRTOS targets using FreeRTOS-Plus-TCP for UDP | `SolidSyslogFreeRtosDatagramStorage`, `SOLIDSYSLOG_FREERTOSDATAGRAM_SIZE`, `SolidSyslogFreeRtosDatagram_Create(storage)`, `_Destroy(datagram)` (wraps `FreeRTOS_socket` / `FreeRTOS_sendto` / `FreeRTOS_closesocket`) | | `SolidSyslogStream.h` | Sender implementors using stream transport | `SolidSyslogStream_Open`, `_Send`, `_Read`, `_Close`, `SolidSyslogSsize` | | `SolidSyslogStreamDefinition.h` | Stream implementors (extension point) | `SolidSyslogStream` vtable struct (`Open`, `Send`, `Read`, `Close`) | +| `SolidSyslogNullStream.h` | Any code installing a no-op stream slot (Open/Close are no-ops, Send returns `true` to drop on the floor so the Store does not fill with undeliverables, Read returns `0` for would-block so the caller does not tear the connection down) | `SolidSyslogNullStream_Get` | | `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(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`. | diff --git a/Core/Interface/SolidSyslogNullStream.h b/Core/Interface/SolidSyslogNullStream.h new file mode 100644 index 00000000..17daa419 --- /dev/null +++ b/Core/Interface/SolidSyslogNullStream.h @@ -0,0 +1,12 @@ +#ifndef SOLIDSYSLOGNULLSTREAM_H +#define SOLIDSYSLOGNULLSTREAM_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogStream* SolidSyslogNullStream_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLSTREAM_H */ diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index e345a030..34a2875a 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -26,6 +26,7 @@ set(SOURCES SolidSyslogCrc16.c SolidSyslogCrc16Policy.c SolidSyslogStream.c + SolidSyslogNullStream.c SolidSyslogStructuredData.c SolidSyslogTimeQualitySd.c SolidSyslogTimeQualitySdStatic.c diff --git a/Core/Source/SolidSyslogNullStream.c b/Core/Source/SolidSyslogNullStream.c new file mode 100644 index 00000000..dd910a4a --- /dev/null +++ b/Core/Source/SolidSyslogNullStream.c @@ -0,0 +1,58 @@ +#include "SolidSyslogNullStream.h" + +#include +#include + +#include "SolidSyslogStreamDefinition.h" + +struct SolidSyslogAddress; + +static bool NullStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr); +static bool NullStream_Send(struct SolidSyslogStream* base, const void* buffer, size_t size); +static SolidSyslogSsize NullStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size); +static void NullStream_Close(struct SolidSyslogStream* base); + +static struct SolidSyslogStream instance = { + .Open = NullStream_Open, + .Send = NullStream_Send, + .Read = NullStream_Read, + .Close = NullStream_Close, +}; + +struct SolidSyslogStream* SolidSyslogNullStream_Get(void) +{ + return &instance; +} + +static bool NullStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr) +{ + (void) base; + (void) addr; + return true; +} + +/* Send returns true so the StreamSender treats the message as delivered + * and the Store does not retain undeliverables. Mirrors NullSender_Send + * and NullDatagram_SendTo. */ +static bool NullStream_Send(struct SolidSyslogStream* base, const void* buffer, size_t size) +{ + (void) base; + (void) buffer; + (void) size; + return true; +} + +/* Read returns 0 ("would-block, nothing available") rather than < 0 (EOF / error) + * so StreamSender does not flag a broken connection and tear it down. */ +static SolidSyslogSsize NullStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size) +{ + (void) base; + (void) buffer; + (void) size; + return 0; +} + +static void NullStream_Close(struct SolidSyslogStream* base) +{ + (void) base; +} diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 7ac93b40..d7706e2d 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -29,6 +29,7 @@ set(TEST_SOURCES SolidSyslogNullBlockDeviceTest.cpp SolidSyslogNullBufferTest.cpp SolidSyslogNullDatagramTest.cpp + SolidSyslogNullStreamTest.cpp SolidSyslogNullMutexTest.cpp SolidSyslogNullSdTest.cpp SolidSyslogNullSenderTest.cpp diff --git a/Tests/SolidSyslogNullStreamTest.cpp b/Tests/SolidSyslogNullStreamTest.cpp new file mode 100644 index 00000000..581f0b73 --- /dev/null +++ b/Tests/SolidSyslogNullStreamTest.cpp @@ -0,0 +1,38 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogNullStream.h" +#include "SolidSyslogStream.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullStream) +{ + struct SolidSyslogStream* stream = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + stream = SolidSyslogNullStream_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullStream, SendReturnsTrueToDropOnTheFloor) +{ + CHECK_TRUE(SolidSyslogStream_Send(stream, "x", 1)); +} + +TEST(SolidSyslogNullStream, OpenReturnsTrue) +{ + CHECK_TRUE(SolidSyslogStream_Open(stream, nullptr)); +} + +TEST(SolidSyslogNullStream, ReadReturnsZeroForWouldBlock) +{ + char buffer[1] = {0}; + CHECK_EQUAL(0, SolidSyslogStream_Read(stream, buffer, sizeof(buffer))); +} + +TEST(SolidSyslogNullStream, CloseDoesNotCrash) +{ + SolidSyslogStream_Close(stream); +} From 3c2c98e43760d974c7efdc7572bdda45b166ec0f Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:12:03 +0100 Subject: [PATCH 03/15] feat: S11.06 add SolidSyslogNullFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared GoF null object for the File vtable, used as the fallback when PosixFile_Create (S11.06), WindowsFile_Create (S11.07), or FatFsFile_Create (S11.09) exhaust their respective pools or hit bad config. Semantics — chosen so a consumer treating NullFile as a non-functional file degrades cleanly: - Open / IsOpen / Read / Exists return false — presents consistently as a closed, empty, non-existent file. - Write / Delete return true — the BlockStore-side caller treats the bytes as persisted (drop-on-the-floor; mirrors NullSender_Send) and delete-of-nonexistent is vacuously success. - Close / SeekTo / Truncate are no-ops. - Size returns 0. In practice the chain is already broken (BlockStore is on NullStore) by the time NullFile is reached, but the semantics here are defensible in isolation too. Lives in the unconditional Core/Source section because SolidSyslogFile.c is unconditional. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + Core/Interface/SolidSyslogNullFile.h | 12 +++ Core/Source/CMakeLists.txt | 1 + Core/Source/SolidSyslogNullFile.c | 109 +++++++++++++++++++++++++++ Tests/CMakeLists.txt | 1 + Tests/SolidSyslogNullFileTest.cpp | 68 +++++++++++++++++ 6 files changed, 192 insertions(+) create mode 100644 Core/Interface/SolidSyslogNullFile.h create mode 100644 Core/Source/SolidSyslogNullFile.c create mode 100644 Tests/SolidSyslogNullFileTest.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 956f4541..34277f6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -354,6 +354,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullStore.h` | System setup code (no store-and-forward) | `SolidSyslogNullStore_Get` | | `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` | +| `SolidSyslogNullFile.h` | Any code installing a no-op file slot (Open/IsOpen/Read/Exists return `false`, Write/Delete return `true` so callers' success paths are not tripped, SeekTo/Truncate/Close are no-ops, Size returns `0`) | `SolidSyslogNullFile_Get` | | `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`) | diff --git a/Core/Interface/SolidSyslogNullFile.h b/Core/Interface/SolidSyslogNullFile.h new file mode 100644 index 00000000..593b788d --- /dev/null +++ b/Core/Interface/SolidSyslogNullFile.h @@ -0,0 +1,12 @@ +#ifndef SOLIDSYSLOGNULLFILE_H +#define SOLIDSYSLOGNULLFILE_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogFile* SolidSyslogNullFile_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLFILE_H */ diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 34a2875a..875c7bdf 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -33,6 +33,7 @@ set(SOURCES SolidSyslogOriginSd.c SolidSyslogOriginSdStatic.c SolidSyslogFile.c + SolidSyslogNullFile.c SolidSyslogBlockStore.c SolidSyslogBlockStoreStatic.c RecordStore.c diff --git a/Core/Source/SolidSyslogNullFile.c b/Core/Source/SolidSyslogNullFile.c new file mode 100644 index 00000000..0c053b56 --- /dev/null +++ b/Core/Source/SolidSyslogNullFile.c @@ -0,0 +1,109 @@ +#include "SolidSyslogNullFile.h" + +#include +#include + +#include "SolidSyslogFileDefinition.h" + +static bool NullFile_Open(struct SolidSyslogFile* base, const char* path); +static void NullFile_Close(struct SolidSyslogFile* base); +static bool NullFile_IsOpen(struct SolidSyslogFile* base); +static bool NullFile_Read(struct SolidSyslogFile* base, void* buf, size_t count); +static bool NullFile_Write(struct SolidSyslogFile* base, const void* buf, size_t count); +static void NullFile_SeekTo(struct SolidSyslogFile* base, size_t offset); +static size_t NullFile_Size(struct SolidSyslogFile* base); +static void NullFile_Truncate(struct SolidSyslogFile* base); +static bool NullFile_Exists(struct SolidSyslogFile* base, const char* path); +static bool NullFile_Delete(struct SolidSyslogFile* base, const char* path); + +static struct SolidSyslogFile instance = { + .Open = NullFile_Open, + .Close = NullFile_Close, + .IsOpen = NullFile_IsOpen, + .Read = NullFile_Read, + .Write = NullFile_Write, + .SeekTo = NullFile_SeekTo, + .Size = NullFile_Size, + .Truncate = NullFile_Truncate, + .Exists = NullFile_Exists, + .Delete = NullFile_Delete, +}; + +struct SolidSyslogFile* SolidSyslogNullFile_Get(void) +{ + return &instance; +} + +/* Open returns false so callers see a consistently non-functional file. + * NullFile is the fallback when PosixFile / WindowsFile / FatFsFile + * Create exhausts the pool — at that point the wider chain + * (BlockStore → NullStore) is already broken; presenting "open failed" + * lets the consumer's existing error path handle it cleanly. */ +static bool NullFile_Open(struct SolidSyslogFile* base, const char* path) +{ + (void) base; + (void) path; + return false; +} + +static void NullFile_Close(struct SolidSyslogFile* base) +{ + (void) base; +} + +static bool NullFile_IsOpen(struct SolidSyslogFile* base) +{ + (void) base; + return false; +} + +static bool NullFile_Read(struct SolidSyslogFile* base, void* buf, size_t count) +{ + (void) base; + (void) buf; + (void) count; + return false; +} + +/* Write returns true so the BlockStore-side caller treats the bytes as + * persisted and does not retry. Mirrors NullSender_Send's drop-on-the-floor. */ +static bool NullFile_Write(struct SolidSyslogFile* base, const void* buf, size_t count) +{ + (void) base; + (void) buf; + (void) count; + return true; +} + +static void NullFile_SeekTo(struct SolidSyslogFile* base, size_t offset) +{ + (void) base; + (void) offset; +} + +static size_t NullFile_Size(struct SolidSyslogFile* base) +{ + (void) base; + return 0U; +} + +static void NullFile_Truncate(struct SolidSyslogFile* base) +{ + (void) base; +} + +static bool NullFile_Exists(struct SolidSyslogFile* base, const char* path) +{ + (void) base; + (void) path; + return false; +} + +/* Delete returns true ("succeeded vacuously") so callers that treat + * delete-of-nonexistent as a no-op are not tripped by the null object. */ +static bool NullFile_Delete(struct SolidSyslogFile* base, const char* path) +{ + (void) base; + (void) path; + return true; +} diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index d7706e2d..91e3db28 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -29,6 +29,7 @@ set(TEST_SOURCES SolidSyslogNullBlockDeviceTest.cpp SolidSyslogNullBufferTest.cpp SolidSyslogNullDatagramTest.cpp + SolidSyslogNullFileTest.cpp SolidSyslogNullStreamTest.cpp SolidSyslogNullMutexTest.cpp SolidSyslogNullSdTest.cpp diff --git a/Tests/SolidSyslogNullFileTest.cpp b/Tests/SolidSyslogNullFileTest.cpp new file mode 100644 index 00000000..bb61eae5 --- /dev/null +++ b/Tests/SolidSyslogNullFileTest.cpp @@ -0,0 +1,68 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogFile.h" +#include "SolidSyslogNullFile.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullFile) +{ + struct SolidSyslogFile* file = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + file = SolidSyslogNullFile_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullFile, OpenReturnsFalse) +{ + CHECK_FALSE(SolidSyslogFile_Open(file, "anywhere")); +} + +TEST(SolidSyslogNullFile, CloseDoesNotCrash) +{ + SolidSyslogFile_Close(file); +} + +TEST(SolidSyslogNullFile, IsOpenReturnsFalse) +{ + CHECK_FALSE(SolidSyslogFile_IsOpen(file)); +} + +TEST(SolidSyslogNullFile, ReadReturnsFalse) +{ + char buffer[1] = {0}; + CHECK_FALSE(SolidSyslogFile_Read(file, buffer, sizeof(buffer))); +} + +TEST(SolidSyslogNullFile, WriteReturnsTrueToDropOnTheFloor) +{ + CHECK_TRUE(SolidSyslogFile_Write(file, "x", 1)); +} + +TEST(SolidSyslogNullFile, SeekToDoesNotCrash) +{ + SolidSyslogFile_SeekTo(file, 0U); +} + +TEST(SolidSyslogNullFile, SizeReturnsZero) +{ + CHECK_EQUAL(0U, SolidSyslogFile_Size(file)); +} + +TEST(SolidSyslogNullFile, TruncateDoesNotCrash) +{ + SolidSyslogFile_Truncate(file); +} + +TEST(SolidSyslogNullFile, ExistsReturnsFalse) +{ + CHECK_FALSE(SolidSyslogFile_Exists(file, "anywhere")); +} + +TEST(SolidSyslogNullFile, DeleteReturnsTrueAsVacuouslySucceeded) +{ + CHECK_TRUE(SolidSyslogFile_Delete(file, "anywhere")); +} From fce49bb1919a3f6230ecef0f3d36a8d454fa2520 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:13:34 +0100 Subject: [PATCH 04/15] feat: S11.06 add SolidSyslogNullResolver Shared GoF null object for the Resolver vtable, used as the fallback when GetAddrInfoResolver_Create (S11.06) or FreeRtosStaticResolver_Create (S11.08) exhaust their respective pools or hit bad config. Single vtable method: - Resolve returns false ("could not resolve") so the caller's existing unresolved-host error path runs naturally. In the pool-exhausted fallback case the matching Sender is already on NullSender, so the send chain is broken end-to-end. Compiled under the same network-backend gate as SolidSyslogResolver.c. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + Core/Interface/SolidSyslogNullResolver.h | 12 +++++++ Core/Source/CMakeLists.txt | 1 + Core/Source/SolidSyslogNullResolver.c | 44 ++++++++++++++++++++++++ Tests/CMakeLists.txt | 1 + Tests/SolidSyslogNullResolverTest.cpp | 23 +++++++++++++ 6 files changed, 82 insertions(+) create mode 100644 Core/Interface/SolidSyslogNullResolver.h create mode 100644 Core/Source/SolidSyslogNullResolver.c create mode 100644 Tests/SolidSyslogNullResolverTest.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 34277f6d..1e5a70e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -325,6 +325,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogTransport.h` | Any code selecting a transport or needing default port constants | `SolidSyslogTransport` enum (`UDP`, `TCP`), `SOLIDSYSLOG_UDP_DEFAULT_PORT`, `SOLIDSYSLOG_TCP_DEFAULT_PORT` | | `SolidSyslogResolver.h` | Any code that needs to resolve a destination | `SolidSyslogResolver_Resolve(resolver, transport, host, port, *out)` | | `SolidSyslogResolverDefinition.h` | Resolver implementors (extension point) | `SolidSyslogResolver` vtable struct (`Resolve`) | +| `SolidSyslogNullResolver.h` | Any code installing a no-op resolver slot (Resolve returns `false` so the caller's unresolved-host error path runs naturally) | `SolidSyslogNullResolver_Get` | | `SolidSyslogFreeRtosStaticResolver.h` | System setup code on FreeRTOS targets pinned to a hardcoded IPv4 destination (no DNS) | `SolidSyslogFreeRtosStaticResolverStorage`, `SOLIDSYSLOG_FREERTOSSTATICRESOLVER_SIZE`, `SolidSyslogFreeRtosStaticResolver_Create(storage, ipv4Octets)`, `_Destroy(resolver)` (ignores host and transport at Resolve time; port is taken from each call) | | `SolidSyslogDatagram.h` | Sender implementors using datagram transport | `SolidSyslogDatagram_Open`, `_SendTo`, `_Close` | | `SolidSyslogDatagramDefinition.h` | Datagram implementors (extension point) | `SolidSyslogDatagram` vtable struct (`Open`, `SendTo`, `Close`) | diff --git a/Core/Interface/SolidSyslogNullResolver.h b/Core/Interface/SolidSyslogNullResolver.h new file mode 100644 index 00000000..6db03731 --- /dev/null +++ b/Core/Interface/SolidSyslogNullResolver.h @@ -0,0 +1,12 @@ +#ifndef SOLIDSYSLOGNULLRESOLVER_H +#define SOLIDSYSLOGNULLRESOLVER_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogResolver* SolidSyslogNullResolver_Get(void); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGNULLRESOLVER_H */ diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 875c7bdf..dbdee9c5 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -58,6 +58,7 @@ endif() if(SOLIDSYSLOG_POSIX OR SOLIDSYSLOG_WINSOCK OR DEFINED ENV{FREERTOS_KERNEL_PATH}) list(APPEND SOURCES SolidSyslogResolver.c + SolidSyslogNullResolver.c SolidSyslogDatagram.c SolidSyslogNullDatagram.c SolidSyslogUdpSender.c diff --git a/Core/Source/SolidSyslogNullResolver.c b/Core/Source/SolidSyslogNullResolver.c new file mode 100644 index 00000000..885c9f14 --- /dev/null +++ b/Core/Source/SolidSyslogNullResolver.c @@ -0,0 +1,44 @@ +#include "SolidSyslogNullResolver.h" + +#include +#include + +#include "SolidSyslogResolverDefinition.h" +#include "SolidSyslogTransport.h" + +struct SolidSyslogAddress; + +static bool NullResolver_Resolve( + struct SolidSyslogResolver* base, + enum SolidSyslogTransport transport, + const char* host, + uint16_t port, + struct SolidSyslogAddress* result +); + +static struct SolidSyslogResolver instance = {.Resolve = NullResolver_Resolve}; + +struct SolidSyslogResolver* SolidSyslogNullResolver_Get(void) +{ + return &instance; +} + +/* Resolve returns false ("could not resolve") so callers' existing + * unresolved-host error path runs naturally. In the pool-exhausted + * fallback case the matching Sender is already on NullSender, so the + * send chain is broken end-to-end. */ +static bool NullResolver_Resolve( + struct SolidSyslogResolver* base, + enum SolidSyslogTransport transport, + const char* host, + uint16_t port, + struct SolidSyslogAddress* result +) +{ + (void) base; + (void) transport; + (void) host; + (void) port; + (void) result; + return false; +} diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 91e3db28..e4a84e33 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -30,6 +30,7 @@ set(TEST_SOURCES SolidSyslogNullBufferTest.cpp SolidSyslogNullDatagramTest.cpp SolidSyslogNullFileTest.cpp + SolidSyslogNullResolverTest.cpp SolidSyslogNullStreamTest.cpp SolidSyslogNullMutexTest.cpp SolidSyslogNullSdTest.cpp diff --git a/Tests/SolidSyslogNullResolverTest.cpp b/Tests/SolidSyslogNullResolverTest.cpp new file mode 100644 index 00000000..61c8c40c --- /dev/null +++ b/Tests/SolidSyslogNullResolverTest.cpp @@ -0,0 +1,23 @@ +#include "CppUTest/TestHarness.h" +#include "SolidSyslogNullResolver.h" +#include "SolidSyslogResolver.h" +#include "SolidSyslogTransport.h" + +// clang-format off +TEST_GROUP(SolidSyslogNullResolver) +{ + struct SolidSyslogResolver* resolver = nullptr; + + void setup() override + { + // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros + resolver = SolidSyslogNullResolver_Get(); + } +}; + +// clang-format on + +TEST(SolidSyslogNullResolver, ResolveReturnsFalse) +{ + CHECK_FALSE(SolidSyslogResolver_Resolve(resolver, SOLIDSYSLOG_TRANSPORT_UDP, "anywhere", 514U, nullptr)); +} From bba44d9bd9e676ca18ff79b9a2449ea0e57b1758 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:16:09 +0100 Subject: [PATCH 05/15] refactor: S11.06 migrate NullMutex to _Get shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The newer GoF nulls (NullSender, NullStore, NullSd, NullSecurityPolicy, NullBuffer, NullBlockDevice, plus this story's NullDatagram, NullStream, NullFile, NullResolver) all use _Get(void) — single static instance, vtable initialised at file scope, no lifecycle. NullMutex was the last holdout still using _Create / _Destroy. Mutex semantics here are stateless (no-op Lock/Unlock), so the Create-time vtable assignment + Destroy-time NULL-out it carried served no purpose. Mechanical: drop _Destroy, replace _Create with _Get returning the file-scope-initialised instance, update all four caller files (NullMutexTest, CircularBufferTest, BlockStoreDrainOrderingTest, SolidSyslogTest) and the CLAUDE.md audience-table row. Pure shape refactor — existing behaviour preserved end-to-end, full suite green. NullMutexTest gains GetReturnsSameInstance to drive the singleton contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- Core/Interface/SolidSyslogNullMutex.h | 5 +---- Core/Source/SolidSyslogNullMutex.c | 19 ++++++------------- ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 3 +-- Tests/SolidSyslogCircularBufferTest.cpp | 11 ++++------- Tests/SolidSyslogNullMutexTest.cpp | 10 +++------- Tests/SolidSyslogTest.cpp | 3 +-- 7 files changed, 17 insertions(+), 36 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1e5a70e9..b039cef8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -346,7 +346,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `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 synchronisation) | `SolidSyslogNullMutex_Create`, `_Destroy` | +| `SolidSyslogNullMutex.h` | System setup code (single-task, no synchronisation) | `SolidSyslogNullMutex_Get` | | `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/SolidSyslogNullMutex.h b/Core/Interface/SolidSyslogNullMutex.h index 4482e26c..d78cd890 100644 --- a/Core/Interface/SolidSyslogNullMutex.h +++ b/Core/Interface/SolidSyslogNullMutex.h @@ -5,10 +5,7 @@ EXTERN_C_BEGIN - struct SolidSyslogMutex; - - struct SolidSyslogMutex* SolidSyslogNullMutex_Create(void); - void SolidSyslogNullMutex_Destroy(void); + struct SolidSyslogMutex* SolidSyslogNullMutex_Get(void); EXTERN_C_END diff --git a/Core/Source/SolidSyslogNullMutex.c b/Core/Source/SolidSyslogNullMutex.c index 4dddc0b8..b48937bd 100644 --- a/Core/Source/SolidSyslogNullMutex.c +++ b/Core/Source/SolidSyslogNullMutex.c @@ -1,25 +1,18 @@ #include "SolidSyslogNullMutex.h" -#include - #include "SolidSyslogMutexDefinition.h" static void NullMutex_Lock(struct SolidSyslogMutex* base); static void NullMutex_Unlock(struct SolidSyslogMutex* base); -static struct SolidSyslogMutex NullMutex_Instance; - -struct SolidSyslogMutex* SolidSyslogNullMutex_Create(void) -{ - NullMutex_Instance.Lock = NullMutex_Lock; - NullMutex_Instance.Unlock = NullMutex_Unlock; - return &NullMutex_Instance; -} +static struct SolidSyslogMutex instance = { + .Lock = NullMutex_Lock, + .Unlock = NullMutex_Unlock, +}; -void SolidSyslogNullMutex_Destroy(void) +struct SolidSyslogMutex* SolidSyslogNullMutex_Get(void) { - NullMutex_Instance.Lock = NULL; - NullMutex_Instance.Unlock = NULL; + return &instance; } static void NullMutex_Lock(struct SolidSyslogMutex* base) diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 6b213b51..79b67a09 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -229,7 +229,7 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) void setup() override { setupBlockDeviceAndPolicy(); - mutex = SolidSyslogNullMutex_Create(); + mutex = SolidSyslogNullMutex_Get(); buffer = SolidSyslogCircularBuffer_Create(mutex, bufferRing, sizeof(bufferRing)); SenderSpy_Init(spy); } @@ -243,7 +243,6 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) store = nullptr; } SolidSyslogCircularBuffer_Destroy(buffer); - SolidSyslogNullMutex_Destroy(); teardownBlockDeviceAndPolicy(); } diff --git a/Tests/SolidSyslogCircularBufferTest.cpp b/Tests/SolidSyslogCircularBufferTest.cpp index 8207c6b1..3b10d567 100644 --- a/Tests/SolidSyslogCircularBufferTest.cpp +++ b/Tests/SolidSyslogCircularBufferTest.cpp @@ -92,13 +92,12 @@ TEST_GROUP_BASE(SolidSyslogCircularBuffer, CircularBufferFixture) void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Get(), ring, sizeof(ring)); } void teardown() override { SolidSyslogCircularBuffer_Destroy(buffer); - SolidSyslogNullMutex_Destroy(); } }; @@ -119,7 +118,7 @@ TEST(SolidSyslogCircularBuffer, UseAfterDestroyIsCrashSafeViaNullBufferVtable) CHECK_FALSE(Read()); LONGS_EQUAL(0, readSize); - buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); // for teardown + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Get(), ring, sizeof(ring)); // for teardown } TEST(SolidSyslogCircularBuffer, ReadFromEmptyReturnsFalse) @@ -279,13 +278,12 @@ TEST_GROUP_BASE(SolidSyslogCircularBufferSmallRing, CircularBufferFixture) void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Create(), ring, sizeof(ring)); + buffer = SolidSyslogCircularBuffer_Create(SolidSyslogNullMutex_Get(), ring, sizeof(ring)); } void teardown() override { SolidSyslogCircularBuffer_Destroy(buffer); - SolidSyslogNullMutex_Destroy(); } }; @@ -390,7 +388,7 @@ TEST_GROUP(SolidSyslogCircularBufferPool) void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - mutex = SolidSyslogNullMutex_Create(); + mutex = SolidSyslogNullMutex_Get(); } void teardown() override @@ -406,7 +404,6 @@ TEST_GROUP(SolidSyslogCircularBufferPool) { SolidSyslogCircularBuffer_Destroy(overflow); } - SolidSyslogNullMutex_Destroy(); ConfigLockFake_Uninstall(); ErrorHandlerFake_Uninstall(); } diff --git a/Tests/SolidSyslogNullMutexTest.cpp b/Tests/SolidSyslogNullMutexTest.cpp index 5deaaeb9..d6520693 100644 --- a/Tests/SolidSyslogNullMutexTest.cpp +++ b/Tests/SolidSyslogNullMutexTest.cpp @@ -10,19 +10,15 @@ TEST_GROUP(SolidSyslogNullMutex) void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - mutex = SolidSyslogNullMutex_Create(); - } - - void teardown() override - { - SolidSyslogNullMutex_Destroy(); + mutex = SolidSyslogNullMutex_Get(); } }; // clang-format on -TEST(SolidSyslogNullMutex, CreateDestroyDoesNotCrash) +TEST(SolidSyslogNullMutex, GetReturnsSameInstance) { + POINTERS_EQUAL(mutex, SolidSyslogNullMutex_Get()); } TEST(SolidSyslogNullMutex, LockUnlockDoesNotCrash) diff --git a/Tests/SolidSyslogTest.cpp b/Tests/SolidSyslogTest.cpp index 8955b92b..c87cd440 100644 --- a/Tests/SolidSyslogTest.cpp +++ b/Tests/SolidSyslogTest.cpp @@ -1543,7 +1543,7 @@ TEST_GROUP(SolidSyslogServiceEagerDrain) fakeSender = SenderFake_Create(); circularBuffer = SolidSyslogCircularBuffer_Create( - SolidSyslogNullMutex_Create(), bufferRing, sizeof(bufferRing)); + SolidSyslogNullMutex_Get(), bufferRing, sizeof(bufferRing)); fakeStore = StoreFake_Create(); SolidSyslogConfig serviceConfig = {}; @@ -1558,7 +1558,6 @@ TEST_GROUP(SolidSyslogServiceEagerDrain) SolidSyslog_Destroy(); StoreFake_Destroy(); SolidSyslogCircularBuffer_Destroy(circularBuffer); - SolidSyslogNullMutex_Destroy(); SenderFake_Destroy(fakeSender); } }; From f85c73c54a5d6f531027e0079c81ec8ddeec7142 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:42:27 +0100 Subject: [PATCH 06/15] refactor: S11.06 migrate PosixMutex onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogPosixMutex.c — vtable + Initialise/Cleanup; no allocation. - SolidSyslogPosixMutexPrivate.h — concrete struct + private signatures, TU-internal. - SolidSyslogPosixMutexStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex bridge. Public API change: Before: SolidSyslogPosixMutex_Create(SolidSyslogPosixMutexStorage*) After: SolidSyslogPosixMutex_Create(void) Public Storage type and SOLIDSYSLOG_POSIX_MUTEX_SIZE enum deleted — caller-supplied storage replaced by the internal pool. _Destroy(base) shape preserved from S11.04. Pool size tunable: SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE, default 1U. Override via SOLIDSYSLOG_USER_TUNABLES_FILE per the existing mechanism. Pool exhaustion returns the shared SolidSyslogNullMutex with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown/stale handle reports WARNING. Same contract as the S11.02 CircularBuffer pilot. Cleanup additionally overwrites the abstract base with NullMutex's vtable so post-destroy Lock/Unlock are safe no-ops rather than NULL function-pointer crashes (mirrors CircularBuffer_Cleanup). Existing tests carry forward as the regression net minus HandleEqualsStorageAddress (no longer meaningful — handle no longer equals injected storage). New 9-test SolidSyslogPosixMutexPool group covers the pool contract: fallback distinctness, error reporting, ConfigLock acquisition pattern (once per probed slot), and stale / unknown destroy reporting. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- Core/Interface/SolidSyslogTunablesDefaults.h | 21 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../Posix/Interface/SolidSyslogPosixMutex.h | 14 +- Platform/Posix/Source/SolidSyslogPosixMutex.c | 32 +--- .../Source/SolidSyslogPosixMutexPrivate.h | 17 ++ .../Source/SolidSyslogPosixMutexStatic.c | 66 +++++++ Tests/SolidSyslogPosixMutexTest.cpp | 170 +++++++++++++++++- 9 files changed, 281 insertions(+), 46 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogPosixMutexStatic.c diff --git a/CLAUDE.md b/CLAUDE.md index b039cef8..c067469b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -347,7 +347,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `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 synchronisation) | `SolidSyslogNullMutex_Get` | -| `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`) | +| `SolidSyslogPosixMutex.h` | System setup code on POSIX targets needing thread-safe buffering | `SolidSyslogPosixMutex_Create(void)`, `_Destroy(base)` (wraps `pthread_mutex_t`). Instance struct lives in a library-internal static pool (E11). Pool-exhaustion fallback is the shared `SolidSyslogNullMutex`. | | `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`) | | `SolidSyslogStore.h` | Library internals consuming a store (Service algorithm) and integrator code querying capacity | `SolidSyslogStore_Write`, `_ReadNextUnsent`, `_MarkSent`, `_HasUnsent`, `_IsHalted`, `_GetTotalBytes`, `_GetUsedBytes` | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index f709c28a..dfa969b4 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -50,6 +50,27 @@ #error "SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogPosixMutex instances the library's internal + * static pool can simultaneously hold. Each instance carries a + * pthread_mutex_t. + * + * Default 1 — almost all integrators wire a single PosixMutex into + * a CircularBuffer or other thread-safe primitive. Bump via + * SOLIDSYSLOG_USER_TUNABLES_FILE if more than one is genuinely + * needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE < 1 +#error "SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 48150c1f..4fb80471 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -63,5 +63,9 @@ "SolidSyslogBlockStore_Create pool exhausted; returning fallback store" #define SOLIDSYSLOG_ERROR_MSG_BLOCKSTORE_UNKNOWN_DESTROY \ "SolidSyslogBlockStore_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_POOL_EXHAUSTED \ + "SolidSyslogPosixMutex_Create pool exhausted; returning fallback mutex" +#define SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_UNKNOWN_DESTROY \ + "SolidSyslogPosixMutex_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index 20c8e64b..c3074867 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -3,6 +3,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogPosixHostname.c Source/SolidSyslogPosixMessageQueueBuffer.c Source/SolidSyslogPosixMutex.c + Source/SolidSyslogPosixMutexStatic.c Source/SolidSyslogPosixProcessId.c Source/SolidSyslogPosixSysUpTime.c Source/SolidSyslogPosixSleep.c diff --git a/Platform/Posix/Interface/SolidSyslogPosixMutex.h b/Platform/Posix/Interface/SolidSyslogPosixMutex.h index 1d9c5a52..c16b91ce 100644 --- a/Platform/Posix/Interface/SolidSyslogPosixMutex.h +++ b/Platform/Posix/Interface/SolidSyslogPosixMutex.h @@ -3,23 +3,11 @@ #include "ExternC.h" -#include - EXTERN_C_BEGIN struct SolidSyslogMutex; - enum - { - SOLIDSYSLOG_POSIX_MUTEX_SIZE = sizeof(intptr_t) * 10U - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_POSIX_MUTEX_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogPosixMutexStorage; - - struct SolidSyslogMutex* SolidSyslogPosixMutex_Create(SolidSyslogPosixMutexStorage * storage); + struct SolidSyslogMutex* SolidSyslogPosixMutex_Create(void); void SolidSyslogPosixMutex_Destroy(struct SolidSyslogMutex * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogPosixMutex.c b/Platform/Posix/Source/SolidSyslogPosixMutex.c index b90c43be..7236ce8e 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMutex.c +++ b/Platform/Posix/Source/SolidSyslogPosixMutex.c @@ -3,46 +3,30 @@ #include #include -#include "SolidSyslogMacros.h" #include "SolidSyslogMutexDefinition.h" - -struct SolidSyslogPosixMutex -{ - struct SolidSyslogMutex Base; - pthread_mutex_t Mutex; -}; - -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogPosixMutex) <= SOLIDSYSLOG_POSIX_MUTEX_SIZE, - "SOLIDSYSLOG_POSIX_MUTEX_SIZE is too small for SolidSyslogPosixMutex layout" -); +#include "SolidSyslogNullMutex.h" +#include "SolidSyslogPosixMutexPrivate.h" static void PosixMutex_Lock(struct SolidSyslogMutex* base); static void PosixMutex_Unlock(struct SolidSyslogMutex* base); -static inline struct SolidSyslogPosixMutex* PosixMutex_SelfFromStorage(SolidSyslogPosixMutexStorage* storage); static inline struct SolidSyslogPosixMutex* PosixMutex_SelfFromBase(struct SolidSyslogMutex* base); -struct SolidSyslogMutex* SolidSyslogPosixMutex_Create(SolidSyslogPosixMutexStorage* storage) +void PosixMutex_Initialise(struct SolidSyslogMutex* base) { - struct SolidSyslogPosixMutex* self = PosixMutex_SelfFromStorage(storage); + struct SolidSyslogPosixMutex* self = PosixMutex_SelfFromBase(base); self->Base.Lock = PosixMutex_Lock; self->Base.Unlock = PosixMutex_Unlock; pthread_mutex_init(&self->Mutex, NULL); - return &self->Base; -} - -static inline struct SolidSyslogPosixMutex* PosixMutex_SelfFromStorage(SolidSyslogPosixMutexStorage* storage) -{ - return (struct SolidSyslogPosixMutex*) storage; } -void SolidSyslogPosixMutex_Destroy(struct SolidSyslogMutex* base) +void PosixMutex_Cleanup(struct SolidSyslogMutex* base) { struct SolidSyslogPosixMutex* self = PosixMutex_SelfFromBase(base); pthread_mutex_destroy(&self->Mutex); - self->Base.Lock = NULL; - self->Base.Unlock = NULL; + /* Overwrite the abstract base with the shared NullMutex vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullMutex_Get(); } static inline struct SolidSyslogPosixMutex* PosixMutex_SelfFromBase(struct SolidSyslogMutex* base) diff --git a/Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h b/Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h new file mode 100644 index 00000000..94e9b87e --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h @@ -0,0 +1,17 @@ +#ifndef SOLIDSYSLOGPOSIXMUTEXPRIVATE_H +#define SOLIDSYSLOGPOSIXMUTEXPRIVATE_H + +#include + +#include "SolidSyslogMutexDefinition.h" + +struct SolidSyslogPosixMutex +{ + struct SolidSyslogMutex Base; + pthread_mutex_t Mutex; +}; + +void PosixMutex_Initialise(struct SolidSyslogMutex* base); +void PosixMutex_Cleanup(struct SolidSyslogMutex* base); + +#endif /* SOLIDSYSLOGPOSIXMUTEXPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c b/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c new file mode 100644 index 00000000..7a0a7984 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c @@ -0,0 +1,66 @@ +#include "SolidSyslogPosixMutex.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullMutex.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPosixMutexPrivate.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +static size_t PosixMutex_IndexFromHandle(const struct SolidSyslogMutex* base); +static void PosixMutex_CleanupAtIndex(size_t index, void* context); + +static bool PosixMutex_InUse[SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE]; +static struct SolidSyslogPosixMutex PosixMutex_Pool[SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE]; +static struct SolidSyslogPoolAllocator PosixMutex_Allocator = {PosixMutex_InUse, SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE}; + +struct SolidSyslogMutex* SolidSyslogPosixMutex_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PosixMutex_Allocator); + struct SolidSyslogMutex* handle = SolidSyslogNullMutex_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&PosixMutex_Allocator, index)) + { + PosixMutex_Initialise(&PosixMutex_Pool[index].Base); + handle = &PosixMutex_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogPosixMutex_Destroy(struct SolidSyslogMutex* base) +{ + size_t index = PosixMutex_IndexFromHandle(base); + bool released = SolidSyslogPoolAllocator_IndexIsValid(&PosixMutex_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&PosixMutex_Allocator, index, PosixMutex_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_UNKNOWN_DESTROY); + } +} + +static size_t PosixMutex_IndexFromHandle(const struct SolidSyslogMutex* base) +{ + size_t result = SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE; poolIndex++) + { + if (base == &PosixMutex_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void PosixMutex_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + PosixMutex_Cleanup(&PosixMutex_Pool[index].Base); +} diff --git a/Tests/SolidSyslogPosixMutexTest.cpp b/Tests/SolidSyslogPosixMutexTest.cpp index 04b96237..bdd76fa2 100644 --- a/Tests/SolidSyslogPosixMutexTest.cpp +++ b/Tests/SolidSyslogPosixMutexTest.cpp @@ -1,17 +1,42 @@ #include "CppUTest/TestHarness.h" + +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogMutex.h" +#include "SolidSyslogMutexDefinition.h" #include "SolidSyslogPosixMutex.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" +#include "TestUtils.h" + +using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings ONCE/NEVER into scope for CALLED_FAKE + +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macros preserve __FILE__/__LINE__ at the call site + +// Asserts handle is non-null and not one of the slots in pool. +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) // clang-format off TEST_GROUP(SolidSyslogPosixMutex) { - SolidSyslogPosixMutexStorage storage{}; - struct SolidSyslogMutex* mutex = nullptr; + struct SolidSyslogMutex* mutex = nullptr; void setup() override { // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - mutex = SolidSyslogPosixMutex_Create(&storage); + mutex = SolidSyslogPosixMutex_Create(); } void teardown() override @@ -26,13 +51,142 @@ TEST(SolidSyslogPosixMutex, CreateDestroyDoesNotCrash) { } -TEST(SolidSyslogPosixMutex, HandleEqualsStorageAddress) -{ - POINTERS_EQUAL(&storage, mutex); -} - TEST(SolidSyslogPosixMutex, LockUnlockDoesNotCrash) { SolidSyslogMutex_Lock(mutex); SolidSyslogMutex_Unlock(mutex); } + +// clang-format off +TEST_GROUP(SolidSyslogPosixMutexPool) +{ + struct SolidSyslogMutex* pooled[SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE] = {}; + struct SolidSyslogMutex* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogPosixMutex_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogPosixMutex_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogPosixMutex_Create(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogPosixMutexPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogPosixMutex_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogPosixMutexPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogPosixMutex_Create(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixMutexPool, FallbackLockUnlockAreNoOps) +{ + FillPool(); + overflow = SolidSyslogPosixMutex_Create(); + + SolidSyslogMutex_Lock(overflow); + SolidSyslogMutex_Unlock(overflow); +} + +TEST(SolidSyslogPosixMutexPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogPosixMutex_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixMutexPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogPosixMutex_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogPosixMutexPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogPosixMutex_Create(); + ConfigLockFake_Install(); + + SolidSyslogPosixMutex_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixMutexPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogMutex stranger = {}; + + SolidSyslogPosixMutex_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogPosixMutexPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogMutex stranger = {}; + + SolidSyslogPosixMutex_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixMutexPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogPosixMutex_Create(); + SolidSyslogPosixMutex_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogPosixMutex_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} From e9b638d9529ec50dd7fb036face8aecd6ba03d2f Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:48:30 +0100 Subject: [PATCH 07/15] refactor: S11.06 migrate PosixDatagram onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogPosixDatagram.c — vtable + Initialise/Cleanup; no allocation. File-scope `instance` removed. - SolidSyslogPosixDatagramPrivate.h — concrete struct + private signatures, TU-internal. - SolidSyslogPosixDatagramStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex. Public API change: Before: void SolidSyslogPosixDatagram_Destroy(void) After: void SolidSyslogPosixDatagram_Destroy(struct SolidSyslogDatagram*) Pool size tunable: SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE, default 1U. Override via SOLIDSYSLOG_USER_TUNABLES_FILE. Pool exhaustion returns the shared SolidSyslogNullDatagram with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown / stale handle reports WARNING. Same contract as PosixMutex. PosixDatagram_Initialise now explicitly resets Fd = INVALID_FD and Connected = false rather than relying on the file-scope designated initialiser. Behaviour-preserving fix that surfaced naturally with the pool migration (slots are zero-initialised, where Fd=0 would be a valid file descriptor). Cleanup additionally overwrites the abstract base with NullDatagram's vtable so post-destroy Open/SendTo/MaxPayload/Close are safe no-ops. Existing 28 tests carry forward as the regression net. Teardown updated to pass the datagram handle in three caller files: SolidSyslogUdpSenderTest.cpp, BddTargetServiceThreadTest.cpp, Bdd/Targets/Linux/main.c. New 9-test SolidSyslogPosixDatagramPool group covers the pool contract: fallback distinctness, error reporting, ConfigLock acquisition pattern, and stale / unknown destroy reporting. Co-Authored-By: Claude Opus 4.7 (1M context) --- Bdd/Targets/Linux/main.c | 6 +- Core/Interface/SolidSyslogTunablesDefaults.h | 20 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../Interface/SolidSyslogPosixDatagram.h | 4 +- .../Posix/Source/SolidSyslogPosixDatagram.c | 45 ++--- .../Source/SolidSyslogPosixDatagramPrivate.h | 18 ++ .../Source/SolidSyslogPosixDatagramStatic.c | 70 ++++++++ .../Targets/BddTargetServiceThreadTest.cpp | 14 +- Tests/SolidSyslogPosixDatagramTest.cpp | 154 +++++++++++++++++- Tests/SolidSyslogUdpSenderTest.cpp | 4 +- 11 files changed, 306 insertions(+), 34 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogPosixDatagramPrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 82283747..5cae26f8 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -56,6 +56,7 @@ static struct SolidSyslogStream* plainTcpStream; static struct SolidSyslogSender* plainTcpSender; static struct SolidSyslogSender* udpSender; static struct SolidSyslogSender* switchingSender; +static struct SolidSyslogDatagram* udpDatagram; static void GetTimeQuality(struct SolidSyslogTimeQuality* timeQuality) { @@ -79,9 +80,10 @@ static struct SolidSyslogSender* CreateSender(const struct BddTargetOptions* opt struct SolidSyslogResolver* resolver = SolidSyslogGetAddrInfoResolver_Create(); + udpDatagram = SolidSyslogPosixDatagram_Create(); static struct SolidSyslogUdpSenderConfig udpConfig = {0}; udpConfig.Resolver = resolver; - udpConfig.Datagram = SolidSyslogPosixDatagram_Create(); + udpConfig.Datagram = udpDatagram; udpConfig.Endpoint = BddTargetUdpConfig_GetEndpoint; udpConfig.EndpointVersion = BddTargetUdpConfig_GetEndpointVersion; udpSender = SolidSyslogUdpSender_Create(&udpConfig); @@ -188,7 +190,7 @@ static void DestroySender(void) SolidSyslogStreamSender_Destroy(plainTcpSender); SolidSyslogPosixTcpStream_Destroy(plainTcpStream); SolidSyslogUdpSender_Destroy(udpSender); - SolidSyslogPosixDatagram_Destroy(); + SolidSyslogPosixDatagram_Destroy(udpDatagram); SolidSyslogGetAddrInfoResolver_Destroy(); } diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index dfa969b4..116e020d 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -71,6 +71,26 @@ #error "SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogPosixDatagram instances the library's internal + * static pool can simultaneously hold. Each instance carries the + * AF_INET socket FD and a one-shot connect flag. + * + * Default 1 — almost all integrators wire a single PosixDatagram into + * a UdpSender. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if more than + * one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE < 1 +#error "SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 4fb80471..9875cd80 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -67,5 +67,9 @@ "SolidSyslogPosixMutex_Create pool exhausted; returning fallback mutex" #define SOLIDSYSLOG_ERROR_MSG_POSIXMUTEX_UNKNOWN_DESTROY \ "SolidSyslogPosixMutex_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_POOL_EXHAUSTED \ + "SolidSyslogPosixDatagram_Create pool exhausted; returning fallback datagram" +#define SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_UNKNOWN_DESTROY \ + "SolidSyslogPosixDatagram_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index c3074867..66225e44 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -10,6 +10,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogAddress.c Source/SolidSyslogGetAddrInfoResolver.c Source/SolidSyslogPosixDatagram.c + Source/SolidSyslogPosixDatagramStatic.c Source/SolidSyslogPosixTcpStream.c Source/SolidSyslogPosixFile.c ) diff --git a/Platform/Posix/Interface/SolidSyslogPosixDatagram.h b/Platform/Posix/Interface/SolidSyslogPosixDatagram.h index d8e5fbcd..a4d06896 100644 --- a/Platform/Posix/Interface/SolidSyslogPosixDatagram.h +++ b/Platform/Posix/Interface/SolidSyslogPosixDatagram.h @@ -5,8 +5,10 @@ EXTERN_C_BEGIN + struct SolidSyslogDatagram; + struct SolidSyslogDatagram* SolidSyslogPosixDatagram_Create(void); - void SolidSyslogPosixDatagram_Destroy(void); + void SolidSyslogPosixDatagram_Destroy(struct SolidSyslogDatagram * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogPosixDatagram.c b/Platform/Posix/Source/SolidSyslogPosixDatagram.c index 9d30381a..6a8253be 100644 --- a/Platform/Posix/Source/SolidSyslogPosixDatagram.c +++ b/Platform/Posix/Source/SolidSyslogPosixDatagram.c @@ -5,13 +5,15 @@ #include #include #include -#include #include +#include #include "SolidSyslogAddressInternal.h" +#include "SolidSyslogDatagram.h" #include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogNullDatagram.h" +#include "SolidSyslogPosixDatagramPrivate.h" #include "SolidSyslogUdpPayload.h" -#include "SolidSyslogDatagram.h" struct SolidSyslogAddress; @@ -20,13 +22,6 @@ enum INVALID_FD = -1 }; -struct SolidSyslogPosixDatagram -{ - struct SolidSyslogDatagram Base; - int Fd; - bool Connected; -}; - static bool PosixDatagram_Open(struct SolidSyslogDatagram* base); static enum SolidSyslogDatagramSendResult PosixDatagram_SendTo( struct SolidSyslogDatagram* base, @@ -44,23 +39,29 @@ static inline bool PosixDatagram_ConnectIfNeeded( ); static inline bool PosixDatagram_IsFileDescriptorValid(int fd); -static struct SolidSyslogPosixDatagram instance = {.Fd = INVALID_FD}; - -struct SolidSyslogDatagram* SolidSyslogPosixDatagram_Create(void) +void PosixDatagram_Initialise(struct SolidSyslogDatagram* base) { - instance.Base.Open = PosixDatagram_Open; - instance.Base.SendTo = PosixDatagram_SendTo; - instance.Base.MaxPayload = PosixDatagram_MaxPayload; - instance.Base.Close = PosixDatagram_Close; - return &instance.Base; + struct SolidSyslogPosixDatagram* self = PosixDatagram_SelfFromBase(base); + self->Base.Open = PosixDatagram_Open; + self->Base.SendTo = PosixDatagram_SendTo; + self->Base.MaxPayload = PosixDatagram_MaxPayload; + self->Base.Close = PosixDatagram_Close; + self->Fd = INVALID_FD; + self->Connected = false; } -void SolidSyslogPosixDatagram_Destroy(void) +void PosixDatagram_Cleanup(struct SolidSyslogDatagram* base) { - instance.Base.Open = NULL; - instance.Base.SendTo = NULL; - instance.Base.MaxPayload = NULL; - instance.Base.Close = NULL; + struct SolidSyslogPosixDatagram* self = PosixDatagram_SelfFromBase(base); + if (PosixDatagram_IsFileDescriptorValid(self->Fd)) + { + close(self->Fd); + self->Fd = INVALID_FD; + self->Connected = false; + } + /* Overwrite the abstract base with the shared NullDatagram vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullDatagram_Get(); } static bool PosixDatagram_Open(struct SolidSyslogDatagram* base) diff --git a/Platform/Posix/Source/SolidSyslogPosixDatagramPrivate.h b/Platform/Posix/Source/SolidSyslogPosixDatagramPrivate.h new file mode 100644 index 00000000..bf9951c8 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixDatagramPrivate.h @@ -0,0 +1,18 @@ +#ifndef SOLIDSYSLOGPOSIXDATAGRAMPRIVATE_H +#define SOLIDSYSLOGPOSIXDATAGRAMPRIVATE_H + +#include + +#include "SolidSyslogDatagramDefinition.h" + +struct SolidSyslogPosixDatagram +{ + struct SolidSyslogDatagram Base; + int Fd; + bool Connected; +}; + +void PosixDatagram_Initialise(struct SolidSyslogDatagram* base); +void PosixDatagram_Cleanup(struct SolidSyslogDatagram* base); + +#endif /* SOLIDSYSLOGPOSIXDATAGRAMPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c b/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c new file mode 100644 index 00000000..b70c08ce --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c @@ -0,0 +1,70 @@ +#include "SolidSyslogPosixDatagram.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullDatagram.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPosixDatagramPrivate.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +static size_t PosixDatagram_IndexFromHandle(const struct SolidSyslogDatagram* base); +static void PosixDatagram_CleanupAtIndex(size_t index, void* context); + +static bool PosixDatagram_InUse[SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE]; +static struct SolidSyslogPosixDatagram PosixDatagram_Pool[SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE]; +static struct SolidSyslogPoolAllocator PosixDatagram_Allocator = { + PosixDatagram_InUse, + SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE +}; + +struct SolidSyslogDatagram* SolidSyslogPosixDatagram_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PosixDatagram_Allocator); + struct SolidSyslogDatagram* handle = SolidSyslogNullDatagram_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&PosixDatagram_Allocator, index)) + { + PosixDatagram_Initialise(&PosixDatagram_Pool[index].Base); + handle = &PosixDatagram_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogPosixDatagram_Destroy(struct SolidSyslogDatagram* base) +{ + size_t index = PosixDatagram_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&PosixDatagram_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&PosixDatagram_Allocator, index, PosixDatagram_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_UNKNOWN_DESTROY); + } +} + +static size_t PosixDatagram_IndexFromHandle(const struct SolidSyslogDatagram* base) +{ + size_t result = SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE; poolIndex++) + { + if (base == &PosixDatagram_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void PosixDatagram_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + PosixDatagram_Cleanup(&PosixDatagram_Pool[index].Base); +} diff --git a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp index 381cfefa..0a15a5b8 100644 --- a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp +++ b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp @@ -49,11 +49,12 @@ static uint32_t BddTargetEndpointVersion() // NOLINT(modernize-use-trailing-retu // clang-format off TEST_GROUP(BddTargetServiceThread) { - struct SolidSyslogSender* sender = nullptr; - struct SolidSyslogBuffer* buffer = nullptr; - struct SolidSyslogStore* store = nullptr; + struct SolidSyslogSender* sender = nullptr; + struct SolidSyslogBuffer* buffer = nullptr; + struct SolidSyslogStore* store = nullptr; + struct SolidSyslogDatagram* datagram = nullptr; // cppcheck-suppress variableScope -- member of TEST_GROUP; scope managed by CppUTest macro - volatile bool shutdown; + volatile bool shutdown; void setup() override { @@ -65,7 +66,8 @@ TEST_GROUP(BddTargetServiceThread) lastSleepMs = 0; sleepShutdownFlag = nullptr; - SolidSyslogUdpSenderConfig udpConfig = {SolidSyslogGetAddrInfoResolver_Create(), SolidSyslogPosixDatagram_Create(), BddTargetEndpoint, BddTargetEndpointVersion}; + datagram = SolidSyslogPosixDatagram_Create(); + SolidSyslogUdpSenderConfig udpConfig = {SolidSyslogGetAddrInfoResolver_Create(), datagram, BddTargetEndpoint, BddTargetEndpointVersion}; sender = SolidSyslogUdpSender_Create(&udpConfig); buffer = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); store = SolidSyslogNullStore_Get(); @@ -79,7 +81,7 @@ TEST_GROUP(BddTargetServiceThread) SolidSyslog_Destroy(); SolidSyslogPosixMessageQueueBuffer_Destroy(); SolidSyslogUdpSender_Destroy(sender); - SolidSyslogPosixDatagram_Destroy(); + SolidSyslogPosixDatagram_Destroy(datagram); SolidSyslogGetAddrInfoResolver_Destroy(); } diff --git a/Tests/SolidSyslogPosixDatagramTest.cpp b/Tests/SolidSyslogPosixDatagramTest.cpp index 0308a271..0a408f8e 100644 --- a/Tests/SolidSyslogPosixDatagramTest.cpp +++ b/Tests/SolidSyslogPosixDatagramTest.cpp @@ -3,9 +3,15 @@ using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings NEVER/ONCE/TWICE/THRICE into scope for the CALLED_* // macros +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" #include "SolidSyslogAddress.h" #include "SolidSyslogDatagram.h" +#include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogPosixDatagram.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" #include "SolidSyslogUdpPayload.h" #include "SocketFake.h" #include @@ -15,6 +21,19 @@ using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-f #include #include +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macros preserve __FILE__/__LINE__ at the call site +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) + // clang-format off static const char* const TEST_MESSAGE = "hello"; static const size_t TEST_MESSAGE_LEN = 5; @@ -47,7 +66,7 @@ TEST_GROUP(SolidSyslogPosixDatagram) void teardown() override { - SolidSyslogPosixDatagram_Destroy(); + SolidSyslogPosixDatagram_Destroy(datagram); } }; @@ -251,3 +270,136 @@ TEST(SolidSyslogPosixDatagram, SendToReturnsFailedWhenConnectFails) SolidSyslogDatagram_SendTo(datagram, TEST_MESSAGE, TEST_MESSAGE_LEN, addr) ); } + +// clang-format off +TEST_GROUP(SolidSyslogPosixDatagramPool) +{ + struct SolidSyslogDatagram* pooled[SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE] = {}; + struct SolidSyslogDatagram* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogPosixDatagram_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogPosixDatagram_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogPosixDatagram_Create(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogPosixDatagramPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogPosixDatagram_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogPosixDatagramPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogPosixDatagram_Create(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixDatagramPool, FallbackSendToReturnsSent) +{ + FillPool(); + overflow = SolidSyslogPosixDatagram_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, SolidSyslogDatagram_SendTo(overflow, "x", 1, nullptr)); +} + +TEST(SolidSyslogPosixDatagramPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogPosixDatagram_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixDatagramPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogPosixDatagram_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogPosixDatagramPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogPosixDatagram_Create(); + ConfigLockFake_Install(); + + SolidSyslogPosixDatagram_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixDatagramPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogDatagram stranger = {}; + + SolidSyslogPosixDatagram_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogPosixDatagramPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogDatagram stranger = {}; + + SolidSyslogPosixDatagram_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixDatagramPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogPosixDatagram_Create(); + SolidSyslogPosixDatagram_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogPosixDatagram_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} diff --git a/Tests/SolidSyslogUdpSenderTest.cpp b/Tests/SolidSyslogUdpSenderTest.cpp index 438db5d2..d6cb9d9b 100644 --- a/Tests/SolidSyslogUdpSenderTest.cpp +++ b/Tests/SolidSyslogUdpSenderTest.cpp @@ -125,9 +125,9 @@ TEST_BASE(UdpSenderTestBase) config = {resolver, datagram, TestEndpoint, TestEndpointVersion}; } - static void teardownFakesWithPosixDatagram() + void teardownFakesWithPosixDatagram() const { - SolidSyslogPosixDatagram_Destroy(); + SolidSyslogPosixDatagram_Destroy(datagram); SolidSyslogGetAddrInfoResolver_Destroy(); } From fcf2a0296a7bd21c94a19b588ad15bca4cc56114 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:54:21 +0100 Subject: [PATCH 08/15] refactor: S11.06 migrate GetAddrInfoResolver onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogGetAddrInfoResolver.c — vtable + Initialise/Cleanup. - SolidSyslogGetAddrInfoResolverPrivate.h — concrete struct + private signatures, TU-internal. - SolidSyslogGetAddrInfoResolverStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex. Public API change: Before: void SolidSyslogGetAddrInfoResolver_Destroy(void) After: void SolidSyslogGetAddrInfoResolver_Destroy(struct SolidSyslogResolver*) Pool size tunable: SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE, default 1U. The class is effectively stateless today — the pool slot carries the vtable holder. Migrating for shape consistency with the other Resolver/Datagram/Stream impls that *will* carry state (FreeRtosStaticResolver in S11.08). Pool exhaustion returns the shared SolidSyslogNullResolver with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown / stale handle reports WARNING. Cleanup overwrites the abstract base with NullResolver's vtable so post-destroy Resolve is a safe no-op (returns false). Callers updated to pass the resolver handle to _Destroy: - Tests/SolidSyslogGetAddrInfoResolverTest.cpp - Tests/SolidSyslogUdpSenderTest.cpp - Tests/SolidSyslogStreamSenderTest.cpp (five teardowns) - Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp - Bdd/Targets/Linux/main.c The StreamSenderConfig group's CreateSender previously held the resolver in a local; promoted to a fixture member to keep the teardown reach. main.c's CreateSender similarly captures into a file-scope sharedResolver pointer for DestroySender. New 9-test SolidSyslogGetAddrInfoResolverPool group covers the pool contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- Bdd/Targets/Linux/main.c | 6 +- Core/Interface/SolidSyslogTunablesDefaults.h | 21 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../SolidSyslogGetAddrInfoResolver.h | 4 +- .../Source/SolidSyslogGetAddrInfoResolver.c | 26 ++- .../SolidSyslogGetAddrInfoResolverPrivate.h | 14 ++ .../SolidSyslogGetAddrInfoResolverStatic.c | 74 ++++++++ .../Targets/BddTargetServiceThreadTest.cpp | 6 +- Tests/SolidSyslogGetAddrInfoResolverTest.cpp | 162 +++++++++++++++++- Tests/SolidSyslogStreamSenderTest.cpp | 17 +- Tests/SolidSyslogUdpSenderTest.cpp | 4 +- 12 files changed, 308 insertions(+), 31 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogGetAddrInfoResolverPrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 5cae26f8..9e23605f 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -57,6 +57,7 @@ static struct SolidSyslogSender* plainTcpSender; static struct SolidSyslogSender* udpSender; static struct SolidSyslogSender* switchingSender; static struct SolidSyslogDatagram* udpDatagram; +static struct SolidSyslogResolver* sharedResolver; static void GetTimeQuality(struct SolidSyslogTimeQuality* timeQuality) { @@ -78,7 +79,8 @@ static struct SolidSyslogSender* CreateSender(const struct BddTargetOptions* opt { bool mtlsModeActive = (strcmp(options->Transport, "mtls") == 0); - struct SolidSyslogResolver* resolver = SolidSyslogGetAddrInfoResolver_Create(); + sharedResolver = SolidSyslogGetAddrInfoResolver_Create(); + struct SolidSyslogResolver* resolver = sharedResolver; udpDatagram = SolidSyslogPosixDatagram_Create(); static struct SolidSyslogUdpSenderConfig udpConfig = {0}; @@ -191,7 +193,7 @@ static void DestroySender(void) SolidSyslogPosixTcpStream_Destroy(plainTcpStream); SolidSyslogUdpSender_Destroy(udpSender); SolidSyslogPosixDatagram_Destroy(udpDatagram); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(sharedResolver); } static void DestroyStore(struct SolidSyslogStore* store, const struct BddTargetOptions* options) diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 116e020d..3f0dc403 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -91,6 +91,27 @@ #error "SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogGetAddrInfoResolver instances the library's + * internal static pool can simultaneously hold. The class is + * effectively stateless today — the pool slot carries the vtable + * holder. + * + * Default 1 — almost all integrators wire a single resolver into + * one or more Senders. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if + * more than one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE < 1 +#error "SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 9875cd80..15a7b961 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -71,5 +71,9 @@ "SolidSyslogPosixDatagram_Create pool exhausted; returning fallback datagram" #define SOLIDSYSLOG_ERROR_MSG_POSIXDATAGRAM_UNKNOWN_DESTROY \ "SolidSyslogPosixDatagram_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_POOL_EXHAUSTED \ + "SolidSyslogGetAddrInfoResolver_Create pool exhausted; returning fallback resolver" +#define SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_UNKNOWN_DESTROY \ + "SolidSyslogGetAddrInfoResolver_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index 66225e44..2925c242 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -9,6 +9,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogPosixSleep.c Source/SolidSyslogAddress.c Source/SolidSyslogGetAddrInfoResolver.c + Source/SolidSyslogGetAddrInfoResolverStatic.c Source/SolidSyslogPosixDatagram.c Source/SolidSyslogPosixDatagramStatic.c Source/SolidSyslogPosixTcpStream.c diff --git a/Platform/Posix/Interface/SolidSyslogGetAddrInfoResolver.h b/Platform/Posix/Interface/SolidSyslogGetAddrInfoResolver.h index db883525..8590496d 100644 --- a/Platform/Posix/Interface/SolidSyslogGetAddrInfoResolver.h +++ b/Platform/Posix/Interface/SolidSyslogGetAddrInfoResolver.h @@ -5,8 +5,10 @@ EXTERN_C_BEGIN + struct SolidSyslogResolver; + struct SolidSyslogResolver* SolidSyslogGetAddrInfoResolver_Create(void); - void SolidSyslogGetAddrInfoResolver_Destroy(void); + void SolidSyslogGetAddrInfoResolver_Destroy(struct SolidSyslogResolver * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolver.c b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolver.c index 74ad459b..79c132b8 100644 --- a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolver.c +++ b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolver.c @@ -1,14 +1,16 @@ #include "SolidSyslogGetAddrInfoResolver.h" +#include #include +#include #include #include -#include -#include -#include #include +#include #include "SolidSyslogAddressInternal.h" +#include "SolidSyslogGetAddrInfoResolverPrivate.h" +#include "SolidSyslogNullResolver.h" #include "SolidSyslogResolverDefinition.h" #include "SolidSyslogTransport.h" @@ -28,22 +30,16 @@ static bool GetAddrInfoResolver_Resolve( ); static int GetAddrInfoResolver_MapTransport(enum SolidSyslogTransport transport); -struct SolidSyslogGetAddrInfoResolver -{ - struct SolidSyslogResolver Base; -}; - -static struct SolidSyslogGetAddrInfoResolver instance; - -struct SolidSyslogResolver* SolidSyslogGetAddrInfoResolver_Create(void) +void GetAddrInfoResolver_Initialise(struct SolidSyslogResolver* base) { - instance.Base.Resolve = GetAddrInfoResolver_Resolve; - return &instance.Base; + base->Resolve = GetAddrInfoResolver_Resolve; } -void SolidSyslogGetAddrInfoResolver_Destroy(void) +void GetAddrInfoResolver_Cleanup(struct SolidSyslogResolver* base) { - instance.Base.Resolve = NULL; + /* Overwrite the abstract base with the shared NullResolver vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullResolver_Get(); } static bool GetAddrInfoResolver_Resolve( diff --git a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverPrivate.h b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverPrivate.h new file mode 100644 index 00000000..2aeb8df6 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverPrivate.h @@ -0,0 +1,14 @@ +#ifndef SOLIDSYSLOGGETADDRINFORESOLVERPRIVATE_H +#define SOLIDSYSLOGGETADDRINFORESOLVERPRIVATE_H + +#include "SolidSyslogResolverDefinition.h" + +struct SolidSyslogGetAddrInfoResolver +{ + struct SolidSyslogResolver Base; +}; + +void GetAddrInfoResolver_Initialise(struct SolidSyslogResolver* base); +void GetAddrInfoResolver_Cleanup(struct SolidSyslogResolver* base); + +#endif /* SOLIDSYSLOGGETADDRINFORESOLVERPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c new file mode 100644 index 00000000..08df0788 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c @@ -0,0 +1,74 @@ +#include "SolidSyslogGetAddrInfoResolver.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogGetAddrInfoResolverPrivate.h" +#include "SolidSyslogNullResolver.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +static size_t GetAddrInfoResolver_IndexFromHandle(const struct SolidSyslogResolver* base); +static void GetAddrInfoResolver_CleanupAtIndex(size_t index, void* context); + +static bool GetAddrInfoResolver_InUse[SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE]; +static struct SolidSyslogGetAddrInfoResolver GetAddrInfoResolver_Pool[SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE]; +static struct SolidSyslogPoolAllocator GetAddrInfoResolver_Allocator = { + GetAddrInfoResolver_InUse, + SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE +}; + +struct SolidSyslogResolver* SolidSyslogGetAddrInfoResolver_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&GetAddrInfoResolver_Allocator); + struct SolidSyslogResolver* handle = SolidSyslogNullResolver_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&GetAddrInfoResolver_Allocator, index)) + { + GetAddrInfoResolver_Initialise(&GetAddrInfoResolver_Pool[index].Base); + handle = &GetAddrInfoResolver_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogGetAddrInfoResolver_Destroy(struct SolidSyslogResolver* base) +{ + size_t index = GetAddrInfoResolver_IndexFromHandle(base); + bool released = SolidSyslogPoolAllocator_IndexIsValid(&GetAddrInfoResolver_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse( + &GetAddrInfoResolver_Allocator, + index, + GetAddrInfoResolver_CleanupAtIndex, + NULL + ); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_UNKNOWN_DESTROY); + } +} + +static size_t GetAddrInfoResolver_IndexFromHandle(const struct SolidSyslogResolver* base) +{ + size_t result = SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE; poolIndex++) + { + if (base == &GetAddrInfoResolver_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void GetAddrInfoResolver_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + GetAddrInfoResolver_Cleanup(&GetAddrInfoResolver_Pool[index].Base); +} diff --git a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp index 0a15a5b8..5dfd8e90 100644 --- a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp +++ b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp @@ -53,6 +53,7 @@ TEST_GROUP(BddTargetServiceThread) struct SolidSyslogBuffer* buffer = nullptr; struct SolidSyslogStore* store = nullptr; struct SolidSyslogDatagram* datagram = nullptr; + struct SolidSyslogResolver* resolver = nullptr; // cppcheck-suppress variableScope -- member of TEST_GROUP; scope managed by CppUTest macro volatile bool shutdown; @@ -66,8 +67,9 @@ TEST_GROUP(BddTargetServiceThread) lastSleepMs = 0; sleepShutdownFlag = nullptr; + resolver = SolidSyslogGetAddrInfoResolver_Create(); datagram = SolidSyslogPosixDatagram_Create(); - SolidSyslogUdpSenderConfig udpConfig = {SolidSyslogGetAddrInfoResolver_Create(), datagram, BddTargetEndpoint, BddTargetEndpointVersion}; + SolidSyslogUdpSenderConfig udpConfig = {resolver, datagram, BddTargetEndpoint, BddTargetEndpointVersion}; sender = SolidSyslogUdpSender_Create(&udpConfig); buffer = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); store = SolidSyslogNullStore_Get(); @@ -82,7 +84,7 @@ TEST_GROUP(BddTargetServiceThread) SolidSyslogPosixMessageQueueBuffer_Destroy(); SolidSyslogUdpSender_Destroy(sender); SolidSyslogPosixDatagram_Destroy(datagram); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } static void Log() diff --git a/Tests/SolidSyslogGetAddrInfoResolverTest.cpp b/Tests/SolidSyslogGetAddrInfoResolverTest.cpp index 4cf99346..308d3794 100644 --- a/Tests/SolidSyslogGetAddrInfoResolverTest.cpp +++ b/Tests/SolidSyslogGetAddrInfoResolverTest.cpp @@ -3,11 +3,17 @@ #include #include +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" #include "SolidSyslogAddress.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogGetAddrInfoResolver.h" +#include "SolidSyslogPrival.h" #include "SolidSyslogResolver.h" +#include "SolidSyslogResolverDefinition.h" #include "SocketFake.h" #include "SolidSyslogTransport.h" +#include "SolidSyslogTunables.h" #include "TestUtils.h" #include "CppUTest/TestHarness.h" @@ -35,7 +41,7 @@ TEST_GROUP(SolidSyslogGetAddrInfoResolver) void teardown() override { - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } bool Resolve(const char* host, uint16_t port, enum SolidSyslogTransport transport = SOLIDSYSLOG_TRANSPORT_UDP) @@ -122,3 +128,157 @@ TEST(SolidSyslogGetAddrInfoResolver, FreesAddrInfoOnSuccess) Resolve(TEST_HOST, TEST_PORT); CALLED_FAKE(SocketFake_FreeAddrInfo, ONCE); } + +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macros preserve __FILE__/__LINE__ at the call site +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) + +// clang-format off +TEST_GROUP(SolidSyslogGetAddrInfoResolverPool) +{ + struct SolidSyslogResolver* pooled[SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE] = {}; + struct SolidSyslogResolver* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogGetAddrInfoResolver_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogGetAddrInfoResolver_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogGetAddrInfoResolver_Create(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogGetAddrInfoResolverPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogGetAddrInfoResolver_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogGetAddrInfoResolver_Create(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, FallbackResolveReturnsFalse) +{ + FillPool(); + overflow = SolidSyslogGetAddrInfoResolver_Create(); + + SolidSyslogAddressStorage resultStorage{}; + CHECK_FALSE(SolidSyslogResolver_Resolve( + overflow, + SOLIDSYSLOG_TRANSPORT_UDP, + TEST_HOST, + TEST_PORT, + SolidSyslogAddress_FromStorage(&resultStorage) + )); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogGetAddrInfoResolver_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogGetAddrInfoResolver_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogGetAddrInfoResolver_Create(); + ConfigLockFake_Install(); + + SolidSyslogGetAddrInfoResolver_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogResolver stranger = {}; + + SolidSyslogGetAddrInfoResolver_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogResolver stranger = {}; + + SolidSyslogGetAddrInfoResolver_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogGetAddrInfoResolverPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogGetAddrInfoResolver_Create(); + SolidSyslogGetAddrInfoResolver_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogGetAddrInfoResolver_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} diff --git a/Tests/SolidSyslogStreamSenderTest.cpp b/Tests/SolidSyslogStreamSenderTest.cpp index 2d5c26ad..46841835 100644 --- a/Tests/SolidSyslogStreamSenderTest.cpp +++ b/Tests/SolidSyslogStreamSenderTest.cpp @@ -111,7 +111,7 @@ TEST_GROUP(SolidSyslogStreamSender) { SolidSyslogStreamSender_Destroy(sender); SolidSyslogPosixTcpStream_Destroy(stream); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } void Send() const @@ -169,7 +169,7 @@ TEST_GROUP(SolidSyslogStreamSenderDestroy) void teardown() override { SolidSyslogPosixTcpStream_Destroy(stream); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } void CreateAndDestroy() @@ -338,7 +338,8 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) // cppcheck-suppress unreadVariable -- assigned in CreateSender; cppcheck does not model CppUTest macros int (*getPortFn)(void) = GetPort; // NOLINT(modernize-redundant-void-arg) -- C idiom SolidSyslogPosixTcpStreamStorage streamStorage{}; - struct SolidSyslogStream* stream = nullptr; + struct SolidSyslogStream* stream = nullptr; + struct SolidSyslogResolver* resolver = nullptr; // 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; @@ -357,15 +358,15 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) { SolidSyslogStreamSender_Destroy(sender); SolidSyslogPosixTcpStream_Destroy(stream); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } void CreateSender() { endpointGetHost = getHostFn; endpointGetPort = getPortFn; - struct SolidSyslogResolver* resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + resolver = SolidSyslogGetAddrInfoResolver_Create(); + 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(&config); @@ -467,7 +468,7 @@ TEST_GROUP(SolidSyslogStreamSenderFailure) { SolidSyslogStreamSender_Destroy(sender); SolidSyslogPosixTcpStream_Destroy(stream); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } // NOLINTNEXTLINE(modernize-use-nodiscard) -- test helper; return value intentionally ignored in some tests @@ -679,7 +680,7 @@ TEST_GROUP(SolidSyslogStreamSenderPool) SolidSyslogStreamSender_Destroy(overflow); } SolidSyslogPosixTcpStream_Destroy(stream); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } struct SolidSyslogSender* MakeSender() diff --git a/Tests/SolidSyslogUdpSenderTest.cpp b/Tests/SolidSyslogUdpSenderTest.cpp index d6cb9d9b..3ab3143e 100644 --- a/Tests/SolidSyslogUdpSenderTest.cpp +++ b/Tests/SolidSyslogUdpSenderTest.cpp @@ -128,13 +128,13 @@ TEST_BASE(UdpSenderTestBase) void teardownFakesWithPosixDatagram() const { SolidSyslogPosixDatagram_Destroy(datagram); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } void teardownFakesWithDatagramFake() const { DatagramFake_Destroy(datagram); - SolidSyslogGetAddrInfoResolver_Destroy(); + SolidSyslogGetAddrInfoResolver_Destroy(resolver); } // NOLINTNEXTLINE(modernize-use-nodiscard) -- many test bodies intentionally discard the return From 311749c5fed159e7155a1a69004fe81965185e42 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 21:57:51 +0100 Subject: [PATCH 09/15] refactor: S11.06 migrate PosixFile onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogPosixFile.c — vtable + Initialise/Cleanup; no allocation. - SolidSyslogPosixFilePrivate.h — concrete struct + private signatures, TU-internal. - SolidSyslogPosixFileStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex. Public API change: Before: SolidSyslogPosixFile_Create(SolidSyslogPosixFileStorage*) After: SolidSyslogPosixFile_Create(void) Public Storage type and SOLIDSYSLOG_POSIX_FILE_SIZE enum deleted — caller-supplied storage replaced by the internal pool. Pool size tunable: SOLIDSYSLOG_POSIX_FILE_POOL_SIZE, default 1U. Override via SOLIDSYSLOG_USER_TUNABLES_FILE. Pool exhaustion returns the shared SolidSyslogNullFile with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown / stale handle reports WARNING. The DEFAULT_INSTANCE / DESTROYED_INSTANCE static templates the old storage-cast shape used are dropped — Initialise sets every vtable field explicitly, and Cleanup overwrites the abstract base with NullFile's vtable for the safe-no-op post-destroy contract. Callers updated: - Tests/SolidSyslogPosixFileTest.cpp — drop storage local, plus remove CreateReturnsHandleInsideCallerSuppliedStorage (handle no longer equals storage by construction). - Tests/SolidSyslogBlockStorePosixTest.cpp — drop fileStorage local. - Bdd/Targets/Linux/main.c — drop fileStorage static. New 9-test SolidSyslogPosixFilePool group covers the pool contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- Bdd/Targets/Linux/main.c | 3 +- CLAUDE.md | 2 +- Core/Interface/SolidSyslogTunablesDefaults.h | 20 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../Posix/Interface/SolidSyslogPosixFile.h | 14 +- Platform/Posix/Source/SolidSyslogPosixFile.c | 70 +++----- .../Source/SolidSyslogPosixFilePrivate.h | 15 ++ .../Posix/Source/SolidSyslogPosixFileStatic.c | 66 +++++++ Tests/SolidSyslogBlockStorePosixTest.cpp | 3 +- Tests/SolidSyslogPosixFileTest.cpp | 170 ++++++++++++++++-- 11 files changed, 291 insertions(+), 77 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogPosixFilePrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogPosixFileStatic.c diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 9e23605f..b2f682a5 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -161,8 +161,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetOptions* optio if (useFile) { - static SolidSyslogPosixFileStorage fileStorage; - storeFile = SolidSyslogPosixFile_Create(&fileStorage); + storeFile = SolidSyslogPosixFile_Create(); storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); diff --git a/CLAUDE.md b/CLAUDE.md index c067469b..6208b1cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -365,7 +365,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullSecurityPolicy.h` | System setup code (no integrity checking) | `SolidSyslogNullSecurityPolicy_Get` | | `SolidSyslogCrc16Policy.h` | System setup code using CRC-16 integrity | `SolidSyslogCrc16Policy_Create`, `_Destroy` | | `SolidSyslogCrc16.h` | Any code needing CRC-16 computation | `SolidSyslogCrc16_Compute` | -| `SolidSyslogPosixFile.h` | System setup code using POSIX file I/O | `SolidSyslogPosixFile_Create`, `_Destroy` | +| `SolidSyslogPosixFile.h` | System setup code using POSIX file I/O | `SolidSyslogPosixFile_Create(void)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11). Pool-exhaustion fallback is the shared `SolidSyslogNullFile`. | | `SolidSyslogWindowsFile.h` | System setup code using Windows file I/O (MSVC ``) | `SolidSyslogWindowsFile_Create`, `_Destroy` | | `SolidSyslogFatFsFile.h` | System setup code on targets using ChaN FatFs as the file layer (bare-metal flash / SD / eMMC, FreeRTOS, Zephyr, NuttX, …) | `SolidSyslogFatFsFileStorage`, `SOLIDSYSLOG_FATFS_FILE_SIZE`, `SolidSyslogFatFsFile_Create(storage)`, `_Destroy(file)` (wraps `f_open` / `f_close` / `f_read` / `f_write` with `f_sync` after every successful write so a power loss never loses a record the BlockStore claimed it stored; integrator supplies `diskio.c` and — if `FF_FS_REENTRANT=1` — `ffsystem.c`) | | `SolidSyslogPosixClock.h` | System setup code using POSIX clock | `SolidSyslogPosixClock_GetTimestamp` | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 3f0dc403..d520f2ad 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -112,6 +112,26 @@ #error "SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogPosixFile instances the library's internal + * static pool can simultaneously hold. Each instance carries an + * int file descriptor. + * + * Default 1 — almost all integrators wire a single PosixFile into a + * FileBlockDevice. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if more + * than one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_POSIX_FILE_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_POSIX_FILE_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_POSIX_FILE_POOL_SIZE < 1 +#error "SOLIDSYSLOG_POSIX_FILE_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 15a7b961..a7f5341e 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -75,5 +75,9 @@ "SolidSyslogGetAddrInfoResolver_Create pool exhausted; returning fallback resolver" #define SOLIDSYSLOG_ERROR_MSG_GETADDRINFORESOLVER_UNKNOWN_DESTROY \ "SolidSyslogGetAddrInfoResolver_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_POSIXFILE_POOL_EXHAUSTED \ + "SolidSyslogPosixFile_Create pool exhausted; returning fallback file" +#define SOLIDSYSLOG_ERROR_MSG_POSIXFILE_UNKNOWN_DESTROY \ + "SolidSyslogPosixFile_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index 2925c242..ce0f563a 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -14,6 +14,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogPosixDatagramStatic.c Source/SolidSyslogPosixTcpStream.c Source/SolidSyslogPosixFile.c + Source/SolidSyslogPosixFileStatic.c ) target_include_directories(${PROJECT_NAME} PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/Interface) diff --git a/Platform/Posix/Interface/SolidSyslogPosixFile.h b/Platform/Posix/Interface/SolidSyslogPosixFile.h index 472e39f9..245504a5 100644 --- a/Platform/Posix/Interface/SolidSyslogPosixFile.h +++ b/Platform/Posix/Interface/SolidSyslogPosixFile.h @@ -1,25 +1,13 @@ #ifndef SOLIDSYSLOGPOSIXFILE_H #define SOLIDSYSLOGPOSIXFILE_H -#include - #include "ExternC.h" struct SolidSyslogFile; EXTERN_C_BEGIN - enum - { - SOLIDSYSLOG_POSIX_FILE_SIZE = sizeof(intptr_t) * 11U - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_POSIX_FILE_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogPosixFileStorage; - - struct SolidSyslogFile* SolidSyslogPosixFile_Create(SolidSyslogPosixFileStorage * storage); + struct SolidSyslogFile* SolidSyslogPosixFile_Create(void); void SolidSyslogPosixFile_Destroy(struct SolidSyslogFile * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogPosixFile.c b/Platform/Posix/Source/SolidSyslogPosixFile.c index 9a8f5956..cb8c9423 100644 --- a/Platform/Posix/Source/SolidSyslogPosixFile.c +++ b/Platform/Posix/Source/SolidSyslogPosixFile.c @@ -1,14 +1,15 @@ #include "SolidSyslogPosixFile.h" #include -#include -#include #include #include +#include #include +#include #include "SolidSyslogFileDefinition.h" -#include "SolidSyslogMacros.h" +#include "SolidSyslogNullFile.h" +#include "SolidSyslogPosixFilePrivate.h" #define OWNER_READ_WRITE (S_IRUSR | S_IWUSR) #define DEFAULT_FILE_PERMISSIONS OWNER_READ_WRITE @@ -29,61 +30,35 @@ static void PosixFile_Truncate(struct SolidSyslogFile* base); static bool PosixFile_Exists(struct SolidSyslogFile* base, const char* path); static bool PosixFile_Delete(struct SolidSyslogFile* base, const char* path); -static inline struct SolidSyslogPosixFile* PosixFile_SelfFromStorage(SolidSyslogPosixFileStorage* storage); static inline struct SolidSyslogPosixFile* PosixFile_SelfFromBase(struct SolidSyslogFile* base); -struct SolidSyslogPosixFile +void PosixFile_Initialise(struct SolidSyslogFile* base) { - struct SolidSyslogFile Base; - int Fd; -}; - -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogPosixFile) <= sizeof(SolidSyslogPosixFileStorage), - "SOLIDSYSLOG_POSIX_FILE_SIZE is too small for struct SolidSyslogPosixFile" -); - -static const struct SolidSyslogPosixFile DEFAULT_INSTANCE = { - {PosixFile_Open, - PosixFile_Close, - PosixFile_IsOpen, - PosixFile_Read, - PosixFile_Write, - PosixFile_SeekTo, - PosixFile_Size, - PosixFile_Truncate, - PosixFile_Exists, - PosixFile_Delete}, - INVALID_FD, -}; - -static const struct SolidSyslogPosixFile DESTROYED_INSTANCE = { - {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL}, - INVALID_FD, -}; - -struct SolidSyslogFile* SolidSyslogPosixFile_Create(SolidSyslogPosixFileStorage* storage) -{ - struct SolidSyslogPosixFile* self = PosixFile_SelfFromStorage(storage); - *self = DEFAULT_INSTANCE; - return &self->Base; -} - -static inline struct SolidSyslogPosixFile* PosixFile_SelfFromStorage(SolidSyslogPosixFileStorage* storage) -{ - return (struct SolidSyslogPosixFile*) storage; + struct SolidSyslogPosixFile* self = PosixFile_SelfFromBase(base); + self->Base.Open = PosixFile_Open; + self->Base.Close = PosixFile_Close; + self->Base.IsOpen = PosixFile_IsOpen; + self->Base.Read = PosixFile_Read; + self->Base.Write = PosixFile_Write; + self->Base.SeekTo = PosixFile_SeekTo; + self->Base.Size = PosixFile_Size; + self->Base.Truncate = PosixFile_Truncate; + self->Base.Exists = PosixFile_Exists; + self->Base.Delete = PosixFile_Delete; + self->Fd = INVALID_FD; } -void SolidSyslogPosixFile_Destroy(struct SolidSyslogFile* base) +void PosixFile_Cleanup(struct SolidSyslogFile* base) { struct SolidSyslogPosixFile* self = PosixFile_SelfFromBase(base); - if (self->Fd != INVALID_FD) { close(self->Fd); + self->Fd = INVALID_FD; } - - *self = DESTROYED_INSTANCE; + /* Overwrite the abstract base with the shared NullFile vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullFile_Get(); } static inline struct SolidSyslogPosixFile* PosixFile_SelfFromBase(struct SolidSyslogFile* base) @@ -101,7 +76,6 @@ static bool PosixFile_Open(struct SolidSyslogFile* base, const char* path) static void PosixFile_Close(struct SolidSyslogFile* base) { struct SolidSyslogPosixFile* self = PosixFile_SelfFromBase(base); - if (self->Fd != INVALID_FD) { close(self->Fd); diff --git a/Platform/Posix/Source/SolidSyslogPosixFilePrivate.h b/Platform/Posix/Source/SolidSyslogPosixFilePrivate.h new file mode 100644 index 00000000..2ed3b584 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixFilePrivate.h @@ -0,0 +1,15 @@ +#ifndef SOLIDSYSLOGPOSIXFILEPRIVATE_H +#define SOLIDSYSLOGPOSIXFILEPRIVATE_H + +#include "SolidSyslogFileDefinition.h" + +struct SolidSyslogPosixFile +{ + struct SolidSyslogFile Base; + int Fd; +}; + +void PosixFile_Initialise(struct SolidSyslogFile* base); +void PosixFile_Cleanup(struct SolidSyslogFile* base); + +#endif /* SOLIDSYSLOGPOSIXFILEPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixFileStatic.c b/Platform/Posix/Source/SolidSyslogPosixFileStatic.c new file mode 100644 index 00000000..f736e679 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixFileStatic.c @@ -0,0 +1,66 @@ +#include "SolidSyslogPosixFile.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullFile.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPosixFilePrivate.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +static size_t PosixFile_IndexFromHandle(const struct SolidSyslogFile* base); +static void PosixFile_CleanupAtIndex(size_t index, void* context); + +static bool PosixFile_InUse[SOLIDSYSLOG_POSIX_FILE_POOL_SIZE]; +static struct SolidSyslogPosixFile PosixFile_Pool[SOLIDSYSLOG_POSIX_FILE_POOL_SIZE]; +static struct SolidSyslogPoolAllocator PosixFile_Allocator = {PosixFile_InUse, SOLIDSYSLOG_POSIX_FILE_POOL_SIZE}; + +struct SolidSyslogFile* SolidSyslogPosixFile_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PosixFile_Allocator); + struct SolidSyslogFile* handle = SolidSyslogNullFile_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&PosixFile_Allocator, index)) + { + PosixFile_Initialise(&PosixFile_Pool[index].Base); + handle = &PosixFile_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_POSIXFILE_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogPosixFile_Destroy(struct SolidSyslogFile* base) +{ + size_t index = PosixFile_IndexFromHandle(base); + bool released = SolidSyslogPoolAllocator_IndexIsValid(&PosixFile_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&PosixFile_Allocator, index, PosixFile_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_POSIXFILE_UNKNOWN_DESTROY); + } +} + +static size_t PosixFile_IndexFromHandle(const struct SolidSyslogFile* base) +{ + size_t result = SOLIDSYSLOG_POSIX_FILE_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_POSIX_FILE_POOL_SIZE; poolIndex++) + { + if (base == &PosixFile_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void PosixFile_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + PosixFile_Cleanup(&PosixFile_Pool[index].Base); +} diff --git a/Tests/SolidSyslogBlockStorePosixTest.cpp b/Tests/SolidSyslogBlockStorePosixTest.cpp index 4b6a7607..ce08a458 100644 --- a/Tests/SolidSyslogBlockStorePosixTest.cpp +++ b/Tests/SolidSyslogBlockStorePosixTest.cpp @@ -37,7 +37,6 @@ TEST_GROUP(SolidSyslogBlockStorePosix) static const size_t RECORD_OVERHEAD = 5; /* 2 (magic) + 2 (length) + 1 (sent flag) */ static const size_t ONE_MAX_MSG_RECORD = SOLIDSYSLOG_MAX_MESSAGE_SIZE + RECORD_OVERHEAD; - SolidSyslogPosixFileStorage fileStorage = {}; struct SolidSyslogFile* file = nullptr; struct SolidSyslogBlockDevice* device = nullptr; struct SolidSyslogStore* store = nullptr; @@ -46,7 +45,7 @@ TEST_GROUP(SolidSyslogBlockStorePosix) void setup() override { CleanStoreFiles(); - file = SolidSyslogPosixFile_Create(&fileStorage); + file = SolidSyslogPosixFile_Create(); device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); std::memset(maxMsg, 'A', sizeof(maxMsg)); } diff --git a/Tests/SolidSyslogPosixFileTest.cpp b/Tests/SolidSyslogPosixFileTest.cpp index 46703ba4..59cc1156 100644 --- a/Tests/SolidSyslogPosixFileTest.cpp +++ b/Tests/SolidSyslogPosixFileTest.cpp @@ -1,16 +1,39 @@ +#include "ConfigLockFake.h" #include "CppUTest/TestHarness.h" +#include "ErrorHandlerFake.h" +#include "SocketFake.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogFile.h" +#include "SolidSyslogFileDefinition.h" #include "SolidSyslogPosixFile.h" -#include "SocketFake.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" +#include "TestUtils.h" #include +using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings NEVER/ONCE/TWICE/THRICE into scope for the CALLED_* + // macros + static const char* const TEST_PATH = "/tmp/test_posix_file.dat"; +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macros preserve __FILE__/__LINE__ at the call site +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) + // clang-format off TEST_GROUP(SolidSyslogPosixFile) { - SolidSyslogPosixFileStorage storage = {}; struct SolidSyslogFile* file = nullptr; void setup() override @@ -18,7 +41,7 @@ TEST_GROUP(SolidSyslogPosixFile) SocketFake_Reset(); remove(TEST_PATH); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - file = SolidSyslogPosixFile_Create(&storage); + file = SolidSyslogPosixFile_Create(); } void teardown() override @@ -40,14 +63,6 @@ TEST(SolidSyslogPosixFile, CreateReturnsNonNull) CHECK_TRUE(file != nullptr); } -TEST(SolidSyslogPosixFile, CreateReturnsHandleInsideCallerSuppliedStorage) -{ - SolidSyslogPosixFileStorage localStorage{}; - struct SolidSyslogFile* localFile = SolidSyslogPosixFile_Create(&localStorage); - POINTERS_EQUAL(&localStorage, localFile); - SolidSyslogPosixFile_Destroy(localFile); -} - TEST(SolidSyslogPosixFile, IsOpenReturnsFalseBeforeOpen) { CHECK_FALSE(SolidSyslogFile_IsOpen(file)); @@ -115,3 +130,136 @@ TEST(SolidSyslogPosixFile, DeleteReturnsFalseForNonexistentFile) { CHECK_FALSE(SolidSyslogFile_Delete(file, "/tmp/nonexistent_posix_file.dat")); } + +// clang-format off +TEST_GROUP(SolidSyslogPosixFilePool) +{ + struct SolidSyslogFile* pooled[SOLIDSYSLOG_POSIX_FILE_POOL_SIZE] = {}; + struct SolidSyslogFile* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogPosixFile_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogPosixFile_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogPosixFile_Create(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogPosixFilePool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogPosixFile_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogPosixFilePool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogPosixFile_Create(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXFILE_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixFilePool, FallbackOpenReturnsFalse) +{ + FillPool(); + overflow = SolidSyslogPosixFile_Create(); + + CHECK_FALSE(SolidSyslogFile_Open(overflow, TEST_PATH)); +} + +TEST(SolidSyslogPosixFilePool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogPosixFile_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixFilePool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogPosixFile_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_POSIX_FILE_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_POSIX_FILE_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogPosixFilePool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogPosixFile_Create(); + ConfigLockFake_Install(); + + SolidSyslogPosixFile_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixFilePool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogFile stranger = {}; + + SolidSyslogPosixFile_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogPosixFilePool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogFile stranger = {}; + + SolidSyslogPosixFile_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXFILE_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixFilePool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogPosixFile_Create(); + SolidSyslogPosixFile_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogPosixFile_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXFILE_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} From 9345fbf81fefab837331a47f8c9212eac848cd99 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 22:04:44 +0100 Subject: [PATCH 10/15] refactor: S11.06 migrate PosixTcpStream onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogPosixTcpStream.c — vtable + Initialise/Cleanup; no allocation. Static DEFAULT_INSTANCE / DESTROYED_INSTANCE templates dropped. - SolidSyslogPosixTcpStreamPrivate.h — concrete struct + private signatures, TU-internal. - SolidSyslogPosixTcpStreamStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex. Public API change: Before: SolidSyslogPosixTcpStream_Create(SolidSyslogPosixTcpStreamStorage*) After: SolidSyslogPosixTcpStream_Create(void) Public Storage type and SOLIDSYSLOG_POSIX_TCP_STREAM_SIZE enum deleted. Pool size tunable: SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE, default 2U (not 1U). The Linux BDD target wires two stream-framed senders behind a SwitchingSender — plain TCP plus TLS that wraps another underlying PosixTcpStream as its transport — so default 1 would silently fall back to NullStream on the second Create. Matches the SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE default bumped to 2 in S11.05 for the same reason. Pool exhaustion returns the shared SolidSyslogNullStream with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown / stale handle reports WARNING. Cleanup overwrites the abstract base with NullStream's vtable for the safe-no-op post-destroy contract. The existing PosixTcpStream connect semantics — non-blocking start, select-bounded wait via CONNECT_TIMEOUT_MICROSECONDS, deferred SO_ERROR read, plus TCP_NODELAY + keepalive + TCP_USER_TIMEOUT — are preserved verbatim. The behavior of the Open() retry loop after a failed Connect (close-on-failure, INVALID_FD reset) is identical to the pre-migration code; only the Create/Destroy lifecycle moves into the pool. Callers updated: - Tests/SolidSyslogPosixTcpStreamTest.cpp — drop storage local, plus remove CreateReturnsHandleInsideCallerSuppliedStorage. - Tests/SolidSyslogStreamSenderTest.cpp — drop streamStorage locals in five TEST_GROUPs. - Bdd/Targets/Linux/main.c — drop plainTcpStreamStorage static. - Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c — drop underlyingStreamStorage static. New 9-test SolidSyslogPosixTcpStreamPool group covers the pool contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../BddTargetTlsSender_OpenSsl_PosixTcp.c | 3 +- Bdd/Targets/Linux/main.c | 3 +- CLAUDE.md | 1 + Core/Interface/SolidSyslogTunablesDefaults.h | 23 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../Interface/SolidSyslogPosixTcpStream.h | 16 +- .../Posix/Source/SolidSyslogPosixTcpStream.c | 55 ++---- .../Source/SolidSyslogPosixTcpStreamPrivate.h | 15 ++ .../Source/SolidSyslogPosixTcpStreamStatic.c | 70 ++++++++ Tests/SolidSyslogPosixTcpStreamTest.cpp | 164 ++++++++++++++++-- Tests/SolidSyslogStreamSenderTest.cpp | 15 +- 12 files changed, 295 insertions(+), 75 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogPosixTcpStreamPrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c diff --git a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c index 648b1768..ab836e2e 100644 --- a/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c +++ b/Bdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.c @@ -10,7 +10,6 @@ struct SolidSyslogResolver; -static SolidSyslogPosixTcpStreamStorage underlyingStreamStorage; static struct SolidSyslogStream* underlyingStream; static SolidSyslogTlsStreamStorage tlsStreamStorage; static struct SolidSyslogStream* tlsStream; @@ -18,7 +17,7 @@ static struct SolidSyslogSender* sender; struct SolidSyslogSender* BddTargetTlsSender_Create(struct SolidSyslogResolver* resolver, bool mtls) { - underlyingStream = SolidSyslogPosixTcpStream_Create(&underlyingStreamStorage); + underlyingStream = SolidSyslogPosixTcpStream_Create(); static struct SolidSyslogTlsStreamConfig tlsStreamConfig; tlsStreamConfig = (struct SolidSyslogTlsStreamConfig) {0}; diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index b2f682a5..4c01add4 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -51,7 +51,6 @@ static const char* const STORE_PATH_PREFIX = "/tmp/STORE"; static const char* const THRESHOLD_MARKER_PATH = "/tmp/solidsyslog_threshold_marker.log"; static struct SolidSyslogFile* storeFile; static struct SolidSyslogBlockDevice* storeBlockDevice; -static SolidSyslogPosixTcpStreamStorage plainTcpStreamStorage; static struct SolidSyslogStream* plainTcpStream; static struct SolidSyslogSender* plainTcpSender; static struct SolidSyslogSender* udpSender; @@ -90,7 +89,7 @@ static struct SolidSyslogSender* CreateSender(const struct BddTargetOptions* opt udpConfig.EndpointVersion = BddTargetUdpConfig_GetEndpointVersion; udpSender = SolidSyslogUdpSender_Create(&udpConfig); - plainTcpStream = SolidSyslogPosixTcpStream_Create(&plainTcpStreamStorage); + plainTcpStream = SolidSyslogPosixTcpStream_Create(); static struct SolidSyslogStreamSenderConfig tcpConfig = {0}; tcpConfig.Resolver = resolver; tcpConfig.Stream = plainTcpStream; diff --git a/CLAUDE.md b/CLAUDE.md index 6208b1cb..87d15bbf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -336,6 +336,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullStream.h` | Any code installing a no-op stream slot (Open/Close are no-ops, Send returns `true` to drop on the floor so the Store does not fill with undeliverables, Read returns `0` for would-block so the caller does not tear the connection down) | `SolidSyslogNullStream_Get` | | `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`. | +| `SolidSyslogPosixTcpStream.h` | System setup code on POSIX targets using non-blocking TCP with bounded connect/keepalive | `SolidSyslogPosixTcpStream_Create(void)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); default pool size 2 to support the BDD-target plain-TCP + TLS-underlying-TCP pair. Pool-exhaustion fallback is the shared `SolidSyslogNullStream`. | | `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` | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index d520f2ad..819ca943 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -132,6 +132,29 @@ #error "SOLIDSYSLOG_POSIX_FILE_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogPosixTcpStream instances the library's internal + * static pool can simultaneously hold. Each instance carries an + * int file descriptor. + * + * Default 2 — the Linux BDD target wires *two* stream-framed senders + * behind a SwitchingSender (plain TCP, plus TLS that wraps another + * underlying PosixTcpStream as its transport), so default 1 would + * silently fall back to NullStream on the second Create. Matches the + * SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE default for the same reason. + * Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if more are needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE 2U +#endif + +#if SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE < 1 +#error "SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index a7f5341e..1ab7b65e 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -79,5 +79,9 @@ "SolidSyslogPosixFile_Create pool exhausted; returning fallback file" #define SOLIDSYSLOG_ERROR_MSG_POSIXFILE_UNKNOWN_DESTROY \ "SolidSyslogPosixFile_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_POOL_EXHAUSTED \ + "SolidSyslogPosixTcpStream_Create pool exhausted; returning fallback stream" +#define SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_UNKNOWN_DESTROY \ + "SolidSyslogPosixTcpStream_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index ce0f563a..bffd6e6f 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -13,6 +13,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogPosixDatagram.c Source/SolidSyslogPosixDatagramStatic.c Source/SolidSyslogPosixTcpStream.c + Source/SolidSyslogPosixTcpStreamStatic.c Source/SolidSyslogPosixFile.c Source/SolidSyslogPosixFileStatic.c ) diff --git a/Platform/Posix/Interface/SolidSyslogPosixTcpStream.h b/Platform/Posix/Interface/SolidSyslogPosixTcpStream.h index 6e9b3697..f1983a1c 100644 --- a/Platform/Posix/Interface/SolidSyslogPosixTcpStream.h +++ b/Platform/Posix/Interface/SolidSyslogPosixTcpStream.h @@ -1,25 +1,13 @@ #ifndef SOLIDSYSLOGPOSIXTCPSTREAM_H #define SOLIDSYSLOGPOSIXTCPSTREAM_H -#include - #include "ExternC.h" -struct SolidSyslogStream; - EXTERN_C_BEGIN - enum - { - SOLIDSYSLOG_POSIX_TCP_STREAM_SIZE = sizeof(intptr_t) * 5U - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_POSIX_TCP_STREAM_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogPosixTcpStreamStorage; + struct SolidSyslogStream; - struct SolidSyslogStream* SolidSyslogPosixTcpStream_Create(SolidSyslogPosixTcpStreamStorage * storage); + struct SolidSyslogStream* SolidSyslogPosixTcpStream_Create(void); void SolidSyslogPosixTcpStream_Destroy(struct SolidSyslogStream * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogPosixTcpStream.c b/Platform/Posix/Source/SolidSyslogPosixTcpStream.c index 4aec7319..8bd40368 100644 --- a/Platform/Posix/Source/SolidSyslogPosixTcpStream.c +++ b/Platform/Posix/Source/SolidSyslogPosixTcpStream.c @@ -13,7 +13,8 @@ #include #include "SolidSyslogAddressInternal.h" -#include "SolidSyslogMacros.h" +#include "SolidSyslogNullStream.h" +#include "SolidSyslogPosixTcpStreamPrivate.h" #include "SolidSyslogStream.h" #include "SolidSyslogStreamDefinition.h" @@ -36,19 +37,11 @@ enum USER_TIMEOUT_MILLISECONDS = 30000 }; -struct SolidSyslogPosixTcpStream -{ - struct SolidSyslogStream Base; - int Fd; -}; - static bool PosixTcpStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr); static bool PosixTcpStream_Send(struct SolidSyslogStream* base, const void* buffer, size_t size); static SolidSyslogSsize PosixTcpStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size); static void PosixTcpStream_Close(struct SolidSyslogStream* base); -static inline struct SolidSyslogPosixTcpStream* PosixTcpStream_SelfFromStorage(SolidSyslogPosixTcpStreamStorage* storage -); static inline struct SolidSyslogPosixTcpStream* PosixTcpStream_SelfFromBase(struct SolidSyslogStream* base); static int PosixTcpStream_OpenAndConfigureSocket(void); @@ -67,39 +60,27 @@ static bool PosixTcpStream_ReadDeferredConnectError(int fd); static bool PosixTcpStream_WroteAllBytes(ssize_t sent, size_t expected); static inline bool PosixTcpStream_WouldBlock(void); -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogPosixTcpStream) <= SOLIDSYSLOG_POSIX_TCP_STREAM_SIZE, - "SOLIDSYSLOG_POSIX_TCP_STREAM_SIZE is too small for struct SolidSyslogPosixTcpStream" -); - -static const struct SolidSyslogPosixTcpStream DEFAULT_INSTANCE = { - {PosixTcpStream_Open, PosixTcpStream_Send, PosixTcpStream_Read, PosixTcpStream_Close}, - INVALID_FD, -}; - -static const struct SolidSyslogPosixTcpStream DESTROYED_INSTANCE = { - {NULL, NULL, NULL, NULL}, - INVALID_FD, -}; - -struct SolidSyslogStream* SolidSyslogPosixTcpStream_Create(SolidSyslogPosixTcpStreamStorage* storage) +void PosixTcpStream_Initialise(struct SolidSyslogStream* base) { - struct SolidSyslogPosixTcpStream* self = PosixTcpStream_SelfFromStorage(storage); - *self = DEFAULT_INSTANCE; - return &self->Base; -} - -static inline struct SolidSyslogPosixTcpStream* PosixTcpStream_SelfFromStorage(SolidSyslogPosixTcpStreamStorage* storage -) -{ - return (struct SolidSyslogPosixTcpStream*) storage; + struct SolidSyslogPosixTcpStream* self = PosixTcpStream_SelfFromBase(base); + self->Base.Open = PosixTcpStream_Open; + self->Base.Send = PosixTcpStream_Send; + self->Base.Read = PosixTcpStream_Read; + self->Base.Close = PosixTcpStream_Close; + self->Fd = INVALID_FD; } -void SolidSyslogPosixTcpStream_Destroy(struct SolidSyslogStream* base) +void PosixTcpStream_Cleanup(struct SolidSyslogStream* base) { struct SolidSyslogPosixTcpStream* self = PosixTcpStream_SelfFromBase(base); - PosixTcpStream_Close(base); - *self = DESTROYED_INSTANCE; + if (PosixTcpStream_IsFileDescriptorValid(self->Fd)) + { + close(self->Fd); + self->Fd = INVALID_FD; + } + /* Overwrite the abstract base with the shared NullStream vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullStream_Get(); } static inline struct SolidSyslogPosixTcpStream* PosixTcpStream_SelfFromBase(struct SolidSyslogStream* base) diff --git a/Platform/Posix/Source/SolidSyslogPosixTcpStreamPrivate.h b/Platform/Posix/Source/SolidSyslogPosixTcpStreamPrivate.h new file mode 100644 index 00000000..3b37a165 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixTcpStreamPrivate.h @@ -0,0 +1,15 @@ +#ifndef SOLIDSYSLOGPOSIXTCPSTREAMPRIVATE_H +#define SOLIDSYSLOGPOSIXTCPSTREAMPRIVATE_H + +#include "SolidSyslogStreamDefinition.h" + +struct SolidSyslogPosixTcpStream +{ + struct SolidSyslogStream Base; + int Fd; +}; + +void PosixTcpStream_Initialise(struct SolidSyslogStream* base); +void PosixTcpStream_Cleanup(struct SolidSyslogStream* base); + +#endif /* SOLIDSYSLOGPOSIXTCPSTREAMPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c b/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c new file mode 100644 index 00000000..896243fa --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c @@ -0,0 +1,70 @@ +#include "SolidSyslogPosixTcpStream.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullStream.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPosixTcpStreamPrivate.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +static size_t PosixTcpStream_IndexFromHandle(const struct SolidSyslogStream* base); +static void PosixTcpStream_CleanupAtIndex(size_t index, void* context); + +static bool PosixTcpStream_InUse[SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE]; +static struct SolidSyslogPosixTcpStream PosixTcpStream_Pool[SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE]; +static struct SolidSyslogPoolAllocator PosixTcpStream_Allocator = { + PosixTcpStream_InUse, + SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE +}; + +struct SolidSyslogStream* SolidSyslogPosixTcpStream_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PosixTcpStream_Allocator); + struct SolidSyslogStream* handle = SolidSyslogNullStream_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&PosixTcpStream_Allocator, index)) + { + PosixTcpStream_Initialise(&PosixTcpStream_Pool[index].Base); + handle = &PosixTcpStream_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogPosixTcpStream_Destroy(struct SolidSyslogStream* base) +{ + size_t index = PosixTcpStream_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&PosixTcpStream_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&PosixTcpStream_Allocator, index, PosixTcpStream_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_UNKNOWN_DESTROY); + } +} + +static size_t PosixTcpStream_IndexFromHandle(const struct SolidSyslogStream* base) +{ + size_t result = SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE; poolIndex++) + { + if (base == &PosixTcpStream_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void PosixTcpStream_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + PosixTcpStream_Cleanup(&PosixTcpStream_Pool[index].Base); +} diff --git a/Tests/SolidSyslogPosixTcpStreamTest.cpp b/Tests/SolidSyslogPosixTcpStreamTest.cpp index e234658d..c061d473 100644 --- a/Tests/SolidSyslogPosixTcpStreamTest.cpp +++ b/Tests/SolidSyslogPosixTcpStreamTest.cpp @@ -11,10 +11,16 @@ using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings NEVER/ONCE/TWICE/THRICE into scope for the CALLED_* // macros +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" #include "SolidSyslogAddress.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogPosixTcpStream.h" +#include "SolidSyslogPrival.h" #include "SolidSyslogStream.h" +#include "SolidSyslogStreamDefinition.h" #include "SolidSyslogTransport.h" +#include "SolidSyslogTunables.h" #include "SocketFake.h" // clang-format off @@ -25,7 +31,6 @@ static const int TEST_PORT = 514; TEST_GROUP(SolidSyslogPosixTcpStream) { - SolidSyslogPosixTcpStreamStorage streamStorage{}; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros struct SolidSyslogStream* stream = nullptr; SolidSyslogAddressStorage addrStorage{}; @@ -36,7 +41,7 @@ TEST_GROUP(SolidSyslogPosixTcpStream) { SocketFake_Reset(); // cppcheck-suppress unreadVariable -- used in tests; cppcheck does not model CppUTest macros - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) -- char-type aliasing, legal and necessary auto* bytes = reinterpret_cast(&addrStorage); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) -- reinterpret to platform layout, storage is intptr_t-aligned @@ -76,14 +81,6 @@ TEST(SolidSyslogPosixTcpStream, CreateDestroyWorksWithoutCrashing) { } -TEST(SolidSyslogPosixTcpStream, CreateReturnsHandleInsideCallerSuppliedStorage) -{ - SolidSyslogPosixTcpStreamStorage storage{}; - struct SolidSyslogStream* localStream = SolidSyslogPosixTcpStream_Create(&storage); - POINTERS_EQUAL(&storage, localStream); - SolidSyslogPosixTcpStream_Destroy(localStream); -} - TEST(SolidSyslogPosixTcpStream, OpenCallsSocketOnce) { SolidSyslogStream_Open(stream, addr); @@ -513,3 +510,150 @@ TEST(SolidSyslogPosixTcpStream, ReadReturnsNegativeOneOnErrorAndClosesSocket) LONGS_EQUAL(-1, Read16ByteBuffer()); CHECK_SOCKET_CLOSED_ONCE(); } + +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) + +// clang-format off +TEST_GROUP(SolidSyslogPosixTcpStreamPool) +{ + struct SolidSyslogStream* pooled[SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE] = {}; + struct SolidSyslogStream* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogPosixTcpStream_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogPosixTcpStream_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogPosixTcpStream_Create(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogPosixTcpStreamPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogPosixTcpStream_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogPosixTcpStreamPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogPosixTcpStream_Create(); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixTcpStreamPool, FallbackSendReturnsTrue) +{ + FillPool(); + overflow = SolidSyslogPosixTcpStream_Create(); + + CHECK_TRUE(SolidSyslogStream_Send(overflow, "x", 1)); +} + +TEST(SolidSyslogPosixTcpStreamPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogPosixTcpStream_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixTcpStreamPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogPosixTcpStream_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogPosixTcpStreamPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogPosixTcpStream_Create(); + ConfigLockFake_Install(); + + SolidSyslogPosixTcpStream_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixTcpStreamPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogStream stranger = {}; + + SolidSyslogPosixTcpStream_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogPosixTcpStreamPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogStream stranger = {}; + + SolidSyslogPosixTcpStream_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixTcpStreamPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogPosixTcpStream_Create(); + SolidSyslogPosixTcpStream_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogPosixTcpStream_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} diff --git a/Tests/SolidSyslogStreamSenderTest.cpp b/Tests/SolidSyslogStreamSenderTest.cpp index 46841835..57feb9ad 100644 --- a/Tests/SolidSyslogStreamSenderTest.cpp +++ b/Tests/SolidSyslogStreamSenderTest.cpp @@ -87,7 +87,6 @@ static uint32_t TestEndpointVersion() // NOLINT(modernize-use-trailing-return-ty TEST_GROUP(SolidSyslogStreamSender) { struct SolidSyslogResolver* resolver = nullptr; - SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion @@ -101,7 +100,7 @@ TEST_GROUP(SolidSyslogStreamSender) endpointVersion = 0; endpointGetPort = GetPort; resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle sender = SolidSyslogStreamSender_Create(&config); @@ -150,7 +149,6 @@ TEST(SolidSyslogStreamSender, FirstSendSetsTcpNoDelay) TEST_GROUP(SolidSyslogStreamSenderDestroy) { struct SolidSyslogResolver* resolver = nullptr; - SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; @@ -161,7 +159,7 @@ TEST_GROUP(SolidSyslogStreamSenderDestroy) endpointVersion = 0; endpointGetPort = GetPort; resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); // cppcheck-suppress unreadVariable -- used in test bodies; cppcheck does not model CppUTest macros config = {resolver, stream, TestEndpoint, TestEndpointVersion}; } @@ -337,7 +335,6 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) const char* (*getHostFn)(void) = GetHost; // NOLINT(modernize-redundant-void-arg) -- C idiom // cppcheck-suppress unreadVariable -- assigned in CreateSender; cppcheck does not model CppUTest macros int (*getPortFn)(void) = GetPort; // NOLINT(modernize-redundant-void-arg) -- C idiom - SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogResolver* resolver = nullptr; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion @@ -366,7 +363,7 @@ TEST_GROUP(SolidSyslogStreamSenderConfig) endpointGetHost = getHostFn; endpointGetPort = getPortFn; resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); struct SolidSyslogStreamSenderConfig config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle sender = SolidSyslogStreamSender_Create(&config); @@ -444,7 +441,6 @@ TEST(SolidSyslogStreamSenderConfig, GetAddrInfoCalledWithHostname) TEST_GROUP(SolidSyslogStreamSenderFailure) { struct SolidSyslogResolver* resolver = nullptr; - SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; // cppcheck-suppress constVariablePointer -- Send requires non-const self; false positive from macro expansion @@ -458,7 +454,7 @@ TEST_GROUP(SolidSyslogStreamSenderFailure) endpointVersion = 0; endpointGetPort = GetPort; resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); config = {resolver, stream, TestEndpoint, TestEndpointVersion}; // cppcheck-suppress unreadVariable -- read by teardown and tests; cppcheck does not model CppUTest lifecycle sender = SolidSyslogStreamSender_Create(&config); @@ -648,7 +644,6 @@ TEST(SolidSyslogStreamSenderFailure, NoEndpointConfiguredConnectsToPortZero) TEST_GROUP(SolidSyslogStreamSenderPool) { struct SolidSyslogResolver* resolver = nullptr; - SolidSyslogPosixTcpStreamStorage streamStorage{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogStreamSenderConfig config; struct SolidSyslogSender* pooled[SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE] = {}; @@ -661,7 +656,7 @@ TEST_GROUP(SolidSyslogStreamSenderPool) endpointVersion = 0; endpointGetPort = GetPort; resolver = SolidSyslogGetAddrInfoResolver_Create(); - stream = SolidSyslogPosixTcpStream_Create(&streamStorage); + stream = SolidSyslogPosixTcpStream_Create(); // cppcheck-suppress unreadVariable -- read by MakeSender; cppcheck does not model CppUTest macros config = {resolver, stream, TestEndpoint, TestEndpointVersion}; } From 11d399615aeea9df3654c3ea6f18d11db5582e50 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 22:09:14 +0100 Subject: [PATCH 11/15] refactor: S11.06 migrate PosixMessageQueueBuffer onto PoolAllocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the canonical 3-TU split + SolidSyslogPoolAllocator shape: - SolidSyslogPosixMessageQueueBuffer.c — vtable + Initialise/Cleanup; no allocation. File-scope `instance` and QueueName helper now per-slot. - SolidSyslogPosixMessageQueueBufferPrivate.h — concrete struct + private signatures, TU-internal. Carries the per-instance mq_t, Formatter name storage, and MaxMessageSize. - SolidSyslogPosixMessageQueueBufferStatic.c — library-internal pool, Create/Destroy, IndexFromHandle, CleanupAtIndex. Public API change: Before: void SolidSyslogPosixMessageQueueBuffer_Destroy(void) After: void SolidSyslogPosixMessageQueueBuffer_Destroy(struct SolidSyslogBuffer*) `_Create(maxMessageSize, maxMessages)` keeps the positional pair unchanged. Pool size tunable: SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE, default 1U. The queue name derives from the process ID, so two slots in the same process would race onto the same `/solidsyslog_` name — an integrator needing N > 1 must additionally distinguish the names. The tunable header comment + the audience-table row + a Static.c comment all flag this limit; the contract today is one MQ buffer per process. Pool exhaustion returns the shared SolidSyslogNullBuffer with an ERROR-severity SolidSyslog_Error report. Destroy of an unknown / stale handle reports WARNING. Cleanup overwrites the abstract base with NullBuffer's vtable for safe-no-op post-destroy. Callers updated: - Tests/SolidSyslogPosixMessageQueueBufferTest.cpp — handle-based teardown. - Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp — handle-based teardown. - Bdd/Targets/Linux/main.c — handle-based teardown. New 9-test SolidSyslogPosixMessageQueueBufferPool group covers the pool contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- Bdd/Targets/Linux/main.c | 2 +- CLAUDE.md | 2 +- Core/Interface/SolidSyslogTunablesDefaults.h | 22 +++ Core/Source/SolidSyslogErrorMessages.h | 4 + Platform/Posix/CMakeLists.txt | 1 + .../SolidSyslogPosixMessageQueueBuffer.h | 4 +- .../SolidSyslogPosixMessageQueueBuffer.c | 56 +++---- ...olidSyslogPosixMessageQueueBufferPrivate.h | 27 +++ ...SolidSyslogPosixMessageQueueBufferStatic.c | 87 ++++++++++ .../Targets/BddTargetServiceThreadTest.cpp | 2 +- ...SolidSyslogPosixMessageQueueBufferTest.cpp | 158 +++++++++++++++++- 11 files changed, 327 insertions(+), 38 deletions(-) create mode 100644 Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h create mode 100644 Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 4c01add4..360ed3a1 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -291,7 +291,7 @@ int main(int argc, char* argv[]) SolidSyslogMetaSd_Destroy(metaSd); SolidSyslogStdAtomicCounter_Destroy(counter); DestroyStore(store, &options); - SolidSyslogPosixMessageQueueBuffer_Destroy(); + SolidSyslogPosixMessageQueueBuffer_Destroy(buffer); DestroySender(); return 0; diff --git a/CLAUDE.md b/CLAUDE.md index 87d15bbf..f0cb87c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -343,7 +343,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `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. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | -| `SolidSyslogPosixMessageQueueBuffer.h` | System setup code using POSIX message queue buffer | `SolidSyslogPosixMessageQueueBuffer_Create`, `_Destroy` | +| `SolidSyslogPosixMessageQueueBuffer.h` | System setup code using POSIX message queue buffer | `SolidSyslogPosixMessageQueueBuffer_Create(maxMessageSize, maxMessages)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11). The queue name derives from the process ID, so the default pool size of 1 is the supported configuration in a single process. Pool-exhaustion fallback is 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`) | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 819ca943..734675aa 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -155,6 +155,28 @@ #error "SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogPosixMessageQueueBuffer instances the library's + * internal static pool can simultaneously hold. Each instance carries + * an mqd_t plus the per-process queue name (Formatter storage). + * + * Default 1 — almost all integrators wire a single MQ-backed buffer. + * The queue name derives from the process ID, so bumping above 1 in + * the same process would race multiple slots onto the same + * `/solidsyslog_` name; an integrator needing N > 1 must + * additionally distinguish the names (out of scope today). + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE < 1 +#error "SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPassthroughBuffer instances the library's * internal static pool can simultaneously hold. Each instance is diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 1ab7b65e..b829e1f9 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -83,5 +83,9 @@ "SolidSyslogPosixTcpStream_Create pool exhausted; returning fallback stream" #define SOLIDSYSLOG_ERROR_MSG_POSIXTCPSTREAM_UNKNOWN_DESTROY \ "SolidSyslogPosixTcpStream_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_POOL_EXHAUSTED \ + "SolidSyslogPosixMessageQueueBuffer_Create pool exhausted; returning fallback buffer" +#define SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY \ + "SolidSyslogPosixMessageQueueBuffer_Destroy called with a handle not issued by this pool" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/Platform/Posix/CMakeLists.txt b/Platform/Posix/CMakeLists.txt index bffd6e6f..5e673d3f 100644 --- a/Platform/Posix/CMakeLists.txt +++ b/Platform/Posix/CMakeLists.txt @@ -2,6 +2,7 @@ target_sources(${PROJECT_NAME} PRIVATE Source/SolidSyslogPosixClock.c Source/SolidSyslogPosixHostname.c Source/SolidSyslogPosixMessageQueueBuffer.c + Source/SolidSyslogPosixMessageQueueBufferStatic.c Source/SolidSyslogPosixMutex.c Source/SolidSyslogPosixMutexStatic.c Source/SolidSyslogPosixProcessId.c diff --git a/Platform/Posix/Interface/SolidSyslogPosixMessageQueueBuffer.h b/Platform/Posix/Interface/SolidSyslogPosixMessageQueueBuffer.h index a513f116..a8fafa98 100644 --- a/Platform/Posix/Interface/SolidSyslogPosixMessageQueueBuffer.h +++ b/Platform/Posix/Interface/SolidSyslogPosixMessageQueueBuffer.h @@ -7,8 +7,10 @@ EXTERN_C_BEGIN + struct SolidSyslogBuffer; + struct SolidSyslogBuffer* SolidSyslogPosixMessageQueueBuffer_Create(size_t maxMessageSize, long maxMessages); - void SolidSyslogPosixMessageQueueBuffer_Destroy(void); + void SolidSyslogPosixMessageQueueBuffer_Destroy(struct SolidSyslogBuffer * base); EXTERN_C_END diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c index 5711857b..bdf7de4a 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c @@ -1,17 +1,19 @@ #include "SolidSyslogPosixMessageQueueBuffer.h" -#include #include +#include #include +#include #include #include "SolidSyslogBufferDefinition.h" #include "SolidSyslogFormatter.h" +#include "SolidSyslogNullBuffer.h" +#include "SolidSyslogPosixMessageQueueBufferPrivate.h" #include "SolidSyslogPosixProcessId.h" enum { - MAX_NAME_SIZE = 64, /* 0600 in octal — owner read/write, equivalent to S_IRUSR | S_IWUSR. Hex form avoids MISRA 7.1. */ OWNER_READ_WRITE = 0x180U }; @@ -22,33 +24,16 @@ static void PosixMessageQueueBuffer_Write(struct SolidSyslogBuffer* base, const static inline struct SolidSyslogPosixMessageQueueBuffer* PosixMessageQueueBuffer_SelfFromBase( struct SolidSyslogBuffer* base ); +static inline const char* PosixMessageQueueBuffer_QueueName(struct SolidSyslogPosixMessageQueueBuffer* self); -struct SolidSyslogPosixMessageQueueBuffer -{ - struct SolidSyslogBuffer Base; - mqd_t Mq; - SolidSyslogFormatterStorage NameStorage[SOLIDSYSLOG_FORMATTER_STORAGE_SIZE(MAX_NAME_SIZE)]; - size_t MaxMessageSize; -}; - -static struct SolidSyslogPosixMessageQueueBuffer PosixMessageQueueBuffer_Instance; - -static inline const char* PosixMessageQueueBuffer_QueueName(void) -{ - return SolidSyslogFormatter_AsFormattedBuffer( - SolidSyslogFormatter_FromStorage(PosixMessageQueueBuffer_Instance.NameStorage) - ); -} - -// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- distinct semantic meaning; struct wrapper would over-engineer -struct SolidSyslogBuffer* SolidSyslogPosixMessageQueueBuffer_Create(size_t maxMessageSize, long maxMessages) +void PosixMessageQueueBuffer_Initialise(struct SolidSyslogBuffer* base, size_t maxMessageSize, long maxMessages) { static const char queueNamePrefix[] = "/solidsyslog_"; - PosixMessageQueueBuffer_Instance = (struct SolidSyslogPosixMessageQueueBuffer) {0}; + struct SolidSyslogPosixMessageQueueBuffer* self = PosixMessageQueueBuffer_SelfFromBase(base); struct SolidSyslogFormatter* name = - SolidSyslogFormatter_Create(PosixMessageQueueBuffer_Instance.NameStorage, MAX_NAME_SIZE); + SolidSyslogFormatter_Create(self->NameStorage, POSIX_MESSAGE_QUEUE_BUFFER_MAX_NAME_SIZE); SolidSyslogFormatter_BoundedString(name, queueNamePrefix, sizeof(queueNamePrefix) - 1U); SolidSyslogPosixProcessId_Get(name); @@ -56,20 +41,25 @@ struct SolidSyslogBuffer* SolidSyslogPosixMessageQueueBuffer_Create(size_t maxMe attr.mq_maxmsg = maxMessages; attr.mq_msgsize = (long) maxMessageSize; - PosixMessageQueueBuffer_Instance.Mq = - mq_open(PosixMessageQueueBuffer_QueueName(), O_CREAT | O_RDWR | O_NONBLOCK, OWNER_READ_WRITE, &attr); - PosixMessageQueueBuffer_Instance.MaxMessageSize = maxMessageSize; - PosixMessageQueueBuffer_Instance.Base.Write = PosixMessageQueueBuffer_Write; - PosixMessageQueueBuffer_Instance.Base.Read = PosixMessageQueueBuffer_Read; + self->Mq = mq_open(PosixMessageQueueBuffer_QueueName(self), O_CREAT | O_RDWR | O_NONBLOCK, OWNER_READ_WRITE, &attr); + self->MaxMessageSize = maxMessageSize; + self->Base.Write = PosixMessageQueueBuffer_Write; + self->Base.Read = PosixMessageQueueBuffer_Read; +} - return &PosixMessageQueueBuffer_Instance.Base; +void PosixMessageQueueBuffer_Cleanup(struct SolidSyslogBuffer* base) +{ + struct SolidSyslogPosixMessageQueueBuffer* self = PosixMessageQueueBuffer_SelfFromBase(base); + mq_close(self->Mq); + mq_unlink(PosixMessageQueueBuffer_QueueName(self)); + /* 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. */ + *base = *SolidSyslogNullBuffer_Get(); } -void SolidSyslogPosixMessageQueueBuffer_Destroy(void) +static inline const char* PosixMessageQueueBuffer_QueueName(struct SolidSyslogPosixMessageQueueBuffer* self) { - mq_close(PosixMessageQueueBuffer_Instance.Mq); - mq_unlink(PosixMessageQueueBuffer_QueueName()); - PosixMessageQueueBuffer_Instance = (struct SolidSyslogPosixMessageQueueBuffer) {0}; + return SolidSyslogFormatter_AsFormattedBuffer(SolidSyslogFormatter_FromStorage(self->NameStorage)); } static bool PosixMessageQueueBuffer_Read(struct SolidSyslogBuffer* base, void* data, size_t maxSize, size_t* bytesRead) diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h new file mode 100644 index 00000000..974b43c4 --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h @@ -0,0 +1,27 @@ +#ifndef SOLIDSYSLOGPOSIXMESSAGEQUEUEBUFFERPRIVATE_H +#define SOLIDSYSLOGPOSIXMESSAGEQUEUEBUFFERPRIVATE_H + +#include +#include + +#include "SolidSyslogBufferDefinition.h" +#include "SolidSyslogFormatter.h" + +enum +{ + POSIX_MESSAGE_QUEUE_BUFFER_MAX_NAME_SIZE = 64 +}; + +struct SolidSyslogPosixMessageQueueBuffer +{ + struct SolidSyslogBuffer Base; + mqd_t Mq; + SolidSyslogFormatterStorage + NameStorage[SOLIDSYSLOG_FORMATTER_STORAGE_SIZE(POSIX_MESSAGE_QUEUE_BUFFER_MAX_NAME_SIZE)]; + size_t MaxMessageSize; +}; + +void PosixMessageQueueBuffer_Initialise(struct SolidSyslogBuffer* base, size_t maxMessageSize, long maxMessages); +void PosixMessageQueueBuffer_Cleanup(struct SolidSyslogBuffer* base); + +#endif /* SOLIDSYSLOGPOSIXMESSAGEQUEUEBUFFERPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c new file mode 100644 index 00000000..0e1adb0f --- /dev/null +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c @@ -0,0 +1,87 @@ +#include "SolidSyslogPosixMessageQueueBuffer.h" + +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullBuffer.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPosixMessageQueueBufferPrivate.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +/* The shared queue name is derived from the process ID — fine for the + * default pool size of 1 instance per process. Bumping the tunable + * above 1 in the same process would race two slots onto the same + * `/solidsyslog_` queue; that scenario is outside today's contract. */ + +struct InitContext +{ + size_t MaxMessageSize; + long MaxMessages; +}; + +static size_t PosixMessageQueueBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); +static void PosixMessageQueueBuffer_CleanupAtIndex(size_t index, void* context); + +static bool PosixMessageQueueBuffer_InUse[SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE]; +static struct SolidSyslogPosixMessageQueueBuffer + PosixMessageQueueBuffer_Pool[SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE]; +static struct SolidSyslogPoolAllocator PosixMessageQueueBuffer_Allocator = { + PosixMessageQueueBuffer_InUse, + SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE +}; + +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- distinct semantic meaning; struct wrapper would over-engineer +struct SolidSyslogBuffer* SolidSyslogPosixMessageQueueBuffer_Create(size_t maxMessageSize, long maxMessages) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&PosixMessageQueueBuffer_Allocator); + struct SolidSyslogBuffer* handle = SolidSyslogNullBuffer_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&PosixMessageQueueBuffer_Allocator, index)) + { + PosixMessageQueueBuffer_Initialise(&PosixMessageQueueBuffer_Pool[index].Base, maxMessageSize, maxMessages); + handle = &PosixMessageQueueBuffer_Pool[index].Base; + } + else + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_POOL_EXHAUSTED); + } + return handle; +} + +void SolidSyslogPosixMessageQueueBuffer_Destroy(struct SolidSyslogBuffer* base) +{ + size_t index = PosixMessageQueueBuffer_IndexFromHandle(base); + bool released = SolidSyslogPoolAllocator_IndexIsValid(&PosixMessageQueueBuffer_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse( + &PosixMessageQueueBuffer_Allocator, + index, + PosixMessageQueueBuffer_CleanupAtIndex, + NULL + ); + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY); + } +} + +static size_t PosixMessageQueueBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base) +{ + size_t result = SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE; poolIndex++) + { + if (base == &PosixMessageQueueBuffer_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void PosixMessageQueueBuffer_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + PosixMessageQueueBuffer_Cleanup(&PosixMessageQueueBuffer_Pool[index].Base); +} diff --git a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp index 5dfd8e90..46459094 100644 --- a/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp +++ b/Tests/Bdd/Targets/BddTargetServiceThreadTest.cpp @@ -81,7 +81,7 @@ TEST_GROUP(BddTargetServiceThread) void teardown() override { SolidSyslog_Destroy(); - SolidSyslogPosixMessageQueueBuffer_Destroy(); + SolidSyslogPosixMessageQueueBuffer_Destroy(buffer); SolidSyslogUdpSender_Destroy(sender); SolidSyslogPosixDatagram_Destroy(datagram); SolidSyslogGetAddrInfoResolver_Destroy(resolver); diff --git a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp index e2484c12..823e6c93 100644 --- a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp +++ b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp @@ -5,7 +5,11 @@ using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings NEVER/ONCE/TWICE/THRICE into scope for the CALLED_* // macros +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" #include "SolidSyslogBuffer.h" +#include "SolidSyslogBufferDefinition.h" +#include "SolidSyslogErrorMessages.h" #include "SolidSyslogPosixMessageQueueBuffer.h" #include "SolidSyslog.h" #include "SolidSyslogConfig.h" @@ -36,7 +40,7 @@ TEST_GROUP(SolidSyslogPosixMessageQueueBuffer) void teardown() override { - SolidSyslogPosixMessageQueueBuffer_Destroy(); + SolidSyslogPosixMessageQueueBuffer_Destroy(buffer); } void Write() const @@ -131,3 +135,155 @@ IGNORE_TEST(SolidSyslogPosixMessageQueueBuffer, HappyPathOnly) // Blocking mode not yet implemented — see S4.5 or later // Read blocks waiting for a message (O_NONBLOCK removed) } + +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) + +// clang-format off +TEST_GROUP(SolidSyslogPosixMessageQueueBufferPool) +{ + struct SolidSyslogBuffer* pooled[SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE] = {}; + struct SolidSyslogBuffer* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogPosixMessageQueueBuffer_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogPosixMessageQueueBuffer_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + ErrorHandlerFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + } + } +}; + +// clang-format on + +TEST(SolidSyslogPosixMessageQueueBufferPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_POOL_EXHAUSTED, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, FallbackReadAndWriteAreNoOps) +{ + FillPool(); + overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + + SolidSyslogBuffer_Write(overflow, "hello", 5); + + char readBuffer[16] = {}; + size_t bytesRead = 99; + CHECK_FALSE(SolidSyslogBuffer_Read(overflow, readBuffer, sizeof(readBuffer), &bytesRead)); + LONGS_EQUAL(0, bytesRead); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + + LONGS_EQUAL(SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + ConfigLockFake_Install(); + + SolidSyslogPosixMessageQueueBuffer_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogBuffer stranger = {}; + + SolidSyslogPosixMessageQueueBuffer_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogBuffer stranger = {}; + + SolidSyslogPosixMessageQueueBuffer_Destroy(&stranger); + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} + +TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + SolidSyslogPosixMessageQueueBuffer_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogPosixMessageQueueBuffer_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); + STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); +} From 6957ca1e695a36fc22c1520befa3510b31238a77 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 22:34:48 +0100 Subject: [PATCH 12/15] chore: S11.06 local gate sweep cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes surfaced by running the local gates over the full migrated tree: - **iwyu**: add explicit `#include "SolidSyslogStream.h"` / `SolidSyslogDatagram.h` in the Null*.c files (transitively pulled in via the Definition.h today but iwyu wants explicit); add a forward `struct SolidSyslog;` declaration to each *Static.c (matches the established pattern from CircularBufferStatic.c); add explicit `` to the NullDatagramTest for `size_t`. - **cppcheck**: suppress two false positives per Pool TEST_GROUP fixture across all six new pool test groups — `constVariable` on the `pooled[]` array (cppcheck doesn't see test bodies assigning into the array) and `knownConditionTrueFalse` on the teardown's `overflow != nullptr` guard (same reason). Existing Pool test groups (CircularBuffer, StreamSender, etc.) pass without these suppressions; cppcheck's CTU analysis is order-sensitive and inconsistent — the new test files happen to be analysed in an order that surfaces it. - **tidy**: add a per-call NOLINT to PosixMessageQueueBuffer_Initialise for `bugprone-easily-swappable-parameters` (mirrors the existing NOLINT on the public `_Create` signature it carries forward). - **dead code**: drop an unused `struct InitContext` left over from an early draft of MessageQueueBufferStatic.c (caught by tidy's readability-identifier-naming). All gates green after these fixes: debug + clang-debug + sanitize + coverage + tidy + cppcheck + iwyu + format. Coverage 100% line on every new file. Co-Authored-By: Claude Opus 4.7 (1M context) --- Core/Source/SolidSyslogNullDatagram.c | 3 +++ Core/Source/SolidSyslogNullStream.c | 1 + .../SolidSyslogGetAddrInfoResolverStatic.c | 2 ++ .../Source/SolidSyslogPosixDatagramStatic.c | 2 ++ .../Posix/Source/SolidSyslogPosixFileStatic.c | 2 ++ .../SolidSyslogPosixMessageQueueBuffer.c | 1 + ...SolidSyslogPosixMessageQueueBufferStatic.c | 8 ++----- .../Source/SolidSyslogPosixMutexStatic.c | 2 ++ .../Source/SolidSyslogPosixTcpStreamStatic.c | 2 ++ Tests/SolidSyslogGetAddrInfoResolverTest.cpp | 2 ++ Tests/SolidSyslogNullDatagramTest.cpp | 2 ++ Tests/SolidSyslogPosixDatagramTest.cpp | 2 ++ Tests/SolidSyslogPosixFileTest.cpp | 2 ++ ...SolidSyslogPosixMessageQueueBufferTest.cpp | 23 ++++++++++++------- Tests/SolidSyslogPosixMutexTest.cpp | 2 ++ Tests/SolidSyslogPosixTcpStreamTest.cpp | 2 ++ 16 files changed, 44 insertions(+), 14 deletions(-) diff --git a/Core/Source/SolidSyslogNullDatagram.c b/Core/Source/SolidSyslogNullDatagram.c index 4e67a95b..19f69b6d 100644 --- a/Core/Source/SolidSyslogNullDatagram.c +++ b/Core/Source/SolidSyslogNullDatagram.c @@ -3,9 +3,12 @@ #include #include +#include "SolidSyslogDatagram.h" #include "SolidSyslogDatagramDefinition.h" #include "SolidSyslogUdpPayload.h" +struct SolidSyslogAddress; + static bool NullDatagram_Open(struct SolidSyslogDatagram* base); static enum SolidSyslogDatagramSendResult NullDatagram_SendTo( struct SolidSyslogDatagram* base, diff --git a/Core/Source/SolidSyslogNullStream.c b/Core/Source/SolidSyslogNullStream.c index dd910a4a..24f211c1 100644 --- a/Core/Source/SolidSyslogNullStream.c +++ b/Core/Source/SolidSyslogNullStream.c @@ -3,6 +3,7 @@ #include #include +#include "SolidSyslogStream.h" #include "SolidSyslogStreamDefinition.h" struct SolidSyslogAddress; diff --git a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c index 08df0788..bcc8a73f 100644 --- a/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c +++ b/Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c @@ -11,6 +11,8 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogResolver; + static size_t GetAddrInfoResolver_IndexFromHandle(const struct SolidSyslogResolver* base); static void GetAddrInfoResolver_CleanupAtIndex(size_t index, void* context); diff --git a/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c b/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c index b70c08ce..afd0666f 100644 --- a/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c @@ -11,6 +11,8 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogDatagram; + static size_t PosixDatagram_IndexFromHandle(const struct SolidSyslogDatagram* base); static void PosixDatagram_CleanupAtIndex(size_t index, void* context); diff --git a/Platform/Posix/Source/SolidSyslogPosixFileStatic.c b/Platform/Posix/Source/SolidSyslogPosixFileStatic.c index f736e679..a2580a04 100644 --- a/Platform/Posix/Source/SolidSyslogPosixFileStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixFileStatic.c @@ -11,6 +11,8 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogFile; + static size_t PosixFile_IndexFromHandle(const struct SolidSyslogFile* base); static void PosixFile_CleanupAtIndex(size_t index, void* context); diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c index bdf7de4a..762c5685 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c @@ -26,6 +26,7 @@ static inline struct SolidSyslogPosixMessageQueueBuffer* PosixMessageQueueBuffer ); static inline const char* PosixMessageQueueBuffer_QueueName(struct SolidSyslogPosixMessageQueueBuffer* self); +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- distinct semantic meaning; mirrors the public _Create signature void PosixMessageQueueBuffer_Initialise(struct SolidSyslogBuffer* base, size_t maxMessageSize, long maxMessages) { static const char queueNamePrefix[] = "/solidsyslog_"; diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c index 0e1adb0f..d846f6a7 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c @@ -11,17 +11,13 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogBuffer; + /* The shared queue name is derived from the process ID — fine for the * default pool size of 1 instance per process. Bumping the tunable * above 1 in the same process would race two slots onto the same * `/solidsyslog_` queue; that scenario is outside today's contract. */ -struct InitContext -{ - size_t MaxMessageSize; - long MaxMessages; -}; - static size_t PosixMessageQueueBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); static void PosixMessageQueueBuffer_CleanupAtIndex(size_t index, void* context); diff --git a/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c b/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c index 7a0a7984..18cbcbe8 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixMutexStatic.c @@ -11,6 +11,8 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogMutex; + static size_t PosixMutex_IndexFromHandle(const struct SolidSyslogMutex* base); static void PosixMutex_CleanupAtIndex(size_t index, void* context); diff --git a/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c b/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c index 896243fa..4a9a2727 100644 --- a/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c @@ -11,6 +11,8 @@ #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +struct SolidSyslogStream; + static size_t PosixTcpStream_IndexFromHandle(const struct SolidSyslogStream* base); static void PosixTcpStream_CleanupAtIndex(size_t index, void* context); diff --git a/Tests/SolidSyslogGetAddrInfoResolverTest.cpp b/Tests/SolidSyslogGetAddrInfoResolverTest.cpp index 308d3794..4b453aaf 100644 --- a/Tests/SolidSyslogGetAddrInfoResolverTest.cpp +++ b/Tests/SolidSyslogGetAddrInfoResolverTest.cpp @@ -146,6 +146,7 @@ TEST(SolidSyslogGetAddrInfoResolver, FreesAddrInfoOnSuccess) // clang-format off TEST_GROUP(SolidSyslogGetAddrInfoResolverPool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogResolver* pooled[SOLIDSYSLOG_GETADDRINFO_RESOLVER_POOL_SIZE] = {}; struct SolidSyslogResolver* overflow = nullptr; @@ -158,6 +159,7 @@ TEST_GROUP(SolidSyslogGetAddrInfoResolverPool) SolidSyslogGetAddrInfoResolver_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogGetAddrInfoResolver_Destroy(overflow); diff --git a/Tests/SolidSyslogNullDatagramTest.cpp b/Tests/SolidSyslogNullDatagramTest.cpp index 922489a1..18d52bd3 100644 --- a/Tests/SolidSyslogNullDatagramTest.cpp +++ b/Tests/SolidSyslogNullDatagramTest.cpp @@ -1,3 +1,5 @@ +#include + #include "CppUTest/TestHarness.h" #include "SolidSyslogDatagram.h" #include "SolidSyslogNullDatagram.h" diff --git a/Tests/SolidSyslogPosixDatagramTest.cpp b/Tests/SolidSyslogPosixDatagramTest.cpp index 0a408f8e..c1eaa50a 100644 --- a/Tests/SolidSyslogPosixDatagramTest.cpp +++ b/Tests/SolidSyslogPosixDatagramTest.cpp @@ -274,6 +274,7 @@ TEST(SolidSyslogPosixDatagram, SendToReturnsFailedWhenConnectFails) // clang-format off TEST_GROUP(SolidSyslogPosixDatagramPool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogDatagram* pooled[SOLIDSYSLOG_POSIX_DATAGRAM_POOL_SIZE] = {}; struct SolidSyslogDatagram* overflow = nullptr; @@ -286,6 +287,7 @@ TEST_GROUP(SolidSyslogPosixDatagramPool) SolidSyslogPosixDatagram_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogPosixDatagram_Destroy(overflow); diff --git a/Tests/SolidSyslogPosixFileTest.cpp b/Tests/SolidSyslogPosixFileTest.cpp index 59cc1156..3c6faf65 100644 --- a/Tests/SolidSyslogPosixFileTest.cpp +++ b/Tests/SolidSyslogPosixFileTest.cpp @@ -134,6 +134,7 @@ TEST(SolidSyslogPosixFile, DeleteReturnsFalseForNonexistentFile) // clang-format off TEST_GROUP(SolidSyslogPosixFilePool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogFile* pooled[SOLIDSYSLOG_POSIX_FILE_POOL_SIZE] = {}; struct SolidSyslogFile* overflow = nullptr; @@ -146,6 +147,7 @@ TEST_GROUP(SolidSyslogPosixFilePool) SolidSyslogPosixFile_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogPosixFile_Destroy(overflow); diff --git a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp index 823e6c93..f9049fdf 100644 --- a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp +++ b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp @@ -153,6 +153,7 @@ IGNORE_TEST(SolidSyslogPosixMessageQueueBuffer, HappyPathOnly) // clang-format off TEST_GROUP(SolidSyslogPosixMessageQueueBufferPool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogBuffer* pooled[SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE] = {}; struct SolidSyslogBuffer* overflow = nullptr; @@ -165,6 +166,7 @@ TEST_GROUP(SolidSyslogPosixMessageQueueBufferPool) SolidSyslogPosixMessageQueueBuffer_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogPosixMessageQueueBuffer_Destroy(overflow); @@ -173,11 +175,16 @@ TEST_GROUP(SolidSyslogPosixMessageQueueBufferPool) ErrorHandlerFake_Uninstall(); } + struct SolidSyslogBuffer* MakeBuffer() + { + return SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + } + void FillPool() { for (auto*& slot : pooled) { - slot = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + slot = MakeBuffer(); } } }; @@ -188,7 +195,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, FillingPoolThenOverflowReturnsDisti { FillPool(); - overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + overflow = MakeBuffer(); CHECK_IS_FALLBACK(overflow, pooled); } @@ -198,7 +205,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, ExhaustedCreateReportsError) ErrorHandlerFake_Install(nullptr); FillPool(); - overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + overflow = MakeBuffer(); CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); @@ -208,7 +215,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, ExhaustedCreateReportsError) TEST(SolidSyslogPosixMessageQueueBufferPool, FallbackReadAndWriteAreNoOps) { FillPool(); - overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + overflow = MakeBuffer(); SolidSyslogBuffer_Write(overflow, "hello", 5); @@ -222,7 +229,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, CreateAcquiresAndReleasesConfigLock { ConfigLockFake_Install(); - pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + pooled[0] = MakeBuffer(); CALLED_FAKE(ConfigLockFake_Lock, ONCE); CALLED_FAKE(ConfigLockFake_Unlock, ONCE); @@ -233,7 +240,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, CreateLocksOncePerSlotProbedWhenPoo FillPool(); ConfigLockFake_Install(); - overflow = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + overflow = MakeBuffer(); LONGS_EQUAL(SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE, ConfigLockFake_LockCallCount()); LONGS_EQUAL(SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE, ConfigLockFake_UnlockCallCount()); @@ -241,7 +248,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, CreateLocksOncePerSlotProbedWhenPoo TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfPooledHandleLocksOnce) { - pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + pooled[0] = MakeBuffer(); ConfigLockFake_Install(); SolidSyslogPosixMessageQueueBuffer_Destroy(pooled[0]); @@ -276,7 +283,7 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfUnknownHandleReportsWarnin TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfStaleHandleReportsWarning) { - pooled[0] = SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); + pooled[0] = MakeBuffer(); SolidSyslogPosixMessageQueueBuffer_Destroy(pooled[0]); ErrorHandlerFake_Install(nullptr); diff --git a/Tests/SolidSyslogPosixMutexTest.cpp b/Tests/SolidSyslogPosixMutexTest.cpp index bdd76fa2..7658c8f6 100644 --- a/Tests/SolidSyslogPosixMutexTest.cpp +++ b/Tests/SolidSyslogPosixMutexTest.cpp @@ -60,6 +60,7 @@ TEST(SolidSyslogPosixMutex, LockUnlockDoesNotCrash) // clang-format off TEST_GROUP(SolidSyslogPosixMutexPool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogMutex* pooled[SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE] = {}; struct SolidSyslogMutex* overflow = nullptr; @@ -72,6 +73,7 @@ TEST_GROUP(SolidSyslogPosixMutexPool) SolidSyslogPosixMutex_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogPosixMutex_Destroy(overflow); diff --git a/Tests/SolidSyslogPosixTcpStreamTest.cpp b/Tests/SolidSyslogPosixTcpStreamTest.cpp index c061d473..c99d1e2f 100644 --- a/Tests/SolidSyslogPosixTcpStreamTest.cpp +++ b/Tests/SolidSyslogPosixTcpStreamTest.cpp @@ -528,6 +528,7 @@ TEST(SolidSyslogPosixTcpStream, ReadReturnsNegativeOneOnErrorAndClosesSocket) // clang-format off TEST_GROUP(SolidSyslogPosixTcpStreamPool) { + // cppcheck-suppress constVariable -- assigned in test bodies; cppcheck does not model CppUTest lifecycle struct SolidSyslogStream* pooled[SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE] = {}; struct SolidSyslogStream* overflow = nullptr; @@ -540,6 +541,7 @@ TEST_GROUP(SolidSyslogPosixTcpStreamPool) SolidSyslogPosixTcpStream_Destroy(handle); } } + // cppcheck-suppress knownConditionTrueFalse -- assigned in test bodies; cppcheck does not model CppUTest lifecycle if (overflow != nullptr) { SolidSyslogPosixTcpStream_Destroy(overflow); From db54dd2dc126ef2b9e279b61ce0c877043183f3a Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 23:01:48 +0100 Subject: [PATCH 13/15] docs: update DEVLOG for S11.06 Co-Authored-By: Claude Opus 4.7 (1M context) --- DEVLOG.md | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/DEVLOG.md b/DEVLOG.md index a11b3b60..fe24abab 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,156 @@ # Dev Log +## 2026-05-19 — S11.06: POSIX adapters onto PoolAllocator + +Closes S11.06 (#405). The first platform-adapter sweep in E11 — applies the +canonical 3-TU split + `SolidSyslogPoolAllocator` to all six stateful POSIX +adapter classes (`PosixMutex`, `PosixFile`, `PosixTcpStream` — storage-cast; +`PosixDatagram`, `GetAddrInfoResolver`, `PosixMessageQueueBuffer` — +file-scope singleton). + +Twelve commits on the branch: + +- `17101b7 … fce49bb` — four new public GoF nulls land first + (`SolidSyslogNullDatagram`, `_NullStream`, `_NullFile`, `_NullResolver`), + each reused by this story plus the upcoming S11.07/S11.08/S11.09 sweeps. +- `bba44d9` — `SolidSyslogNullMutex` migrated from `_Create`/`_Destroy` to + the `_Get(void)` shape. It was the last stateless GoF null still on the + old shape. Five caller test files updated. +- `f85c73c … 11d3996` — six per-class POSIX migrations, one commit each, + in the order from the issue body. Public `Storage` types and + `SOLIDSYSLOG_*_SIZE` enums deleted from `Platform/Posix/Interface/`; + `_Create` drops the `storage` parameter; `_Destroy(base)` shape adopted + for the three previously `_Destroy(void)` singletons. +- `6957ca1` — local gate sweep cleanup (iwyu, cppcheck CTU false positives, + tidy NOLINT for an `_Initialise` that mirrors a `_Create`'s existing + `bugprone-easily-swappable-parameters` suppression). + +### Decisions + +- **Four GoF nulls bundled in this story up-front.** Same precedent as + S11.04 (`NullSender` + `NullSd`). The pool-exhaustion fallback for + `Datagram` / `Stream` / `File` / `Resolver` is needed *now* by S11.06's + six classes, and by every platform sweep that follows. Bundling the + nulls into this story avoids preceding-story bookkeeping (a la + `NullBuffer` rename in S11.03) and lets the cross-sweep types ship on + the same review surface as their first consumer. + +- **NullDatagram MaxPayload returns `SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD`, + not 0.** MaxPayload is reachable from `UdpSender_RetryAfterOversize`, + and only that path — but since the call site uses the value as a + clip limit, a 0 there would silently drop everything. The IPv6-safe + default (1232) keeps any incidental caller of MaxPayload sane while + the surrounding SendTo-returns-SENT contract still drops the message + on the floor. + +- **NullStream Read returns 0 (would-block), not -1 (EOF/error).** A < 0 + return from Read would force `StreamSender` to flag a broken connection + and tear it down, defeating the "drop on the floor" contract. + Documented in the production source so future Stream impls follow the + same shape for their Null sibling. + +- **NullFile per-method semantics chosen for clean-degrade.** `Open` / + `IsOpen` / `Read` / `Exists` return `false` (presents consistently as a + closed, empty, non-existent file). `Write` / `Delete` return `true` + (drop-on-the-floor for the Write, vacuous-success for the Delete). + In the realistic pool-exhausted scenario the wider chain is already + broken (`BlockStore` on `NullStore`) by the time `NullFile` is + reached, but the semantics here are defensible in isolation too — + any direct caller of `NullFile` degrades cleanly rather than crashing + or spinning on a contradictory state. + +- **NullMutex `_Get` migration sits inside S11.06, not a separate + cleanup.** Asked pre-flight; rolled in here because `PosixMutex`'s + pool-exhaustion fallback wiring needs `NullMutex_Get()` in five test + files anyway, and the other Null-* helpers landing in this story are + already on `_Get`. `Crc16Policy` is the only remaining Null-shaped + class still on `_Create`/`_Destroy` (deliberate — it's not a null + object, it's a stateless policy); deferred to a future small cleanup + if it's worth doing at all. + +- **`GetAddrInfoResolver` stays pool-allocated, not `_Get`.** Mid-sweep + call: the class is *truly stateless today* (its slot just holds the + vtable), and `_Get` would be the more honest shape. Rejected in + favour of symmetry with `FreeRtosStaticResolver` (S11.08) which + carries 4 bytes of IPv4 octet state and therefore needs the pool. + Splitting the two `Resolver` impls onto different lifecycles would + cost long-term consistency for a marginal short-term simplification. + Recorded as an open question in the issue body and explicitly closed + here. + +- **`PosixDatagram` Initialise sets `Fd = INVALID_FD` and + `Connected = false` explicitly.** Pre-migration the file-scope + singleton's `Fd = INVALID_FD` initial value came from the + declaration, and `_Create` never reset it. After Close + a second + Create, the slot relied on Close having set `Fd = -1` and + `Connected = false`. Pool slots are zero-initialised — `Fd = 0` + is a valid FD (stdin) — so the pool migration had to set both + fields explicitly in `_Initialise`. Latent re-Create bug closed + by the migration. + +- **`HandleEqualsStorageAddress` test dropped from + `SolidSyslogPosixMutexTest.cpp`.** Pre-migration it pinned the + invariant "the returned handle equals the address of the storage the + caller supplied" — meaningful for the storage-cast shape, no longer + meaningful when the slot is library-internal. The other existing + tests carry through as the regression net. + +- **Pool-size override validated at 3 for every new tunable.** Built and + ran the full suite at + `SOLIDSYSLOG_POSIX_MUTEX_POOL_SIZE=3` / + `_POSIX_DATAGRAM_POOL_SIZE=3` / + `_GETADDRINFO_RESOLVER_POOL_SIZE=3` / + `_POSIX_FILE_POOL_SIZE=3` / + `_POSIX_TCP_STREAM_POOL_SIZE=3` / + `_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE=3` (single + `SOLIDSYSLOG_USER_TUNABLES_FILE` override). All 1258 tests green. + +- **Verified in `cpputest-freertos` container per + `feedback_verify_in_freertos_host_image`.** Full POSIX suite + the + five FreeRTOS-specific host tests (`FreeRtosDatagram`, `FreeRtosMutex`, + `FreeRtosStaticResolver`, `FreeRtosSysUpTime`, `FreeRtosTcpStream`) + all green; the new tunables propagate cleanly through the FreeRTOS + build's CMake config. + +- **All local gates green.** debug + clang-debug + sanitize + coverage + + tidy + cppcheck + iwyu + clang-format. Coverage 100% line on every + new and migrated file (lcov per-file table confirms). cppcheck-misra + produced no new findings vs `main` — the per-class storage-cast + deviations for `PosixMutex` / `PosixFile` / `PosixTcpStream` + evaporated (the bonus E11 promised). + +### Deferred + +- **`Crc16Policy` shape cleanup.** Last class still on `_Create`/ + `_Destroy` for a stateless thing. Not a null object so doesn't need + `_Get`; could become a constexpr-style policy struct or a function + pair. Not worth its own story today; will fall out of S11.10 / S11.11 + cleanups if anything touches it. + +- **POSIX function-adapter consistency review.** `PosixHostname`, + `PosixProcessId`, `PosixSysUpTime`, `PosixSleep`, `PosixClock` all + use bare function names (`SolidSyslogPosix_Get` / + `_GetTimestamp`). Out of scope per E11 (stateless, no Create), but + worth a glance at S11.11 wrap-up to confirm the audience-table rows + still read consistently after the platform sweeps. + +### Open questions + +- **`PosixMessageQueueBuffer` `mq_open` failure mode.** Today's pool + Create returns the slot unconditionally and stashes whatever + `mqd_t` comes back from `mq_open`; the migration preserves that. + An `mq_open` failure should arguably route to the shared + `SolidSyslogNullBuffer` with a separate error message — but that + shifts the bad-setup contract and belongs in E12, not this sweep. + Flagged in the issue body, deliberately deferred. + +- **`PosixTcpStream`'s `SO_SNDTIMEO` bounded-connect timeout** wasn't + exercised by S11.06's pool tests (they only stress the slot + allocator, not the underlying socket logic). The existing TcpStream + suite carries that contract forward unchanged — no regression risk — + but if the timeout semantics ever drift, the failure mode would be + a Service-loop wedge that's not easy to catch in unit tests. + ## 2026-05-19 — BDD target stderr handler: fatal exit on ERROR severity Follow-up from S11.05 part B post-merge. The PR's final CI run took 23m From b104cfa2f8cf9461f827192f571d24323950a567 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Tue, 19 May 2026 23:27:51 +0100 Subject: [PATCH 14/15] fix: S11.06 CI gate fixes + CodeRabbit feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three classes of fixes folded into one commit: **MSVC /W4 /WX (build-windows-msvc):** the generic `CHECK_EQUAL` macro triggers C4127 ("conditional expression is constant") under MSVC's warnings-as-errors at four sites. Other Null tests in the codebase avoid it by using the specialized `LONGS_EQUAL` / `UNSIGNED_LONGS_EQUAL` macros instead — convention I missed. Switched the four sites in NullDatagramTest / NullFileTest / NullStreamTest to match. **clang-tidy (analyze-tidy):** `Tests/SolidSyslogPosixMessageQueueBufferTest.cpp:178` MakeBuffer fixture helper marked `static` — it references no instance state, so `readability-convert-member-functions-to-static` fires. Other test files' MakeBuffer keeps non-static because they reference fixture data. **CodeRabbit feedback (PR #407):** - **PosixMutex pthread_mutex_init failure.** Initialise now checks the return value; on failure (ENOMEM / EAGAIN under memory pressure) installs the shared NullMutex vtable so Lock/Unlock are safe no-ops. Cleanup gates pthread_mutex_destroy on the PosixMutex vtable being installed, so destroy doesn't run on an uninitialised native handle. Defensive — pthread_mutex_init essentially never fails in our environment; the failure-path else is uncovered (no function-pointer seam exists for testing it, and adding one for a single defensive branch is more cost than the value). - **PosixMessageQueueBuffer slot-indexed queue names.** Pre-fix, each pool slot mapped to `/solidsyslog_` so bumping `SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE` above 1 would alias every slot onto the same kernel queue object. Initialise gains a `size_t slotIndex` parameter; Static.c passes the pool index; queue name becomes `/solidsyslog__`. 64-byte name buffer is plenty of headroom. Audience-table row in CLAUDE.md updated. Tested via the existing `tunable-override-debug` preset: bumped `SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE` to 2 in Tests/Fixtures/SmallMessageSizeTunables.h and added a new `EachPooledHandleHasIsolatedQueue` test (gated by `#if POOL_SIZE >= 2`) that writes through slot 0 and confirms slot 1 doesn't see the message. Default-config build compiles the test out; CI's build-linux-tunable-override job picks it up. CodeRabbit's #1 (NullMutex `instance` rename to project-style) and #4 (extract `CHECK_IS_FALLBACK` to a shared helper) deferred — the former requires a tree-wide rename across all 11 GoF nulls (worth its own naming-sweep story per `project_naming_sweep_deferred`); the latter is pre-existing duplication that predates S11.06 and belongs in a focused test-helper consolidation story. All gates green locally: debug, sanitize, tidy, cppcheck, clang-format, coverage. 99.5% overall line coverage; PosixMutex.c dips to 95.7% (1 defensive line uncovered). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- .../SolidSyslogPosixMessageQueueBuffer.c | 16 ++++++++++++-- ...olidSyslogPosixMessageQueueBufferPrivate.h | 7 +++++- ...SolidSyslogPosixMessageQueueBufferStatic.c | 15 ++++++++----- Platform/Posix/Source/SolidSyslogPosixMutex.c | 21 ++++++++++++++---- Tests/Fixtures/SmallMessageSizeTunables.h | 16 ++++++++++---- Tests/SolidSyslogNullDatagramTest.cpp | 4 ++-- Tests/SolidSyslogNullFileTest.cpp | 2 +- Tests/SolidSyslogNullStreamTest.cpp | 2 +- ...SolidSyslogPosixMessageQueueBufferTest.cpp | 22 ++++++++++++++++++- 10 files changed, 85 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f0cb87c2..2bd8194d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -343,7 +343,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `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. Pool exhaustion resolves to the shared `SolidSyslogNullBuffer`. | -| `SolidSyslogPosixMessageQueueBuffer.h` | System setup code using POSIX message queue buffer | `SolidSyslogPosixMessageQueueBuffer_Create(maxMessageSize, maxMessages)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11). The queue name derives from the process ID, so the default pool size of 1 is the supported configuration in a single process. Pool-exhaustion fallback is the shared `SolidSyslogNullBuffer`. | +| `SolidSyslogPosixMessageQueueBuffer.h` | System setup code using POSIX message queue buffer | `SolidSyslogPosixMessageQueueBuffer_Create(maxMessageSize, maxMessages)`, `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); each slot's queue name is `/solidsyslog__` so multiple in-process slots remain isolated. Pool-exhaustion fallback is 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`) | diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c index 762c5685..91342f5a 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "SolidSyslogBufferDefinition.h" @@ -26,17 +27,28 @@ static inline struct SolidSyslogPosixMessageQueueBuffer* PosixMessageQueueBuffer ); static inline const char* PosixMessageQueueBuffer_QueueName(struct SolidSyslogPosixMessageQueueBuffer* self); -// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- distinct semantic meaning; mirrors the public _Create signature -void PosixMessageQueueBuffer_Initialise(struct SolidSyslogBuffer* base, size_t maxMessageSize, long maxMessages) +// NOLINTBEGIN(bugprone-easily-swappable-parameters) -- distinct semantic meaning; mirrors the public _Create signature plus a per-slot discriminator +void PosixMessageQueueBuffer_Initialise( + struct SolidSyslogBuffer* base, + size_t maxMessageSize, + long maxMessages, + size_t slotIndex +) +// NOLINTEND(bugprone-easily-swappable-parameters) { static const char queueNamePrefix[] = "/solidsyslog_"; struct SolidSyslogPosixMessageQueueBuffer* self = PosixMessageQueueBuffer_SelfFromBase(base); + /* Queue name: /solidsyslog__. The pid keeps the name + * unique per process; the slot index keeps multiple in-process pool + * entries from aliasing onto the same kernel queue object. */ struct SolidSyslogFormatter* name = SolidSyslogFormatter_Create(self->NameStorage, POSIX_MESSAGE_QUEUE_BUFFER_MAX_NAME_SIZE); SolidSyslogFormatter_BoundedString(name, queueNamePrefix, sizeof(queueNamePrefix) - 1U); SolidSyslogPosixProcessId_Get(name); + SolidSyslogFormatter_AsciiCharacter(name, '_'); + SolidSyslogFormatter_Uint32(name, (uint32_t) slotIndex); struct mq_attr attr = {0}; attr.mq_maxmsg = maxMessages; diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h index 974b43c4..43a2272d 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h @@ -21,7 +21,12 @@ struct SolidSyslogPosixMessageQueueBuffer size_t MaxMessageSize; }; -void PosixMessageQueueBuffer_Initialise(struct SolidSyslogBuffer* base, size_t maxMessageSize, long maxMessages); +void PosixMessageQueueBuffer_Initialise( + struct SolidSyslogBuffer* base, + size_t maxMessageSize, + long maxMessages, + size_t slotIndex +); void PosixMessageQueueBuffer_Cleanup(struct SolidSyslogBuffer* base); #endif /* SOLIDSYSLOGPOSIXMESSAGEQUEUEBUFFERPRIVATE_H */ diff --git a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c index d846f6a7..88435a28 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c +++ b/Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c @@ -13,10 +13,10 @@ struct SolidSyslogBuffer; -/* The shared queue name is derived from the process ID — fine for the - * default pool size of 1 instance per process. Bumping the tunable - * above 1 in the same process would race two slots onto the same - * `/solidsyslog_` queue; that scenario is outside today's contract. */ +/* Each pool slot's queue name is `/solidsyslog__`. The + * pid keeps the name unique per process; the slot index keeps multiple + * in-process pool entries from aliasing onto the same kernel queue + * object when the pool tunable is bumped above 1. */ static size_t PosixMessageQueueBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base); static void PosixMessageQueueBuffer_CleanupAtIndex(size_t index, void* context); @@ -36,7 +36,12 @@ struct SolidSyslogBuffer* SolidSyslogPosixMessageQueueBuffer_Create(size_t maxMe struct SolidSyslogBuffer* handle = SolidSyslogNullBuffer_Get(); if (SolidSyslogPoolAllocator_IndexIsValid(&PosixMessageQueueBuffer_Allocator, index)) { - PosixMessageQueueBuffer_Initialise(&PosixMessageQueueBuffer_Pool[index].Base, maxMessageSize, maxMessages); + PosixMessageQueueBuffer_Initialise( + &PosixMessageQueueBuffer_Pool[index].Base, + maxMessageSize, + maxMessages, + index + ); handle = &PosixMessageQueueBuffer_Pool[index].Base; } else diff --git a/Platform/Posix/Source/SolidSyslogPosixMutex.c b/Platform/Posix/Source/SolidSyslogPosixMutex.c index 7236ce8e..523ff809 100644 --- a/Platform/Posix/Source/SolidSyslogPosixMutex.c +++ b/Platform/Posix/Source/SolidSyslogPosixMutex.c @@ -15,15 +15,28 @@ static inline struct SolidSyslogPosixMutex* PosixMutex_SelfFromBase(struct Solid void PosixMutex_Initialise(struct SolidSyslogMutex* base) { struct SolidSyslogPosixMutex* self = PosixMutex_SelfFromBase(base); - self->Base.Lock = PosixMutex_Lock; - self->Base.Unlock = PosixMutex_Unlock; - pthread_mutex_init(&self->Mutex, NULL); + if (pthread_mutex_init(&self->Mutex, NULL) == 0) + { + self->Base.Lock = PosixMutex_Lock; + self->Base.Unlock = PosixMutex_Unlock; + } + else + { + /* pthread_mutex_init failed (ENOMEM, EAGAIN). Install the shared + * NullMutex vtable so Lock/Unlock are safe no-ops; Cleanup will + * see the marker and skip pthread_mutex_destroy on the + * uninitialised native handle. */ + *base = *SolidSyslogNullMutex_Get(); + } } void PosixMutex_Cleanup(struct SolidSyslogMutex* base) { struct SolidSyslogPosixMutex* self = PosixMutex_SelfFromBase(base); - pthread_mutex_destroy(&self->Mutex); + if (self->Base.Lock == PosixMutex_Lock) + { + pthread_mutex_destroy(&self->Mutex); + } /* Overwrite the abstract base with the shared NullMutex vtable so * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ *base = *SolidSyslogNullMutex_Get(); diff --git a/Tests/Fixtures/SmallMessageSizeTunables.h b/Tests/Fixtures/SmallMessageSizeTunables.h index a65901ce..a515b06d 100644 --- a/Tests/Fixtures/SmallMessageSizeTunables.h +++ b/Tests/Fixtures/SmallMessageSizeTunables.h @@ -1,5 +1,13 @@ -/* Test fixture: drives a smaller-than-default SOLIDSYSLOG_MAX_MESSAGE_SIZE - * via the SOLIDSYSLOG_USER_TUNABLES_FILE override mechanism. Wired in by - * the `tunable-override-debug` CMake preset to prove that the user-config - * override path actually flows through to the compiler. */ +/* Test fixture: drives override values via the SOLIDSYSLOG_USER_TUNABLES_FILE + * mechanism. Wired in by the `tunable-override-debug` CMake preset to + * prove that the user-config override path actually flows through to the + * compiler. + * + * - SOLIDSYSLOG_MAX_MESSAGE_SIZE smaller than the default proves the + * message-size knob. + * - SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE bumped to 2 enables + * `EachPooledHandleHasIsolatedQueue` in SolidSyslogPosixMessageQueueBufferTest + * to exercise the slot-indexed queue-name discriminator (CodeRabbit + * feedback on PR #407). */ #define SOLIDSYSLOG_MAX_MESSAGE_SIZE 512 +#define SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE 2U diff --git a/Tests/SolidSyslogNullDatagramTest.cpp b/Tests/SolidSyslogNullDatagramTest.cpp index 18d52bd3..e9893aaa 100644 --- a/Tests/SolidSyslogNullDatagramTest.cpp +++ b/Tests/SolidSyslogNullDatagramTest.cpp @@ -21,7 +21,7 @@ TEST_GROUP(SolidSyslogNullDatagram) TEST(SolidSyslogNullDatagram, SendToReturnsSentToDropOnTheFloor) { - CHECK_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, SolidSyslogDatagram_SendTo(datagram, "x", 1, nullptr)); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, SolidSyslogDatagram_SendTo(datagram, "x", 1, nullptr)); } TEST(SolidSyslogNullDatagram, OpenReturnsTrue) @@ -31,7 +31,7 @@ TEST(SolidSyslogNullDatagram, OpenReturnsTrue) TEST(SolidSyslogNullDatagram, MaxPayloadReturnsIpv6SafeDefault) { - CHECK_EQUAL((size_t) SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD, SolidSyslogDatagram_MaxPayload(datagram)); + UNSIGNED_LONGS_EQUAL((size_t) SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD, SolidSyslogDatagram_MaxPayload(datagram)); } TEST(SolidSyslogNullDatagram, CloseDoesNotCrash) diff --git a/Tests/SolidSyslogNullFileTest.cpp b/Tests/SolidSyslogNullFileTest.cpp index bb61eae5..8d5d7e00 100644 --- a/Tests/SolidSyslogNullFileTest.cpp +++ b/Tests/SolidSyslogNullFileTest.cpp @@ -49,7 +49,7 @@ TEST(SolidSyslogNullFile, SeekToDoesNotCrash) TEST(SolidSyslogNullFile, SizeReturnsZero) { - CHECK_EQUAL(0U, SolidSyslogFile_Size(file)); + UNSIGNED_LONGS_EQUAL(0U, SolidSyslogFile_Size(file)); } TEST(SolidSyslogNullFile, TruncateDoesNotCrash) diff --git a/Tests/SolidSyslogNullStreamTest.cpp b/Tests/SolidSyslogNullStreamTest.cpp index 581f0b73..06d6ba00 100644 --- a/Tests/SolidSyslogNullStreamTest.cpp +++ b/Tests/SolidSyslogNullStreamTest.cpp @@ -29,7 +29,7 @@ TEST(SolidSyslogNullStream, OpenReturnsTrue) TEST(SolidSyslogNullStream, ReadReturnsZeroForWouldBlock) { char buffer[1] = {0}; - CHECK_EQUAL(0, SolidSyslogStream_Read(stream, buffer, sizeof(buffer))); + LONGS_EQUAL(0, SolidSyslogStream_Read(stream, buffer, sizeof(buffer))); } TEST(SolidSyslogNullStream, CloseDoesNotCrash) diff --git a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp index f9049fdf..ac77990e 100644 --- a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp +++ b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp @@ -175,7 +175,7 @@ TEST_GROUP(SolidSyslogPosixMessageQueueBufferPool) ErrorHandlerFake_Uninstall(); } - struct SolidSyslogBuffer* MakeBuffer() + static struct SolidSyslogBuffer* MakeBuffer() { return SolidSyslogPosixMessageQueueBuffer_Create(SOLIDSYSLOG_MAX_MESSAGE_SIZE, 10); } @@ -294,3 +294,23 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfStaleHandleReportsWarning) LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); } + +/* Pool-size >= 2 isolation contract. Exercised by the + * `tunable-override-debug` preset (which sets the pool size to 2 via + * Tests/Fixtures/SmallMessageSizeTunables.h); the default build compiles + * the test out because the pool size is 1 and a two-slot scenario isn't + * representable. */ +#if SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE >= 2 +TEST(SolidSyslogPosixMessageQueueBufferPool, EachPooledHandleHasIsolatedQueue) +{ + pooled[0] = MakeBuffer(); + pooled[1] = MakeBuffer(); + + SolidSyslogBuffer_Write(pooled[0], "slot0", 5); + + char readBuffer[SOLIDSYSLOG_MAX_MESSAGE_SIZE] = {}; + size_t bytesRead = 99U; + CHECK_FALSE(SolidSyslogBuffer_Read(pooled[1], readBuffer, sizeof(readBuffer), &bytesRead)); + LONGS_EQUAL(0, bytesRead); +} +#endif From 283d6e89b16da244b14dd574d95fbe79e029401a Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 20 May 2026 07:53:38 +0100 Subject: [PATCH 15/15] fix: S11.06 replace #if pool-size gate with runtime TEST_EXIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conditional compilation on a tunable hid the test from the run entirely at default pool size 1, making the result dishonest — the test simply disappeared rather than being accounted for. Replace the `#if SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE >= 2` wrapper with a runtime guard: if the pool size can't host a second slot, UT_PRINT a notice and TEST_EXIT. The test is now in the count in every build; the default build prints "Pool size < 2 — slot isolation only observable under tunable-override-debug" and exits cleanly; the tunable-override build exercises the assertion. Switched the test body from fixture's `pooled[]` array (sized to SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE — `pooled[1]` is out-of-bounds at pool size 1) to local `slotZero`/`slotOne` pointers. Cleanup happens before the CHECK_FALSE/LONGS_EQUAL so a failing assertion doesn't leak pool slots. Audit of the rest of the tree found no other test-side conditional compilation introduced by S11.06. Pre-existing patterns remain in Tests/SolidSyslogTunablesTest.cpp:10 (#ifdef around static_assert that proves the user-tunables override flowed through) and the standard header-guard / __cplusplus / SOLIDSYSLOG_USER_TUNABLES_FILE include mechanism / SOLIDSYSLOG_*_POOL_SIZE compile-time floor checks. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...SolidSyslogPosixMessageQueueBufferTest.cpp | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp index ac77990e..71de18ee 100644 --- a/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp +++ b/Tests/SolidSyslogPosixMessageQueueBufferTest.cpp @@ -295,22 +295,34 @@ TEST(SolidSyslogPosixMessageQueueBufferPool, DestroyOfStaleHandleReportsWarning) STRCMP_EQUAL(SOLIDSYSLOG_ERROR_MSG_POSIXMESSAGEQUEUEBUFFER_UNKNOWN_DESTROY, ErrorHandlerFake_LastMessage()); } -/* Pool-size >= 2 isolation contract. Exercised by the - * `tunable-override-debug` preset (which sets the pool size to 2 via - * Tests/Fixtures/SmallMessageSizeTunables.h); the default build compiles - * the test out because the pool size is 1 and a two-slot scenario isn't - * representable. */ -#if SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE >= 2 +/* Slot-indexed queue names are only observable with at least two pool + * slots. The default build runs at pool size 1; the + * `tunable-override-debug` preset bumps it to 2 (see + * Tests/Fixtures/SmallMessageSizeTunables.h). When the runtime pool + * size can't host a second slot, print a notice and exit cleanly via + * TEST_EXIT so the test is honestly accounted for in the run rather + * than compiled out. Local pointers (not fixture's `pooled[]`) so the + * second handle has compile-time storage even at pool size 1. */ TEST(SolidSyslogPosixMessageQueueBufferPool, EachPooledHandleHasIsolatedQueue) { - pooled[0] = MakeBuffer(); - pooled[1] = MakeBuffer(); + if (SOLIDSYSLOG_POSIX_MESSAGE_QUEUE_BUFFER_POOL_SIZE < 2U) + { + UT_PRINT("Pool size < 2 — slot isolation only observable under tunable-override-debug"); + TEST_EXIT; + } - SolidSyslogBuffer_Write(pooled[0], "slot0", 5); + struct SolidSyslogBuffer* slotZero = MakeBuffer(); + struct SolidSyslogBuffer* slotOne = MakeBuffer(); + + SolidSyslogBuffer_Write(slotZero, "slot0", 5U); char readBuffer[SOLIDSYSLOG_MAX_MESSAGE_SIZE] = {}; size_t bytesRead = 99U; - CHECK_FALSE(SolidSyslogBuffer_Read(pooled[1], readBuffer, sizeof(readBuffer), &bytesRead)); + bool readSucceeded = SolidSyslogBuffer_Read(slotOne, readBuffer, sizeof(readBuffer), &bytesRead); + + SolidSyslogPosixMessageQueueBuffer_Destroy(slotZero); + SolidSyslogPosixMessageQueueBuffer_Destroy(slotOne); + + CHECK_FALSE(readSucceeded); LONGS_EQUAL(0, bytesRead); } -#endif