diff --git a/Bdd/Targets/FreeRtos/main.c b/Bdd/Targets/FreeRtos/main.c index c7a74fd4..e776e8e7 100644 --- a/Bdd/Targets/FreeRtos/main.c +++ b/Bdd/Targets/FreeRtos/main.c @@ -195,7 +195,6 @@ static volatile bool solidSyslogTeardown = false; static const char STORE_PATH_PREFIX[] = "STORE"; static SolidSyslogFatFsFileStorage storeFileStorage; -static SolidSyslogBlockStoreStorage blockStoreStorage; /* FATFS object lives in .bss because f_mount stores its address inside the * FatFs volume registry — the object must outlive every f_open / f_stat / @@ -604,7 +603,7 @@ static bool RebuildWithFileStore(void) .OnThresholdCrossed = NULL, .ThresholdContext = &pendingCapacityThreshold, }; - currentStore = SolidSyslogBlockStore_Create(&blockStoreStorage, &storeConfig); + currentStore = SolidSyslogBlockStore_Create(&storeConfig); currentStoreIsFile = true; solidSyslogConfig.Store = currentStore; diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index 97100a08..82283747 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -175,8 +175,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetOptions* optio storeConfig.OnThresholdCrossed = OnThresholdCrossed; storeConfig.ThresholdContext = &capacityThreshold; - static SolidSyslogBlockStoreStorage storeStorage; - return SolidSyslogBlockStore_Create(&storeStorage, &storeConfig); + return SolidSyslogBlockStore_Create(&storeConfig); } return SolidSyslogNullStore_Get(); diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index 8deb76f0..ef1fb6d9 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -249,8 +249,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetWindowsOptions storeConfig.OnThresholdCrossed = OnThresholdCrossed; storeConfig.ThresholdContext = &capacityThreshold; - static SolidSyslogBlockStoreStorage storeStorage; - return SolidSyslogBlockStore_Create(&storeStorage, &storeConfig); + return SolidSyslogBlockStore_Create(&storeConfig); } return SolidSyslogNullStore_Get(); diff --git a/CLAUDE.md b/CLAUDE.md index 90bc2f61..240652d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -356,7 +356,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullBlockDevice.h` | Any code installing a no-op block device slot (every method returns `false` / `0` — disk doesn't exist) | `SolidSyslogNullBlockDevice_Get` | | `SolidSyslogBlockDeviceDefinition.h` | BlockDevice implementors (extension point) | `SolidSyslogBlockDevice` vtable struct (`Acquire`, `Dispose`, `Exists`, `Read`, `Append`, `WriteAt`, `Size`) | | `SolidSyslogFileBlockDevice.h` | System setup code wiring a file-backed block device | `SolidSyslogFileBlockDevice_Create(file, pathPrefix)` (sequence-numbered filenames `.log`, one cached file handle), `_Destroy(base)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool exhaustion resolves to the shared `SolidSyslogNullBlockDevice`. | -| `SolidSyslogBlockStore.h` | System setup code using a BlockDevice-backed store | `SolidSyslogBlockStoreStorage`, `SOLIDSYSLOG_BLOCKSTORE_STORAGE_SIZE`, `SolidSyslogBlockStoreConfig` (with `blockDevice`, `storeFullContext`, `getCapacityThreshold`, `onThresholdCrossed`, `thresholdContext`), `SolidSyslogBlockStore_Create(storage, config)`, `_Destroy(store)`, `SolidSyslogDiscardPolicy` (`SolidSyslogDiscardPolicy_Oldest` / `_Newest` / `_Halt`), `SolidSyslogStoreFullCallback(void* context)`, `SolidSyslogStoreThresholdFunction(void* context)` (returns threshold in bytes; 0 disables; queried each Write), `SolidSyslogStoreThresholdCallback(void* context)` (edge-triggered; PassthroughBuffer recursion gotcha — see header) | +| `SolidSyslogBlockStore.h` | System setup code using a BlockDevice-backed store | `SolidSyslogBlockStoreConfig` (with `blockDevice`, `storeFullContext`, `getCapacityThreshold`, `onThresholdCrossed`, `thresholdContext`), `SolidSyslogBlockStore_Create(config)`, `_Destroy(store)`, `SolidSyslogDiscardPolicy` (`SolidSyslogDiscardPolicy_Oldest` / `_Newest` / `_Halt`), `SolidSyslogStoreFullCallback(void* context)`, `SolidSyslogStoreThresholdFunction(void* context)` (returns threshold in bytes; 0 disables; queried each Write), `SolidSyslogStoreThresholdCallback(void* context)` (edge-triggered; PassthroughBuffer recursion gotcha — see header). Instance struct lives in a library-internal static pool (E11); each slot composes one inner RecordStore + BlockSequence drawn 1:1 from sibling pools sized off the same `SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE` tunable. Pool exhaustion (or either inner Create failing) resolves to the shared `SolidSyslogNullStore`. | | `SolidSyslogSecurityPolicyDefinition.h` | SecurityPolicy implementors (extension point) | `SolidSyslogSecurityPolicy` vtable struct, `SOLIDSYSLOG_MAX_INTEGRITY_SIZE` | | `SolidSyslogNullSecurityPolicy.h` | System setup code (no integrity checking) | `SolidSyslogNullSecurityPolicy_Get` | | `SolidSyslogCrc16Policy.h` | System setup code using CRC-16 integrity | `SolidSyslogCrc16Policy_Create`, `_Destroy` | diff --git a/Core/Interface/SolidSyslogBlockStore.h b/Core/Interface/SolidSyslogBlockStore.h index 8b2b45f0..279a4774 100644 --- a/Core/Interface/SolidSyslogBlockStore.h +++ b/Core/Interface/SolidSyslogBlockStore.h @@ -2,10 +2,7 @@ #define SOLIDSYSLOGBLOCKSTORE_H #include -#include -#include "SolidSyslogSecurityPolicyDefinition.h" -#include "SolidSyslogTunables.h" #include "ExternC.h" struct SolidSyslogStore; @@ -13,6 +10,7 @@ struct SolidSyslogStore; EXTERN_C_BEGIN struct SolidSyslogBlockDevice; + struct SolidSyslogSecurityPolicy; enum SolidSyslogDiscardPolicy { @@ -48,21 +46,7 @@ EXTERN_C_BEGIN void* ThresholdContext; }; - enum - { - SOLIDSYSLOG_BLOCK_STORE_STORAGE_SIZE = - (sizeof(intptr_t) * 32U) + SOLIDSYSLOG_MAX_MESSAGE_SIZE + SOLIDSYSLOG_MAX_INTEGRITY_SIZE + 16U - }; - - typedef struct - { - intptr_t slots[(SOLIDSYSLOG_BLOCK_STORE_STORAGE_SIZE + sizeof(intptr_t) - 1U) / sizeof(intptr_t)]; - } SolidSyslogBlockStoreStorage; - - struct SolidSyslogStore* SolidSyslogBlockStore_Create( - SolidSyslogBlockStoreStorage * storage, - const struct SolidSyslogBlockStoreConfig* config - ); + struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBlockStoreConfig* config); void SolidSyslogBlockStore_Destroy(struct SolidSyslogStore * base); EXTERN_C_END diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 447373bb..f709c28a 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -135,6 +135,31 @@ #error "SOLIDSYSLOG_STREAM_SENDER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogBlockStore instances the library's internal + * static pool can simultaneously hold. Sizes three pools 1:1 — the + * BlockStore slot itself, plus the TU-internal RecordStore and + * BlockSequence pools that each BlockStore composes. The 1:1 + * invariant means a BlockStore slot is guaranteed a free RecordStore + * and BlockSequence on Create; if any inner Create returns its + * fallback under normal use, the BlockStore as a whole resolves to + * SolidSyslogNullStore. + * + * Default 1 — almost all integrators wire a single store-and-forward + * BlockStore. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if more than + * one is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE +/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */ +#define SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE < 1 +#error "SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogFileBlockDevice instances the library's internal * static pool can simultaneously hold. Each instance carries the cached diff --git a/Core/Source/BlockSequence.c b/Core/Source/BlockSequence.c index b2d59f14..0f280b00 100644 --- a/Core/Source/BlockSequence.c +++ b/Core/Source/BlockSequence.c @@ -1,6 +1,13 @@ -#include "BlockSequence.h" +#include "BlockSequencePrivate.h" #include "SolidSyslogBlockDevice.h" +#include "SolidSyslogBlockStore.h" + +#include +#include +#include + +struct SolidSyslogBlockDevice; enum { @@ -39,7 +46,7 @@ static inline size_t BlockSequence_ClampToRange(size_t value, size_t min, size_t return result; } -void BlockSequence_Init(struct BlockSequence* blockSequence, const struct BlockSequenceConfig* config) +void BlockSequence_Initialise(struct BlockSequence* blockSequence, const struct BlockSequenceConfig* config) { blockSequence->BlockDevice = config->BlockDevice; blockSequence->MaxBlockSize = config->MaxBlockSize; @@ -61,6 +68,14 @@ void BlockSequence_Init(struct BlockSequence* blockSequence, const struct BlockS blockSequence->WriteBlockCorrupt = false; } +void BlockSequence_Cleanup(struct BlockSequence* blockSequence) +{ + /* No owned resources to release. The BlockDevice pointer is caller-owned + * (integrator-supplied via BlockSequenceConfig.BlockDevice); the next + * _Initialise overwrites every field, so clearing here would be churn. */ + (void) blockSequence; +} + static bool BlockSequence_ScanForExistingBlocks(struct BlockSequence* blockSequence); static inline void BlockSequence_NotifyThresholdCrossed(struct BlockSequence* blockSequence); diff --git a/Core/Source/BlockSequence.h b/Core/Source/BlockSequencePrivate.h similarity index 84% rename from Core/Source/BlockSequence.h rename to Core/Source/BlockSequencePrivate.h index 50bf33d0..ef34140b 100644 --- a/Core/Source/BlockSequence.h +++ b/Core/Source/BlockSequencePrivate.h @@ -1,5 +1,5 @@ -#ifndef SOLIDSYSLOG_BLOCKSEQUENCE_H -#define SOLIDSYSLOG_BLOCKSEQUENCE_H +#ifndef SOLIDSYSLOG_BLOCKSEQUENCEPRIVATE_H +#define SOLIDSYSLOG_BLOCKSEQUENCEPRIVATE_H #include "SolidSyslogBlockStore.h" @@ -44,7 +44,12 @@ struct BlockSequence bool WriteBlockCorrupt; }; -void BlockSequence_Init(struct BlockSequence* blockSequence, const struct BlockSequenceConfig* config); +void BlockSequence_Initialise(struct BlockSequence* blockSequence, const struct BlockSequenceConfig* config); +void BlockSequence_Cleanup(struct BlockSequence* blockSequence); + +struct BlockSequence* BlockSequence_Create(const struct BlockSequenceConfig* config); +void BlockSequence_Destroy(struct BlockSequence* blockSequence); + bool BlockSequence_Open(struct BlockSequence* blockSequence); bool BlockSequence_PrepareForWrite(struct BlockSequence* blockSequence, size_t recordSize, bool* readBlockChanged); @@ -65,4 +70,4 @@ bool BlockSequence_IsHalted(const struct BlockSequence* blockSequence); size_t BlockSequence_TotalBytes(const struct BlockSequence* blockSequence); size_t BlockSequence_UsedBytes(const struct BlockSequence* blockSequence); -#endif /* SOLIDSYSLOG_BLOCKSEQUENCE_H */ +#endif /* SOLIDSYSLOG_BLOCKSEQUENCEPRIVATE_H */ diff --git a/Core/Source/BlockSequenceStatic.c b/Core/Source/BlockSequenceStatic.c new file mode 100644 index 00000000..4dc5e49b --- /dev/null +++ b/Core/Source/BlockSequenceStatic.c @@ -0,0 +1,60 @@ +#include +#include + +#include "BlockSequencePrivate.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogTunables.h" + +static size_t BlockSequence_IndexFromHandle(const struct BlockSequence* blockSequence); +static void BlockSequence_CleanupAtIndex(size_t index, void* context); + +static bool BlockSequence_InUse[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct BlockSequence BlockSequence_Pool[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct SolidSyslogPoolAllocator BlockSequence_Allocator = { + BlockSequence_InUse, + SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE +}; + +struct BlockSequence* BlockSequence_Create(const struct BlockSequenceConfig* config) +{ + struct BlockSequence* result = NULL; + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&BlockSequence_Allocator); + if (SolidSyslogPoolAllocator_IndexIsValid(&BlockSequence_Allocator, index)) + { + BlockSequence_Initialise(&BlockSequence_Pool[index], config); + result = &BlockSequence_Pool[index]; + } + return result; +} + +void BlockSequence_Destroy(struct BlockSequence* blockSequence) +{ + if (blockSequence != NULL) + { + size_t index = BlockSequence_IndexFromHandle(blockSequence); + if (SolidSyslogPoolAllocator_IndexIsValid(&BlockSequence_Allocator, index)) + { + SolidSyslogPoolAllocator_FreeIfInUse(&BlockSequence_Allocator, index, BlockSequence_CleanupAtIndex, NULL); + } + } +} + +static size_t BlockSequence_IndexFromHandle(const struct BlockSequence* blockSequence) +{ + size_t result = SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; poolIndex++) + { + if (blockSequence == &BlockSequence_Pool[poolIndex]) + { + result = poolIndex; + break; + } + } + return result; +} + +static void BlockSequence_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + BlockSequence_Cleanup(&BlockSequence_Pool[index]); +} diff --git a/Core/Source/CMakeLists.txt b/Core/Source/CMakeLists.txt index 69269cc5..0aff1dce 100644 --- a/Core/Source/CMakeLists.txt +++ b/Core/Source/CMakeLists.txt @@ -33,8 +33,11 @@ set(SOURCES SolidSyslogOriginSdStatic.c SolidSyslogFile.c SolidSyslogBlockStore.c + SolidSyslogBlockStoreStatic.c RecordStore.c + RecordStoreStatic.c BlockSequence.c + BlockSequenceStatic.c SolidSyslogBlockDevice.c SolidSyslogFileBlockDevice.c SolidSyslogFileBlockDeviceStatic.c diff --git a/Core/Source/RecordStore.c b/Core/Source/RecordStore.c index 33e1194c..06cf0689 100644 --- a/Core/Source/RecordStore.c +++ b/Core/Source/RecordStore.c @@ -1,10 +1,15 @@ -#include "RecordStore.h" +#include "RecordStorePrivate.h" #include "SolidSyslogBlockDevice.h" +#include "SolidSyslogSecurityPolicyDefinition.h" #include "SolidSyslogTunables.h" +#include +#include #include +struct SolidSyslogBlockDevice; + enum { MAGIC_SIZE = 2, @@ -71,7 +76,7 @@ static inline size_t RecordStore_SentFlagOffset( return RecordStore_IntegrityChecksumOffset(recordStart, dataLength) + recordStore->SecurityPolicy->IntegritySize; } -void RecordStore_Init(struct RecordStore* recordStore, struct SolidSyslogSecurityPolicy* securityPolicy) +void RecordStore_Initialise(struct RecordStore* recordStore, struct SolidSyslogSecurityPolicy* securityPolicy) { recordStore->SecurityPolicy = securityPolicy; recordStore->HasReadRecord = false; @@ -79,6 +84,14 @@ void RecordStore_Init(struct RecordStore* recordStore, struct SolidSyslogSecurit recordStore->LastSentFlagOffset = 0; } +void RecordStore_Cleanup(struct RecordStore* recordStore) +{ + /* No owned resources to release. The next _Initialise overwrites every + * field, so leave the slot's bytes alone — clearing them would be + * write-then-overwrite churn. */ + (void) recordStore; +} + size_t RecordStore_RecordSize(const struct RecordStore* recordStore, uint16_t dataLength) { return (size_t) MAGIC_SIZE + RECORD_LENGTH_SIZE + dataLength + recordStore->SecurityPolicy->IntegritySize + diff --git a/Core/Source/RecordStore.h b/Core/Source/RecordStorePrivate.h similarity index 75% rename from Core/Source/RecordStore.h rename to Core/Source/RecordStorePrivate.h index 34daff2d..5cd6caf8 100644 --- a/Core/Source/RecordStore.h +++ b/Core/Source/RecordStorePrivate.h @@ -1,5 +1,5 @@ -#ifndef SOLIDSYSLOG_RECORDSTORE_H -#define SOLIDSYSLOG_RECORDSTORE_H +#ifndef SOLIDSYSLOG_RECORDSTOREPRIVATE_H +#define SOLIDSYSLOG_RECORDSTOREPRIVATE_H #include "SolidSyslogSecurityPolicyDefinition.h" #include "SolidSyslogTunables.h" @@ -24,7 +24,11 @@ struct RecordStore uint8_t Buffer[RECORD_BUFFER_SIZE]; }; -void RecordStore_Init(struct RecordStore* recordStore, struct SolidSyslogSecurityPolicy* securityPolicy); +void RecordStore_Initialise(struct RecordStore* recordStore, struct SolidSyslogSecurityPolicy* securityPolicy); +void RecordStore_Cleanup(struct RecordStore* recordStore); + +struct RecordStore* RecordStore_Create(struct SolidSyslogSecurityPolicy* securityPolicy); +void RecordStore_Destroy(struct RecordStore* recordStore); size_t RecordStore_RecordSize(const struct RecordStore* recordStore, uint16_t dataLength); @@ -62,4 +66,4 @@ size_t RecordStore_FindFirstUnsent( bool* corrupt ); -#endif /* SOLIDSYSLOG_RECORDSTORE_H */ +#endif /* SOLIDSYSLOG_RECORDSTOREPRIVATE_H */ diff --git a/Core/Source/RecordStoreStatic.c b/Core/Source/RecordStoreStatic.c new file mode 100644 index 00000000..0009fa9a --- /dev/null +++ b/Core/Source/RecordStoreStatic.c @@ -0,0 +1,59 @@ +#include +#include + +#include "RecordStorePrivate.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogSecurityPolicy; + +static size_t RecordStore_IndexFromHandle(const struct RecordStore* recordStore); +static void RecordStore_CleanupAtIndex(size_t index, void* context); + +static bool RecordStore_InUse[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct RecordStore RecordStore_Pool[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct SolidSyslogPoolAllocator RecordStore_Allocator = {RecordStore_InUse, SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE}; + +struct RecordStore* RecordStore_Create(struct SolidSyslogSecurityPolicy* securityPolicy) +{ + struct RecordStore* result = NULL; + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&RecordStore_Allocator); + if (SolidSyslogPoolAllocator_IndexIsValid(&RecordStore_Allocator, index)) + { + RecordStore_Initialise(&RecordStore_Pool[index], securityPolicy); + result = &RecordStore_Pool[index]; + } + return result; +} + +void RecordStore_Destroy(struct RecordStore* recordStore) +{ + if (recordStore != NULL) + { + size_t index = RecordStore_IndexFromHandle(recordStore); + if (SolidSyslogPoolAllocator_IndexIsValid(&RecordStore_Allocator, index)) + { + SolidSyslogPoolAllocator_FreeIfInUse(&RecordStore_Allocator, index, RecordStore_CleanupAtIndex, NULL); + } + } +} + +static size_t RecordStore_IndexFromHandle(const struct RecordStore* recordStore) +{ + size_t result = SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; poolIndex++) + { + if (recordStore == &RecordStore_Pool[poolIndex]) + { + result = poolIndex; + break; + } + } + return result; +} + +static void RecordStore_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + RecordStore_Cleanup(&RecordStore_Pool[index]); +} diff --git a/Core/Source/SolidSyslogBlockStore.c b/Core/Source/SolidSyslogBlockStore.c index 2c38f459..18e50436 100644 --- a/Core/Source/SolidSyslogBlockStore.c +++ b/Core/Source/SolidSyslogBlockStore.c @@ -1,13 +1,13 @@ #include "SolidSyslogBlockStore.h" #include +#include -#include "BlockSequence.h" -#include "RecordStore.h" -#include "SolidSyslogTunables.h" +#include "BlockSequencePrivate.h" +#include "RecordStorePrivate.h" #include "SolidSyslogBlockDevice.h" -#include "SolidSyslogMacros.h" -#include "SolidSyslogNullSecurityPolicy.h" +#include "SolidSyslogBlockStorePrivate.h" +#include "SolidSyslogNullStore.h" #include "SolidSyslogStoreDefinition.h" /* vtable — forward-declared because BlockStore_InitialiseVtable references them before their definitions */ @@ -20,102 +20,53 @@ static size_t BlockStore_GetTotalBytes(struct SolidSyslogStore* base); static size_t BlockStore_GetUsedBytes(struct SolidSyslogStore* base); static bool BlockStore_IsTransient(struct SolidSyslogStore* base); -/* ------------------------------------------------------------------ - * Instance - * ----------------------------------------------------------------*/ - -struct SolidSyslogBlockStore -{ - struct SolidSyslogStore Base; - struct RecordStore RecordStore; - struct BlockSequence BlockSequence; -}; - -SOLIDSYSLOG_STATIC_ASSERT( - sizeof(struct SolidSyslogBlockStore) <= sizeof(SolidSyslogBlockStoreStorage), - "SOLIDSYSLOG_BLOCK_STORE_STORAGE_SIZE is too small for struct SolidSyslogBlockStore" -); - -static const struct SolidSyslogBlockStore DEFAULT_INSTANCE = {0}; - -static inline struct SolidSyslogBlockStore* BlockStore_SelfFromStorage(SolidSyslogBlockStoreStorage* storage); static inline struct SolidSyslogBlockStore* BlockStore_SelfFromBase(struct SolidSyslogStore* base); -static inline struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy( - struct SolidSyslogSecurityPolicy* configured -); -static inline struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( - const struct SolidSyslogBlockStoreConfig* config, - const struct RecordStore* recordStore -); static inline void BlockStore_InitialiseVtable(struct SolidSyslogBlockStore* self); static void BlockStore_ResumeFromExistingBlock(struct SolidSyslogBlockStore* self); /* ------------------------------------------------------------------ - * Create + * Initialise / Cleanup — private lifecycle pair invoked by Static.c. + * + * Initialise stores the two inner pool pointers (Static.c acquired them + * before this call), wires the vtable, and runs the existing-block + * resume scan. Cleanup is a pure vtable swap to NullStore for + * use-after-destroy crash-safety — Static.c destroys the inner pool + * slots outside the outer FreeIfInUse lock, which keeps the per-pool + * ConfigLock acquisitions sequential rather than nested. * ----------------------------------------------------------------*/ -struct SolidSyslogStore* SolidSyslogBlockStore_Create( - SolidSyslogBlockStoreStorage* storage, +void BlockStore_Initialise( + struct SolidSyslogStore* base, + struct RecordStore* recordStore, + struct BlockSequence* blockSequence, const struct SolidSyslogBlockStoreConfig* config ) { - struct SolidSyslogBlockStore* self = BlockStore_SelfFromStorage(storage); - *self = DEFAULT_INSTANCE; - - RecordStore_Init(&self->RecordStore, BlockStore_ResolveSecurityPolicy(config->SecurityPolicy)); - - struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config, &self->RecordStore); - BlockSequence_Init(&self->BlockSequence, &blockConfig); - + (void) config; /* config drove the inner Creates already; nothing left to copy. */ + struct SolidSyslogBlockStore* self = BlockStore_SelfFromBase(base); + self->RecordStore = recordStore; + self->BlockSequence = blockSequence; BlockStore_InitialiseVtable(self); - if (BlockSequence_Open(&self->BlockSequence)) + if (BlockSequence_Open(self->BlockSequence)) { BlockStore_ResumeFromExistingBlock(self); } - - return &self->Base; } -static inline struct SolidSyslogBlockStore* BlockStore_SelfFromStorage(SolidSyslogBlockStoreStorage* storage) +void BlockStore_Cleanup(struct SolidSyslogStore* base) { - return (struct SolidSyslogBlockStore*) storage; + /* Overwrite the abstract base with the shared NullStore vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. + * The inner pool pointers are still in the slot at this point — Static.c + * pulls them out before FreeIfInUse and destroys them after the outer + * lock is released. */ + *base = *SolidSyslogNullStore_Get(); } -static inline struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy( - struct SolidSyslogSecurityPolicy* configured -) -{ - struct SolidSyslogSecurityPolicy* resolved = configured; - - if ((resolved == NULL) || (resolved->IntegritySize > SOLIDSYSLOG_MAX_INTEGRITY_SIZE)) - { - resolved = SolidSyslogNullSecurityPolicy_Get(); - } - - return resolved; -} - -static inline struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( - const struct SolidSyslogBlockStoreConfig* config, - const struct RecordStore* recordStore -) +static inline struct SolidSyslogBlockStore* BlockStore_SelfFromBase(struct SolidSyslogStore* base) { - size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); - size_t maxBlockSize = (config->MaxBlockSize < minBlockSize) ? minBlockSize : config->MaxBlockSize; - - struct BlockSequenceConfig blockConfig = { - .BlockDevice = config->BlockDevice, - .MaxBlockSize = maxBlockSize, - .MaxBlocks = config->MaxBlocks, - .DiscardPolicy = config->DiscardPolicy, - .OnStoreFull = config->OnStoreFull, - .StoreFullContext = config->StoreFullContext, - .GetCapacityThreshold = config->GetCapacityThreshold, - .OnThresholdCrossed = config->OnThresholdCrossed, - .ThresholdContext = config->ThresholdContext, - }; - return blockConfig; + return (struct SolidSyslogBlockStore*) base; } static inline void BlockStore_InitialiseVtable(struct SolidSyslogBlockStore* self) @@ -132,39 +83,24 @@ static inline void BlockStore_InitialiseVtable(struct SolidSyslogBlockStore* sel static void BlockStore_ResumeFromExistingBlock(struct SolidSyslogBlockStore* self) { - struct SolidSyslogBlockDevice* device = BlockSequence_BlockDevice(&self->BlockSequence); - size_t readSequence = BlockSequence_ReadSequence(&self->BlockSequence); + struct SolidSyslogBlockDevice* device = BlockSequence_BlockDevice(self->BlockSequence); + size_t readSequence = BlockSequence_ReadSequence(self->BlockSequence); /* Bound the scan by the read block's actual size, not WritePosition. On a * multi-block resume the read block is a closed earlier block whose size * is independent of the write block's fill level. */ size_t readBlockSize = SolidSyslogBlockDevice_Size(device, readSequence); bool corrupt = false; - size_t cursor = RecordStore_FindFirstUnsent(&self->RecordStore, device, readSequence, readBlockSize, &corrupt); + size_t cursor = RecordStore_FindFirstUnsent(self->RecordStore, device, readSequence, readBlockSize, &corrupt); - BlockSequence_SetReadCursor(&self->BlockSequence, cursor); + BlockSequence_SetReadCursor(self->BlockSequence, cursor); if (corrupt) { - BlockSequence_MarkWriteBlockCorrupt(&self->BlockSequence); + BlockSequence_MarkWriteBlockCorrupt(self->BlockSequence); } } -/* ------------------------------------------------------------------ - * Destroy - * ----------------------------------------------------------------*/ - -void SolidSyslogBlockStore_Destroy(struct SolidSyslogStore* base) -{ - struct SolidSyslogBlockStore* self = BlockStore_SelfFromBase(base); - *self = DEFAULT_INSTANCE; -} - -static inline struct SolidSyslogBlockStore* BlockStore_SelfFromBase(struct SolidSyslogStore* base) -{ - return (struct SolidSyslogBlockStore*) base; -} - /* ------------------------------------------------------------------ * BlockStore_Write * ----------------------------------------------------------------*/ @@ -178,26 +114,26 @@ static bool BlockStore_Write(struct SolidSyslogStore* base, const void* data, si static bool BlockStore_StoreRecord(struct SolidSyslogBlockStore* self, const void* data, size_t size) { - size_t recordSize = RecordStore_RecordSize(&self->RecordStore, (uint16_t) size); + size_t recordSize = RecordStore_RecordSize(self->RecordStore, (uint16_t) size); bool readBlockChanged = false; bool written = false; - if (BlockSequence_PrepareForWrite(&self->BlockSequence, recordSize, &readBlockChanged)) + if (BlockSequence_PrepareForWrite(self->BlockSequence, recordSize, &readBlockChanged)) { if (readBlockChanged) { - RecordStore_ForgetLastRead(&self->RecordStore); + RecordStore_ForgetLastRead(self->RecordStore); } if (RecordStore_Append( - &self->RecordStore, - BlockSequence_BlockDevice(&self->BlockSequence), - BlockSequence_WriteSequence(&self->BlockSequence), + self->RecordStore, + BlockSequence_BlockDevice(self->BlockSequence), + BlockSequence_WriteSequence(self->BlockSequence), data, size )) { - BlockSequence_NoteRecordWritten(&self->BlockSequence, recordSize); + BlockSequence_NoteRecordWritten(self->BlockSequence, recordSize); written = true; } } @@ -211,22 +147,22 @@ static bool BlockStore_StoreRecord(struct SolidSyslogBlockStore* self, const voi static bool BlockStore_HasUnsent(struct SolidSyslogStore* base) { - return BlockSequence_HasUnsent(&BlockStore_SelfFromBase(base)->BlockSequence); + return BlockSequence_HasUnsent(BlockStore_SelfFromBase(base)->BlockSequence); } static bool BlockStore_IsHalted(struct SolidSyslogStore* base) { - return BlockSequence_IsHalted(&BlockStore_SelfFromBase(base)->BlockSequence); + return BlockSequence_IsHalted(BlockStore_SelfFromBase(base)->BlockSequence); } static size_t BlockStore_GetTotalBytes(struct SolidSyslogStore* base) { - return BlockSequence_TotalBytes(&BlockStore_SelfFromBase(base)->BlockSequence); + return BlockSequence_TotalBytes(BlockStore_SelfFromBase(base)->BlockSequence); } static size_t BlockStore_GetUsedBytes(struct SolidSyslogStore* base) { - return BlockSequence_UsedBytes(&BlockStore_SelfFromBase(base)->BlockSequence); + return BlockSequence_UsedBytes(BlockStore_SelfFromBase(base)->BlockSequence); } /* BlockStore retains records — a BlockStore_Write rejection here is the discard @@ -250,14 +186,14 @@ static bool BlockStore_ReadNextUnsent(struct SolidSyslogStore* base, void* data, bool read = false; *bytesRead = 0; - if (BlockSequence_HasUnsent(&self->BlockSequence)) + if (BlockSequence_HasUnsent(self->BlockSequence)) { read = BlockStore_ReadCurrent(self, data, maxSize, bytesRead); - while (!read && BlockSequence_ReadIsBehindWrite(&self->BlockSequence)) + while (!read && BlockSequence_ReadIsBehindWrite(self->BlockSequence)) { - BlockSequence_AdvanceToNextReadBlock(&self->BlockSequence); - RecordStore_ForgetLastRead(&self->RecordStore); + BlockSequence_AdvanceToNextReadBlock(self->BlockSequence); + RecordStore_ForgetLastRead(self->RecordStore); read = BlockStore_ReadCurrent(self, data, maxSize, bytesRead); } } @@ -268,10 +204,10 @@ static bool BlockStore_ReadNextUnsent(struct SolidSyslogStore* base, void* data, static bool BlockStore_ReadCurrent(struct SolidSyslogBlockStore* self, void* data, size_t maxSize, size_t* bytesRead) { return RecordStore_Read( - &self->RecordStore, - BlockSequence_BlockDevice(&self->BlockSequence), - BlockSequence_ReadSequence(&self->BlockSequence), - BlockSequence_ReadCursor(&self->BlockSequence), + self->RecordStore, + BlockSequence_BlockDevice(self->BlockSequence), + BlockSequence_ReadSequence(self->BlockSequence), + BlockSequence_ReadCursor(self->BlockSequence), data, maxSize, bytesRead @@ -287,20 +223,16 @@ static void BlockStore_MarkSent(struct SolidSyslogStore* base) struct SolidSyslogBlockStore* self = BlockStore_SelfFromBase(base); size_t nextCursor = 0; - if (RecordStore_MarkLastReadAsSent( - &self->RecordStore, - BlockSequence_BlockDevice(&self->BlockSequence), - &nextCursor - )) + if (RecordStore_MarkLastReadAsSent(self->RecordStore, BlockSequence_BlockDevice(self->BlockSequence), &nextCursor)) { - BlockSequence_SetReadCursor(&self->BlockSequence, nextCursor); + BlockSequence_SetReadCursor(self->BlockSequence, nextCursor); bool readBlockChanged = false; - BlockSequence_DisposeReadBlockIfDrained(&self->BlockSequence, &readBlockChanged); + BlockSequence_DisposeReadBlockIfDrained(self->BlockSequence, &readBlockChanged); if (readBlockChanged) { - RecordStore_ForgetLastRead(&self->RecordStore); + RecordStore_ForgetLastRead(self->RecordStore); } } } diff --git a/Core/Source/SolidSyslogBlockStorePrivate.h b/Core/Source/SolidSyslogBlockStorePrivate.h new file mode 100644 index 00000000..b17d8f50 --- /dev/null +++ b/Core/Source/SolidSyslogBlockStorePrivate.h @@ -0,0 +1,29 @@ +#ifndef SOLIDSYSLOGBLOCKSTOREPRIVATE_H +#define SOLIDSYSLOGBLOCKSTOREPRIVATE_H + +#include "SolidSyslogStoreDefinition.h" + +struct RecordStore; +struct BlockSequence; +struct SolidSyslogBlockStoreConfig; + +struct SolidSyslogBlockStore +{ + struct SolidSyslogStore Base; + struct RecordStore* RecordStore; + struct BlockSequence* BlockSequence; +}; + +/* _Initialise wires the vtable + composes the inner pool slots that the + * caller already acquired. The caller (Static.c) acquires the inner slots + * itself so it can route to NullStore_Get() if either Create returns NULL + * without ever having to undo a partial _Initialise. */ +void BlockStore_Initialise( + struct SolidSyslogStore* base, + struct RecordStore* recordStore, + struct BlockSequence* blockSequence, + const struct SolidSyslogBlockStoreConfig* config +); +void BlockStore_Cleanup(struct SolidSyslogStore* base); + +#endif /* SOLIDSYSLOGBLOCKSTOREPRIVATE_H */ diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c new file mode 100644 index 00000000..ef0988a7 --- /dev/null +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -0,0 +1,155 @@ +#include "SolidSyslogBlockStore.h" + +#include +#include + +#include "BlockSequencePrivate.h" +#include "RecordStorePrivate.h" +#include "SolidSyslogBlockStorePrivate.h" +#include "SolidSyslogError.h" +#include "SolidSyslogErrorMessages.h" +#include "SolidSyslogNullSecurityPolicy.h" +#include "SolidSyslogNullStore.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogSecurityPolicyDefinition.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogStore; + +static size_t BlockStore_IndexFromHandle(const struct SolidSyslogStore* base); +static void BlockStore_CleanupAtIndex(size_t index, void* context); +static struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy(struct SolidSyslogSecurityPolicy* configured); +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( + const struct SolidSyslogBlockStoreConfig* config, + const struct RecordStore* recordStore +); + +static bool BlockStore_InUse[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct SolidSyslogBlockStore BlockStore_Pool[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE]; +static struct SolidSyslogPoolAllocator BlockStore_Allocator = {BlockStore_InUse, SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE}; + +struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBlockStoreConfig* config) +{ + struct SolidSyslogStore* result = SolidSyslogNullStore_Get(); + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&BlockStore_Allocator); + + if (SolidSyslogPoolAllocator_IndexIsValid(&BlockStore_Allocator, index)) + { + struct SolidSyslogSecurityPolicy* policy = BlockStore_ResolveSecurityPolicy(config->SecurityPolicy); + struct RecordStore* recordStore = RecordStore_Create(policy); + + if (recordStore != NULL) + { + struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config, recordStore); + struct BlockSequence* blockSequence = BlockSequence_Create(&blockConfig); + + if (blockSequence != NULL) + { + BlockStore_Initialise(&BlockStore_Pool[index].Base, recordStore, blockSequence, config); + result = &BlockStore_Pool[index].Base; + } + else + { + RecordStore_Destroy(recordStore); + SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); + } + } + else + { + SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); + } + } + + if (result == SolidSyslogNullStore_Get()) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_BLOCKSTORE_POOL_EXHAUSTED); + } + + return result; +} + +void SolidSyslogBlockStore_Destroy(struct SolidSyslogStore* base) +{ + size_t index = BlockStore_IndexFromHandle(base); + bool released = false; + + if (SolidSyslogPoolAllocator_IndexIsValid(&BlockStore_Allocator, index)) + { + /* Pull the inner pool pointers out of the slot before FreeIfInUse runs + * the BlockStore_Cleanup callback, because Cleanup overwrites the + * abstract base with the NullStore vtable but the derived RecordStore / + * BlockSequence pointers stay in the slot. After the outer FreeIfInUse + * releases the ConfigLock we destroy the inner slots — keeps each pool's + * lock acquisition sequential rather than nested. */ + struct RecordStore* recordStore = BlockStore_Pool[index].RecordStore; + struct BlockSequence* blockSequence = BlockStore_Pool[index].BlockSequence; + + released = SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, BlockStore_CleanupAtIndex, NULL); + + if (released) + { + BlockSequence_Destroy(blockSequence); + RecordStore_Destroy(recordStore); + } + } + + if (!released) + { + SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_BLOCKSTORE_UNKNOWN_DESTROY); + } +} + +static size_t BlockStore_IndexFromHandle(const struct SolidSyslogStore* base) +{ + size_t result = SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE; poolIndex++) + { + if (base == &BlockStore_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static void BlockStore_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + BlockStore_Cleanup(&BlockStore_Pool[index].Base); +} + +static struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy(struct SolidSyslogSecurityPolicy* configured) +{ + struct SolidSyslogSecurityPolicy* resolved = configured; + + if ((resolved == NULL) || (resolved->IntegritySize > SOLIDSYSLOG_MAX_INTEGRITY_SIZE)) + { + resolved = SolidSyslogNullSecurityPolicy_Get(); + } + + return resolved; +} + +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( + const struct SolidSyslogBlockStoreConfig* config, + const struct RecordStore* recordStore +) +{ + size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); + size_t maxBlockSize = (config->MaxBlockSize < minBlockSize) ? minBlockSize : config->MaxBlockSize; + + struct BlockSequenceConfig blockConfig = { + .BlockDevice = config->BlockDevice, + .MaxBlockSize = maxBlockSize, + .MaxBlocks = config->MaxBlocks, + .DiscardPolicy = config->DiscardPolicy, + .OnStoreFull = config->OnStoreFull, + .StoreFullContext = config->StoreFullContext, + .GetCapacityThreshold = config->GetCapacityThreshold, + .OnThresholdCrossed = config->OnThresholdCrossed, + .ThresholdContext = config->ThresholdContext, + }; + return blockConfig; +} diff --git a/Core/Source/SolidSyslogErrorMessages.h b/Core/Source/SolidSyslogErrorMessages.h index 21cb2163..48150c1f 100644 --- a/Core/Source/SolidSyslogErrorMessages.h +++ b/Core/Source/SolidSyslogErrorMessages.h @@ -59,5 +59,9 @@ "SolidSyslogFileBlockDevice_Create pool exhausted; returning fallback block device" #define SOLIDSYSLOG_ERROR_MSG_FILEBLOCKDEVICE_UNKNOWN_DESTROY \ "SolidSyslogFileBlockDevice_Destroy called with a handle not issued by this pool" +#define SOLIDSYSLOG_ERROR_MSG_BLOCKSTORE_POOL_EXHAUSTED \ + "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" #endif /* SOLIDSYSLOGERRORMESSAGES_H */ diff --git a/DEVLOG.md b/DEVLOG.md index f2129a06..7f9d4a4c 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,111 @@ # Dev Log +## 2026-05-19 — S11.05 part B: BlockStore composition onto PoolAllocator + +Closes S11.05. The composition migration that part A deferred — RecordStore +and BlockSequence are TU-internal sub-components BlockStore composes by +value today, so part A couldn't touch them without rewriting BlockStore's +slot layout. This PR does the three steps in sequence on the same branch: + +- `0602362` — RecordStore onto PoolAllocator. Rename `RecordStore.h` -> + `RecordStorePrivate.h`; `_Init` -> `_Initialise`; new no-op `_Cleanup`; + new `RecordStoreStatic.c` with the pool + public `_Create` / `_Destroy`. + Pool sized by a new `SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE` tunable + (default 1, floor 1). TU-internal classes return NULL on exhaustion — + no shared null-object — because the only consumer is BlockStore_Create + which handles the NULL. +- `9b9e957` — BlockSequence onto PoolAllocator. Same shape as RecordStore. + Pool sized 1:1 off the same tunable — no separate symbol. Tests update: + `BlockSequenceTest.cpp` switches from stack-allocated `struct + BlockSequence sequence = {}` to a pointer obtained via `BlockSequence_Create` + with paired `_Destroy` in teardown. +- `d5f0399` — BlockStore composition rewrite. Slot stops embedding + RecordStore + BlockSequence by value, holds pointers into their pools + instead. Public header drops `SolidSyslogBlockStoreStorage` typedef + + the `SOLIDSYSLOG_BLOCK_STORE_STORAGE_SIZE` enum; `_Create` loses its + storage parameter. Cleanup is a pure NullStore vtable swap; Static.c + destroys the inner pool slots after the outer FreeIfInUse releases + the ConfigLock to keep each pool's lock acquisition sequential rather + than nested. + +### Decisions + +- **TU-internal Private.h is test-visible.** The existing memory + `project_e11_three_tu_split` said "tests must not include Private.h" + but that rule was written for classes with a public `Interface/` + header. RecordStore and BlockSequence have no public header — their + accessor signatures live nowhere else, so tests must include their + Private.h. Clarified the memory inline on this branch: the rule + applies only to classes that *have* a public header. TU-internal + classes' Private.h stays test-visible; what changes after migration + is that the test no longer stack-allocates the struct, it goes + through `Class_Create`/`_Destroy`. + +- **TU-internal `_Create` returns NULL on exhaustion; `_Destroy(NULL)` + is silent.** No shared null-object pattern. The only legitimate way + for a caller to hold a NULL handle is a failed Create, and the + consumer's own error reporting (BlockStore_Create's + `BLOCKSTORE_POOL_EXHAUSTED` error) covers that. A second + Destroy-time warning would be redundant noise on the same bad-setup. + +- **BlockStore_Cleanup is pure (vtable swap only); Static.c destroys + the inner pool slots outside the outer lock.** The briefing + originally described "BlockStore_Cleanup calls their _Destroy in + reverse." That works for the embedding-by-value shape but here the + outer FreeIfInUse holds the shared `SolidSyslog_LockConfig` mutex + while calling Cleanup, and re-entering it via the inner pools' + FreeIfInUse would deadlock on a non-recursive integrator lock + (POSIX pthread_mutex default, for example). Static.c pulls the + inner pointers out of the slot before calling FreeIfInUse on the + outer pool, then destroys them after the outer lock is released. + Each pool's lock acquisition is sequential rather than nested, and + the "BlockSequence first then RecordStore" reverse-order intent + from the briefing is preserved at the Static.c orchestration layer. + +- **Static.c orchestrates the security-policy resolution and + block-config build.** `BlockStore_ResolveSecurityPolicy` and + `BlockStore_BuildBlockSequenceConfig` move out of the algorithm TU + into Static.c, because Static.c is now the orchestrator that calls + `RecordStore_Create(policy)` then `BlockSequence_Create(&blockConfig)`. + Keeps the "Static.c is the only TU that talks to PoolAllocator and + emits Error" rule intact. + +- **Existing `DoubleDestroyDoesNotCrash` test now exercises the + UNKNOWN_DESTROY warning path.** Under the old singleton shape, the + second Destroy was a no-op (`*self = DEFAULT_INSTANCE;` on + already-zeroed memory). Under the pool shape, the second Destroy + finds the slot's index but FreeIfInUse returns false (not in use) + and emits the BLOCKSTORE_UNKNOWN_DESTROY warning at WARNING + severity. The test's assertion is unchanged ("does not crash") — + the behaviour shift is documented here so the noisy warning isn't + surprising on review. + +- **Pool-size override validated at 3.** Full suite green at + `SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE=3` via the + `SOLIDSYSLOG_USER_TUNABLES_FILE` override mechanism. Bumping the + single symbol grew all three pools (BlockStore, RecordStore, + BlockSequence) — the 1:1 invariant the briefing called out as the + thing to prove holds. 1188 tests total (+6 from part A: 4 + pool-contract tests in `RecordStorePoolTest.cpp` and + `BlockSequencePoolTest.cpp`, 2 in the new + `SolidSyslogBlockStorePool` group inside `SolidSyslogBlockStoreTest.cpp`). + +### Deferred + +- **DEVLOG cadence on multi-commit branches.** Part A's DEVLOG was + added as a separate `docs:` commit before opening the PR. Part B + adopts the same shape — this entry is the only thing this commit + changes. Keeping it that way means each functional commit on the + branch stays narrowly scoped to its migration and reviewers can + read the rationale here without trawling individual commit bodies. + +- **E11 epic status.** With BlockStore migrated, every class in the + E11 sweep is on PoolAllocator. The follow-up dynamic-allocation + epic (per `project_allocation_epic` memory) is the natural next + one to schedule — adds a heap-allocated `ClassDynamic.c` TU per + class behind a CMake strategy flag, public Create/Destroy names + unchanged. + ## 2026-05-19 — S11.05 part A: public storage-cast classes onto PoolAllocator + shared null objects This is the first half of S11.05. The story body opened by listing diff --git a/Tests/BlockSequencePoolTest.cpp b/Tests/BlockSequencePoolTest.cpp new file mode 100644 index 00000000..d06b05c5 --- /dev/null +++ b/Tests/BlockSequencePoolTest.cpp @@ -0,0 +1,78 @@ +#include "CppUTest/TestHarness.h" + +extern "C" +{ +#include "BlockSequencePrivate.h" +#include "SolidSyslogTunables.h" +} + +// clang-format off +TEST_GROUP(BlockSequencePool) +{ + /* BlockSequence_Initialise only copies the config fields into the slot + * (no dereferences); a zero-filled config is enough to exercise the + * pool lifecycle without standing up a fake BlockDevice. */ + struct BlockSequenceConfig config = {}; + struct BlockSequence* pooled[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE] = {}; + struct BlockSequence* overflow = nullptr; + + void teardown() override + { + for (auto*& slot : pooled) + { + BlockSequence_Destroy(slot); + slot = nullptr; + } + BlockSequence_Destroy(overflow); + overflow = nullptr; + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = BlockSequence_Create(&config); + } + } +}; + +// clang-format on + +TEST(BlockSequencePool, CreateReturnsNonNullForFreshPool) +{ + struct BlockSequence* slot = BlockSequence_Create(&config); + CHECK_TEXT(slot != nullptr, "first Create on empty pool returned NULL"); + BlockSequence_Destroy(slot); +} + +TEST(BlockSequencePool, FillingPoolThenOverflowReturnsNull) +{ + FillPool(); + + overflow = BlockSequence_Create(&config); + + CHECK_TEXT(overflow == nullptr, "exhausted pool should return NULL, not a handle"); + for (auto* slot : pooled) + { + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); + } +} + +TEST(BlockSequencePool, DestroyReleasesSlotForReuse) +{ + FillPool(); + + BlockSequence_Destroy(pooled[0]); + pooled[0] = BlockSequence_Create(&config); + + CHECK_TEXT(pooled[0] != nullptr, "reacquire after Destroy returned NULL"); +} + +TEST(BlockSequencePool, DestroyOfNullIsSilentNoop) +{ + /* TU-internal classes return NULL on exhaustion (no shared null-object). + * The only legitimate path to a NULL handle is a failed Create, and the + * consumer's own error reporting covers that. _Destroy(NULL) must therefore + * be a silent no-op. */ + BlockSequence_Destroy(nullptr); +} diff --git a/Tests/BlockSequenceTest.cpp b/Tests/BlockSequenceTest.cpp index ee25c9ec..b85ade15 100644 --- a/Tests/BlockSequenceTest.cpp +++ b/Tests/BlockSequenceTest.cpp @@ -9,7 +9,7 @@ extern "C" { -#include "BlockSequence.h" +#include "BlockSequencePrivate.h" } #include "SolidSyslogBlockDeviceDefinition.h" @@ -145,7 +145,7 @@ TEST_GROUP(BlockSequenceScan) { ScanFake fakeDevice = {}; std::set existing; - struct BlockSequence sequence = {}; + struct BlockSequence* sequence = nullptr; void setup() override { @@ -164,7 +164,13 @@ TEST_GROUP(BlockSequenceScan) config.MaxBlockSize = 1000; config.MaxBlocks = 99; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST; - BlockSequence_Init(&sequence, &config); + // cppcheck-suppress unreadVariable -- read across TEST_GROUP methods; cppcheck does not model CppUTest macros + sequence = BlockSequence_Create(&config); + } + + void teardown() override + { + BlockSequence_Destroy(sequence); } }; @@ -172,25 +178,25 @@ TEST_GROUP(BlockSequenceScan) TEST(BlockSequenceScan, ColdStartAcquiresBlockZero) { - CHECK_TRUE(BlockSequence_Open(&sequence)); - LONGS_EQUAL(0, BlockSequence_ReadSequence(&sequence)); - LONGS_EQUAL(0, BlockSequence_WriteSequence(&sequence)); + CHECK_TRUE(BlockSequence_Open(sequence)); + LONGS_EQUAL(0, BlockSequence_ReadSequence(sequence)); + LONGS_EQUAL(0, BlockSequence_WriteSequence(sequence)); } TEST(BlockSequenceScan, ResumesContiguousLinearRange) { existing = {2, 3, 4}; - CHECK_TRUE(BlockSequence_Open(&sequence)); - LONGS_EQUAL(2, BlockSequence_ReadSequence(&sequence)); - LONGS_EQUAL(4, BlockSequence_WriteSequence(&sequence)); + CHECK_TRUE(BlockSequence_Open(sequence)); + LONGS_EQUAL(2, BlockSequence_ReadSequence(sequence)); + LONGS_EQUAL(4, BlockSequence_WriteSequence(sequence)); } TEST(BlockSequenceScan, ResumesAtZeroWhenOnlyBlockZeroExists) { existing = {0}; - CHECK_TRUE(BlockSequence_Open(&sequence)); - LONGS_EQUAL(0, BlockSequence_ReadSequence(&sequence)); - LONGS_EQUAL(0, BlockSequence_WriteSequence(&sequence)); + CHECK_TRUE(BlockSequence_Open(sequence)); + LONGS_EQUAL(0, BlockSequence_ReadSequence(sequence)); + LONGS_EQUAL(0, BlockSequence_WriteSequence(sequence)); } /* After enough rotations, the on-disk block range straddles the 99 -> 00 @@ -201,17 +207,17 @@ TEST(BlockSequenceScan, ResumesAtZeroWhenOnlyBlockZeroExists) TEST(BlockSequenceScan, ResumesWrappedSequenceRangeCorrectly) { existing = {98, 99, 0, 1}; - CHECK_TRUE(BlockSequence_Open(&sequence)); - LONGS_EQUAL(98, BlockSequence_ReadSequence(&sequence)); - LONGS_EQUAL(1, BlockSequence_WriteSequence(&sequence)); + CHECK_TRUE(BlockSequence_Open(sequence)); + LONGS_EQUAL(98, BlockSequence_ReadSequence(sequence)); + LONGS_EQUAL(1, BlockSequence_WriteSequence(sequence)); } TEST(BlockSequenceScan, ResumesWrappedSingleBlockAtBoundary) { existing = {99, 0}; - CHECK_TRUE(BlockSequence_Open(&sequence)); - LONGS_EQUAL(99, BlockSequence_ReadSequence(&sequence)); - LONGS_EQUAL(0, BlockSequence_WriteSequence(&sequence)); + CHECK_TRUE(BlockSequence_Open(sequence)); + LONGS_EQUAL(99, BlockSequence_ReadSequence(sequence)); + LONGS_EQUAL(0, BlockSequence_WriteSequence(sequence)); } namespace @@ -231,7 +237,7 @@ TEST_GROUP(BlockSequenceRotation) std::set existing; std::vector calls; std::map sizes; - struct BlockSequence sequence = {}; + struct BlockSequence* sequence = nullptr; void setup() override { @@ -242,7 +248,9 @@ TEST_GROUP(BlockSequenceRotation) fakeDevice.Base.Append = FakeAppend; fakeDevice.Base.WriteAt = FakeWriteAt; fakeDevice.Base.Size = FakeSize; + // cppcheck-suppress unreadVariable -- read indirectly via FakeExists; cppcheck does not model the function-pointer indirection fakeDevice.existing = &existing; + // cppcheck-suppress unreadVariable -- read indirectly via RecordCall; cppcheck does not model the function-pointer indirection fakeDevice.calls = &calls; // cppcheck-suppress unreadVariable -- read indirectly via FakeSize; cppcheck does not model the function-pointer indirection fakeDevice.sizes = &sizes; @@ -252,17 +260,22 @@ TEST_GROUP(BlockSequenceRotation) config.MaxBlockSize = ROTATION_BLOCK_SIZE; config.MaxBlocks = 99; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST; - BlockSequence_Init(&sequence, &config); + sequence = BlockSequence_Create(&config); - BlockSequence_Open(&sequence); /* cold start: Acquire(0) */ + BlockSequence_Open(sequence); /* cold start: Acquire(0) */ /* Simulate one record's worth of data in block 0 — production rotation * never seals an empty block, and the dispose-on-empty trigger uses * device.Size to decide drained-ness. */ - BlockSequence_NoteRecordWritten(&sequence, SIMULATED_RECORD_SIZE); - sizes[BlockSequence_WriteSequence(&sequence)] = SIMULATED_RECORD_SIZE; + BlockSequence_NoteRecordWritten(sequence, SIMULATED_RECORD_SIZE); + sizes[BlockSequence_WriteSequence(sequence)] = SIMULATED_RECORD_SIZE; calls.clear(); } + void teardown() override + { + BlockSequence_Destroy(sequence); + } + [[nodiscard]] bool DisposePrecedesAcquire(size_t blockIndex) const { std::ptrdiff_t disposeAt = -1; @@ -281,10 +294,10 @@ TEST_GROUP(BlockSequenceRotation) return (disposeAt >= 0) && (acquireAt >= 0) && (disposeAt < acquireAt); } - void ForceRotation() + void ForceRotation() const { bool readBlockChanged = false; - BlockSequence_PrepareForWrite(&sequence, ROTATION_BLOCK_SIZE + 1, &readBlockChanged); + BlockSequence_PrepareForWrite(sequence, ROTATION_BLOCK_SIZE + 1, &readBlockChanged); } }; @@ -321,7 +334,7 @@ TEST(BlockSequenceRotation, RotationFailsWhenStaleBlockDisposeFails) fakeDevice.failNextDispose = true; bool readBlockChanged = false; - bool acquired = BlockSequence_PrepareForWrite(&sequence, ROTATION_BLOCK_SIZE + 1, &readBlockChanged); + bool acquired = BlockSequence_PrepareForWrite(sequence, ROTATION_BLOCK_SIZE + 1, &readBlockChanged); CHECK_FALSE(acquired); for (const auto& call : calls) diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index ad551178..84155c17 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -61,6 +61,8 @@ set(TEST_SOURCES SolidSyslogBlockDeviceTest.cpp SolidSyslogFileBlockDeviceTest.cpp BlockSequenceTest.cpp + BlockSequencePoolTest.cpp + RecordStorePoolTest.cpp main.cpp ) diff --git a/Tests/RecordStorePoolTest.cpp b/Tests/RecordStorePoolTest.cpp new file mode 100644 index 00000000..7836b6f9 --- /dev/null +++ b/Tests/RecordStorePoolTest.cpp @@ -0,0 +1,81 @@ +#include "CppUTest/TestHarness.h" + +extern "C" +{ +#include "RecordStorePrivate.h" +#include "SolidSyslogNullSecurityPolicy.h" +#include "SolidSyslogTunables.h" +} + +// clang-format off +TEST_GROUP(RecordStorePool) +{ + struct SolidSyslogSecurityPolicy* policy = nullptr; + struct RecordStore* pooled[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE] = {}; + struct RecordStore* overflow = nullptr; + + void setup() override + { + policy = SolidSyslogNullSecurityPolicy_Get(); + } + + void teardown() override + { + for (auto*& slot : pooled) + { + RecordStore_Destroy(slot); + slot = nullptr; + } + RecordStore_Destroy(overflow); + overflow = nullptr; + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = RecordStore_Create(policy); + } + } +}; + +// clang-format on + +TEST(RecordStorePool, CreateReturnsNonNullForFreshPool) +{ + struct RecordStore* slot = RecordStore_Create(policy); + CHECK_TEXT(slot != nullptr, "first Create on empty pool returned NULL"); + RecordStore_Destroy(slot); +} + +TEST(RecordStorePool, FillingPoolThenOverflowReturnsNull) +{ + FillPool(); + + overflow = RecordStore_Create(policy); + + CHECK_TEXT(overflow == nullptr, "exhausted pool should return NULL, not a handle"); + for (auto* slot : pooled) + { + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); + } +} + +TEST(RecordStorePool, DestroyReleasesSlotForReuse) +{ + FillPool(); + + RecordStore_Destroy(pooled[0]); + pooled[0] = RecordStore_Create(policy); + + CHECK_TEXT(pooled[0] != nullptr, "reacquire after Destroy returned NULL"); +} + +TEST(RecordStorePool, DestroyOfNullIsSilentNoop) +{ + /* TU-internal classes return NULL on exhaustion (no shared null-object). + * The only legitimate path to a NULL handle is a failed Create, and the + * consumer's own error reporting covers that. _Destroy(NULL) must therefore + * be a silent no-op. */ + RecordStore_Destroy(nullptr); +} diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 463a8f9b..6b213b51 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -137,7 +137,6 @@ TEST_BASE(DrainTestFixtureBase) // clang-format off TEST_GROUP_BASE(BlockStoreDrainOrdering, DrainTestFixtureBase) { - SolidSyslogBlockStoreStorage storeStorage = {}; struct SolidSyslogStore* store = nullptr; void setup() override @@ -162,7 +161,7 @@ TEST_GROUP_BASE(BlockStoreDrainOrdering, DrainTestFixtureBase) config.MaxBlocks = cfg.MaxBlocks; config.DiscardPolicy = cfg.DiscardPolicy; config.SecurityPolicy = policy; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } // Writes one record whose payload encodes sequenceId in the first 4 @@ -222,7 +221,6 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) * until Service drains them (unlike BufferFake which only keeps * the last one). */ uint8_t bufferRing[SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(16)] = {}; - SolidSyslogBlockStoreStorage storeStorage = {}; struct SolidSyslogStore* store = nullptr; struct SolidSyslogMutex* mutex = nullptr; struct SolidSyslogBuffer* buffer = nullptr; @@ -258,7 +256,7 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) storeCfg.MaxBlocks = cfg.MaxBlocks; storeCfg.DiscardPolicy = cfg.DiscardPolicy; storeCfg.SecurityPolicy = policy; - store = SolidSyslogBlockStore_Create(&storeStorage, &storeCfg); + store = SolidSyslogBlockStore_Create(&storeCfg); struct SolidSyslogConfig sysCfg = {}; sysCfg.Buffer = buffer; diff --git a/Tests/SolidSyslogBlockStorePosixTest.cpp b/Tests/SolidSyslogBlockStorePosixTest.cpp index 78d9ebb1..4b6a7607 100644 --- a/Tests/SolidSyslogBlockStorePosixTest.cpp +++ b/Tests/SolidSyslogBlockStorePosixTest.cpp @@ -12,8 +12,6 @@ static const char* const TEST_PATH_PREFIX = "/tmp/test_posix_store"; -static SolidSyslogBlockStoreStorage storeStorage = {}; - static void CleanStoreFiles() { glob_t results = {}; @@ -70,7 +68,7 @@ TEST_GROUP(SolidSyslogBlockStorePosix) config.MaxBlocks = maxBlocks; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void WriteMaxMsg() diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index 46217606..6fa6e745 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -7,6 +7,7 @@ #include "SolidSyslogCrc16Policy.h" #include "SolidSyslogFile.h" #include "SolidSyslogFileBlockDevice.h" +#include "SolidSyslogNullStore.h" #include "SolidSyslogSecurityPolicyDefinition.h" #include "SolidSyslogStore.h" #include "SolidSyslogTunables.h" @@ -50,10 +51,6 @@ static const struct SolidSyslogBlockStoreConfig DEFAULT_CONFIG = { nullptr, }; -/* Single backing slab reused across tests — tests run serially and Destroy - * resets the store, so one storage instance is sufficient. */ -static SolidSyslogBlockStoreStorage storeStorage = {}; - static struct SolidSyslogBlockStoreConfig MakeConfig(struct SolidSyslogBlockDevice* device) { struct SolidSyslogBlockStoreConfig config = DEFAULT_CONFIG; @@ -105,7 +102,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStore, BlockDeviceTestBase) setupBlockDeviceFakes(); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void teardown() override @@ -334,13 +331,13 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreResume, BlockDeviceTestBase) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); // cppcheck-suppress unreadVariable -- used by WriteMessages/DrainMessages; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); WriteMessages(total); DrainMessages(markedSent); SolidSyslogBlockStore_Destroy(store); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void WriteMessages(int count) const @@ -452,7 +449,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreDestroy, BlockDeviceTestBase) TEST(SolidSyslogBlockStoreDestroy, DoubleDestroyDoesNotCrash) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - struct SolidSyslogStore* store = SolidSyslogBlockStore_Create(&storeStorage, &config); + struct SolidSyslogStore* store = SolidSyslogBlockStore_Create(&config); SolidSyslogBlockStore_Destroy(store); SolidSyslogBlockStore_Destroy(store); } @@ -482,7 +479,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.MaxBlocks = maxBlocks; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void CreateWithMaxBlockSize(size_t maxBlockSize) @@ -490,7 +487,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.MaxBlockSize = maxBlockSize; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void CreateWithPathPrefix(const char* prefix) @@ -500,7 +497,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) device = SolidSyslogFileBlockDevice_Create(file, prefix); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void VerifyWriteAndReadBack() const @@ -579,7 +576,7 @@ TEST(SolidSyslogBlockStoreConfig, NullSecurityPolicyDefaultsToNoOp) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.SecurityPolicy = nullptr; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); VerifyWriteAndReadBack(); } @@ -592,7 +589,7 @@ TEST(SolidSyslogBlockStoreConfig, OversizedSecurityPolicyLeavesNoIntegrityGap) }; struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.SecurityPolicy = &oversizedPolicy; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); const char body[] = "HELLO WORLD"; const size_t bodyLen = sizeof(body) - 1; @@ -632,7 +629,7 @@ TEST(SolidSyslogBlockStoreErrors, OpenFailureStillReturnsNonNull) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); FileFake_FailNextOpen(file); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); CHECK_TRUE(store != nullptr); } @@ -642,14 +639,14 @@ TEST(SolidSyslogBlockStoreErrors, TransientOpenFailureRecoversOnNextWrite) * on the next operation that needs the file. */ struct SolidSyslogBlockStoreConfig config = MakeConfig(device); FileFake_FailNextOpen(file); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); CHECK_TRUE(SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN)); } TEST(SolidSyslogBlockStoreErrors, WriteReturnsFalseOnWriteFailure) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); FileFake_FailNextWrite(file); CHECK_FALSE(SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN)); } @@ -657,7 +654,7 @@ TEST(SolidSyslogBlockStoreErrors, WriteReturnsFalseOnWriteFailure) TEST(SolidSyslogBlockStoreErrors, ReadReturnsFalseOnReadFailure) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN); FileFake_FailNextRead(file); @@ -670,7 +667,7 @@ TEST(SolidSyslogBlockStoreErrors, ReadReturnsFalseOnReadFailure) TEST(SolidSyslogBlockStoreErrors, MarkSentDoesNotAdvanceWhenWriteFails) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN); char buf[TEST_BUF_SIZE]; @@ -720,7 +717,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreRotation, BlockDeviceTestBase) config.OnStoreFull = onStoreFull; config.StoreFullContext = storeFullContext; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void WriteMaxMsg() @@ -1430,7 +1427,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreIntegrity, BlockDeviceTestBase) config.BlockDevice = device; config.SecurityPolicy = &spyPolicy; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void teardown() override @@ -1531,7 +1528,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCorruption, BlockDeviceTestBase) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } }; @@ -1574,7 +1571,7 @@ TEST(SolidSyslogBlockStoreCorruption, ValidRecordBeforeCorruptionIsReadable) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.SecurityPolicy = SolidSyslogCrc16Policy_Create(); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, "first", 5); SolidSyslogStore_Write(store, "second", 6); SolidSyslogBlockStore_Destroy(store); @@ -1597,7 +1594,7 @@ TEST(SolidSyslogBlockStoreCorruption, ValidRecordBeforeCorruptionIsReadable) SolidSyslogFile_Close(file); /* Re-open: first record is valid, second is corrupt */ - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); char buf[TEST_BUF_SIZE] = {}; size_t bytesRead = 0; @@ -1610,7 +1607,7 @@ TEST(SolidSyslogBlockStoreCorruption, IntegrityFailureReadReturnsFalse) { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.SecurityPolicy = SolidSyslogCrc16Policy_Create(); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN); SolidSyslogBlockStore_Destroy(store); @@ -1621,7 +1618,7 @@ TEST(SolidSyslogBlockStoreCorruption, IntegrityFailureReadReturnsFalse) SolidSyslogFile_Write(file, &corrupt, 1); SolidSyslogFile_Close(file); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); char buf[TEST_BUF_SIZE]; size_t bytesRead = 0; CHECK_FALSE(SolidSyslogStore_ReadNextUnsent(store, buf, sizeof(buf), &bytesRead)); @@ -1632,7 +1629,7 @@ TEST(SolidSyslogBlockStoreCorruption, InvalidLengthReadReturnsFalse) /* Write many records to make the file large enough that a bogus length * doesn't hit EOF — the length check must reject it explicitly */ struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); char largeMsg[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; memset(largeMsg, 'X', sizeof(largeMsg)); @@ -1647,7 +1644,7 @@ TEST(SolidSyslogBlockStoreCorruption, InvalidLengthReadReturnsFalse) SolidSyslogFile_Write(file, &badLength, 2); SolidSyslogFile_Close(file); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); char buf[TEST_BUF_SIZE]; size_t bytesRead = 0; CHECK_FALSE(SolidSyslogStore_ReadNextUnsent(store, buf, sizeof(buf), &bytesRead)); @@ -1689,7 +1686,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCorruptionRecovery, BlockDeviceTestBase) config.MaxBlocks = maxBlocks; config.SecurityPolicy = policy; // cppcheck-suppress unreadVariable -- used across TEST_GROUP methods; cppcheck does not model CppUTest macros - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void WriteMaxMsg() @@ -1780,7 +1777,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCapacity, BlockDeviceTestBase) void CreateDefault() { struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- maxBlockSize is a byte size, maxBlocks is a count; distinct semantics @@ -1791,7 +1788,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCapacity, BlockDeviceTestBase) config.MaxBlockSize = maxBlockSize; config.MaxBlocks = maxBlocks; config.DiscardPolicy = policy; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } void WriteMaxMsg() @@ -1926,7 +1923,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCapacityThreshold, BlockDeviceTestBase) config.GetCapacityThreshold = ReturnsConfiguredThreshold; config.OnThresholdCrossed = CountThresholdCrossings; thresholdReturnValue = threshold; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); } }; @@ -1973,7 +1970,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, ReArmsAfterFallingEdgeOnDiscardOlde config.OnThresholdCrossed = CountThresholdCrossings; /* Threshold sits between 3 and 4 records: 4-records crosses, 3-records is below. */ thresholdReturnValue = (3 * MAX_MSG_RECORD) + 1; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* block 0: 1 record */ SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* block 0: 2 records (full) */ @@ -2004,7 +2001,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, DoesNotFireWhenThresholdFunctionIsN struct SolidSyslogBlockStoreConfig config = MakeConfig(device); config.GetCapacityThreshold = nullptr; config.OnThresholdCrossed = CountThresholdCrossings; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN); @@ -2038,7 +2035,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, ContextIsPassedToBothCallbacks) config.OnThresholdCrossed = CaptureThresholdCallbackContext; config.ThresholdContext = &sentinel; thresholdReturnValue = TEST_DATA_LEN; - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN); @@ -2086,7 +2083,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, AtFullCapacityWithHaltThresholdFire config.OnThresholdCrossed = RecordThresholdFireOrder; /* Threshold = total: only the sticky-100% engagement on a failed Write reaches it. */ thresholdReturnValue = 2 * (MAX_MSG_RECORD + SLACK); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* block 0 partially full */ SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* rotate; block 1 partially full */ @@ -2115,7 +2112,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, StickyHundredPercentDoesNotRefireTh config.GetCapacityThreshold = ReturnsConfiguredThreshold; config.OnThresholdCrossed = CountThresholdCrossings; thresholdReturnValue = 2 * (MAX_MSG_RECORD + SLACK); - store = SolidSyslogBlockStore_Create(&storeStorage, &config); + store = SolidSyslogBlockStore_Create(&config); SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* fills block 0 partially */ SolidSyslogStore_Write(store, maxMsg, sizeof(maxMsg)); /* fills block 1 partially */ @@ -2151,7 +2148,7 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, FiresOnCreateWhenResumedUsageAboveT { { struct SolidSyslogBlockStoreConfig preConfig = MakeConfig(device); - struct SolidSyslogStore* preStore = SolidSyslogBlockStore_Create(&storeStorage, &preConfig); + struct SolidSyslogStore* preStore = SolidSyslogBlockStore_Create(&preConfig); SolidSyslogStore_Write(preStore, TEST_DATA, TEST_DATA_LEN); SolidSyslogBlockStore_Destroy(preStore); } @@ -2161,3 +2158,90 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, FiresOnCreateWhenResumedUsageAboveT CALLED_FUNCTION(CountThresholdCrossings, ONCE); } + +/* ------------------------------------------------------------------ + * Pool — prove SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE caps live instances + * and overflow resolves to the shared SolidSyslogNullStore. Generic + * pool mechanics (per-probe lock, stale-handle warning) are covered + * by SolidSyslogPoolAllocatorTest.cpp. + * ----------------------------------------------------------------*/ + +// clang-format off +TEST_GROUP_BASE(SolidSyslogBlockStorePool, BlockDeviceTestBase) +{ + struct SolidSyslogStore* pooled[SOLIDSYSLOG_BLOCK_STORE_POOL_SIZE] = {}; + struct SolidSyslogStore* overflow = nullptr; + + void setup() override + { + setupBlockDeviceFakes(); + } + + void teardown() override + { + for (auto*& slot : pooled) + { + SolidSyslogBlockStore_Destroy(slot); + slot = nullptr; + } + SolidSyslogBlockStore_Destroy(overflow); + overflow = nullptr; + teardownBlockDeviceFakes(); + } + + struct SolidSyslogStore* MakeStore() + { + struct SolidSyslogBlockStoreConfig config = MakeConfig(device); + return SolidSyslogBlockStore_Create(&config); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = MakeStore(); + } + } +}; + +// clang-format on + +TEST(SolidSyslogBlockStorePool, FillingPoolThenOverflowResolvesToNullStore) +{ + FillPool(); + + overflow = MakeStore(); + + /* Overflow resolves to the shared NullStore_Get() — distinct from every + * pool slot, and the same singleton consumers of SolidSyslogConfig.Store + * see when no store is wired. */ + CHECK_TEXT(overflow == SolidSyslogNullStore_Get(), "overflow did not resolve to NullStore"); + for (auto* slot : pooled) + { + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); + CHECK_TEXT(slot != SolidSyslogNullStore_Get(), "pool slot collided with NullStore singleton"); + CHECK_TEXT(overflow != slot, "Fallback handle collided with a pool slot"); + } +} + +TEST(SolidSyslogBlockStorePool, UseAfterDestroyIsCrashSafeViaNullStoreVtable) +{ + /* After Destroy, the slot's vtable is overwritten with NullStore's so + * Write/Read/etc are safe no-ops rather than NULL-fn-pointer crashes. + * Pin the contract that Write drops, ReadNextUnsent reports nothing, + * and the rest of the vtable doesn't crash. */ + struct SolidSyslogStore* store = MakeStore(); + pooled[0] = store; /* keep the handle live so teardown's Destroy hits the same slot — second Destroy is the + known-issued-handle case */ + SolidSyslogBlockStore_Destroy(store); + pooled[0] = nullptr; + + /* Vtable now matches NullStore — Write drops, ReadNextUnsent has nothing. */ + CHECK_FALSE(SolidSyslogStore_Write(store, TEST_DATA, TEST_DATA_LEN)); + char buf[TEST_BUF_SIZE] = {}; + size_t bytesRead = 99; + CHECK_FALSE(SolidSyslogStore_ReadNextUnsent(store, buf, sizeof(buf), &bytesRead)); + LONGS_EQUAL(0, bytesRead); + CHECK_FALSE(SolidSyslogStore_HasUnsent(store)); + SolidSyslogStore_MarkSent(store); /* no-op */ +} diff --git a/docs/NAMING.md b/docs/NAMING.md index 32fb4c21..4d1aef1b 100644 --- a/docs/NAMING.md +++ b/docs/NAMING.md @@ -322,36 +322,23 @@ static bool CircularBuffer_Read(struct SolidSyslogBuffer* base, #### The storage cast: `_SelfFromStorage` -For classes still on the caller-supplied-storage pattern (BlockStore, -the Posix/Windows/FreeRTOS mutexes and streams, …), `_Create` takes -opaque storage and re-interprets it as the concrete struct. The same -convention applies — one named `static inline` helper per class: +For classes still on the caller-supplied-storage pattern (the +Posix/Windows/FreeRTOS mutexes and streams, FatFs/TLS adapters, …), +`_Create` takes opaque storage and re-interprets it as the concrete +struct. The same convention applies — one named `static inline` helper +per class: ```c -static inline struct SolidSyslogBlockStore* -BlockStore_SelfFromStorage(SolidSyslogBlockStoreStorage* storage) +static inline struct SolidSyslogPosixMutex* +PosixMutex_SelfFromStorage(SolidSyslogPosixMutexStorage* storage) { - return (struct SolidSyslogBlockStore*) storage; + return (struct SolidSyslogPosixMutex*) storage; } ``` -`_Create` becomes: - -```c -struct SolidSyslogStore* SolidSyslogBlockStore_Create( - SolidSyslogBlockStoreStorage* storage, ... -) -{ - struct SolidSyslogBlockStore* self = BlockStore_SelfFromStorage(storage); - ... - return &self->Base; -} -``` - -Classes migrated under E11 (currently only `SolidSyslogCircularBuffer`) -no longer use this cast — their instance struct lives in a -library-internal static pool, and `_Create` returns a slot pointer -without any storage cast. +Classes migrated under E11 no longer use this cast — their instance +struct lives in a library-internal static pool, and `_Create` returns +a slot pointer without any storage cast. Helpers are named per Tier 2 (`Class_Function`, `static inline`, no `SolidSyslog` prefix). Placement follows the function-ordering rule: