From c79a5c2744034119caf31120e981863ff53041bd Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 15:58:20 +0100 Subject: [PATCH 1/7] feat: S21.04 add GetBlockSize to the BlockDevice contract Device-wide block capacity, distinct from Size(blockIndex) occupancy. NullBlockDevice returns 0. Pure addition; no caller uses it yet. Co-Authored-By: Claude Opus 4.8 (1M context) --- Core/Interface/SolidSyslogBlockDevice.h | 6 ++++++ Core/Interface/SolidSyslogBlockDeviceDefinition.h | 1 + Core/Source/SolidSyslogBlockDevice.c | 5 +++++ Core/Source/SolidSyslogNullBlockDevice.c | 8 ++++++++ Tests/SolidSyslogNullBlockDeviceTest.cpp | 5 +++++ 5 files changed, 25 insertions(+) diff --git a/Core/Interface/SolidSyslogBlockDevice.h b/Core/Interface/SolidSyslogBlockDevice.h index 1267974f..e8b598d8 100644 --- a/Core/Interface/SolidSyslogBlockDevice.h +++ b/Core/Interface/SolidSyslogBlockDevice.h @@ -39,6 +39,12 @@ EXTERN_C_BEGIN ); size_t SolidSyslogBlockDevice_Size(struct SolidSyslogBlockDevice * device, size_t blockIndex); + /* The device-wide block capacity in bytes — how large each block can grow. Distinct from + * Size(blockIndex), which reports the current occupancy of one block. The device is the + * single source of truth for this value; a BlockStore reads it at construction rather than + * taking a separate configured size. */ + size_t SolidSyslogBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice * device); + EXTERN_C_END #endif /* SOLIDSYSLOGBLOCKDEVICE_H */ diff --git a/Core/Interface/SolidSyslogBlockDeviceDefinition.h b/Core/Interface/SolidSyslogBlockDeviceDefinition.h index c88fb64e..2bf3ed36 100644 --- a/Core/Interface/SolidSyslogBlockDeviceDefinition.h +++ b/Core/Interface/SolidSyslogBlockDeviceDefinition.h @@ -23,6 +23,7 @@ EXTERN_C_BEGIN size_t count ); size_t (*Size)(struct SolidSyslogBlockDevice* base, size_t blockIndex); + size_t (*GetBlockSize)(struct SolidSyslogBlockDevice* base); }; EXTERN_C_END diff --git a/Core/Source/SolidSyslogBlockDevice.c b/Core/Source/SolidSyslogBlockDevice.c index e0feee21..682b2eae 100644 --- a/Core/Source/SolidSyslogBlockDevice.c +++ b/Core/Source/SolidSyslogBlockDevice.c @@ -55,3 +55,8 @@ size_t SolidSyslogBlockDevice_Size(struct SolidSyslogBlockDevice* device, size_t { return device->Size(device, blockIndex); } + +size_t SolidSyslogBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice* device) +{ + return device->GetBlockSize(device); +} diff --git a/Core/Source/SolidSyslogNullBlockDevice.c b/Core/Source/SolidSyslogNullBlockDevice.c index 26c70a57..64bdd6f9 100644 --- a/Core/Source/SolidSyslogNullBlockDevice.c +++ b/Core/Source/SolidSyslogNullBlockDevice.c @@ -31,6 +31,7 @@ static bool NullBlockDevice_Append( size_t count ); static size_t NullBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t blockIndex); +static size_t NullBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice* base); struct SolidSyslogBlockDevice* SolidSyslogNullBlockDevice_Get(void) { @@ -42,6 +43,7 @@ struct SolidSyslogBlockDevice* SolidSyslogNullBlockDevice_Get(void) .Append = NullBlockDevice_Append, .WriteAt = NullBlockDevice_WriteAt, .Size = NullBlockDevice_Size, + .GetBlockSize = NullBlockDevice_GetBlockSize, }; return &instance; } @@ -125,3 +127,9 @@ static size_t NullBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t b (void) blockIndex; return 0; } + +static size_t NullBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice* base) +{ + (void) base; + return 0; +} diff --git a/Tests/SolidSyslogNullBlockDeviceTest.cpp b/Tests/SolidSyslogNullBlockDeviceTest.cpp index ebc187cc..6f3f54a9 100644 --- a/Tests/SolidSyslogNullBlockDeviceTest.cpp +++ b/Tests/SolidSyslogNullBlockDeviceTest.cpp @@ -56,6 +56,11 @@ TEST(SolidSyslogNullBlockDevice, SizeReturnsZero) LONGS_EQUAL(0, SolidSyslogBlockDevice_Size(device, 0)); } +TEST(SolidSyslogNullBlockDevice, GetBlockSizeReturnsZero) +{ + LONGS_EQUAL(0, SolidSyslogBlockDevice_GetBlockSize(device)); +} + TEST(SolidSyslogNullBlockDevice, GetIsIdempotentAndReturnsSameInstance) { POINTERS_EQUAL(device, SolidSyslogNullBlockDevice_Get()); From 0b066fd977ee1a518a8096b0ca1d2042c2e75592 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 16:15:18 +0100 Subject: [PATCH 2/7] feat: S21.04 FileBlockDevice takes block size at Create, reports via GetBlockSize Create gains a blockSize parameter, stored and returned via GetBlockSize. Callers updated: BDD targets route their runtime size into the device; BlockStore tests pass a placeholder (config.MaxBlockSize still drives the store until the next slice wires it to the device). Co-Authored-By: Claude Opus 4.8 (1M context) --- Bdd/Targets/Common/BddTargetFreeRtosPipeline.c | 2 +- Bdd/Targets/Linux/main.c | 2 +- Bdd/Targets/Windows/BddTargetWindows.c | 2 +- Core/Interface/SolidSyslogFileBlockDevice.h | 11 +++++++++-- Core/Source/SolidSyslogFileBlockDevice.c | 11 ++++++++++- Core/Source/SolidSyslogFileBlockDevicePrivate.h | 4 +++- Core/Source/SolidSyslogFileBlockDeviceStatic.c | 8 ++++++-- Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp | 2 +- Tests/SolidSyslogBlockStorePosixTest.cpp | 2 +- Tests/SolidSyslogBlockStoreTest.cpp | 4 ++-- Tests/SolidSyslogFileBlockDeviceTest.cpp | 9 +++++++-- 11 files changed, 42 insertions(+), 15 deletions(-) diff --git a/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c b/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c index 91999f6b..c2050edc 100644 --- a/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c +++ b/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c @@ -559,7 +559,7 @@ static bool RebuildWithFileStore(void) DestroyCurrentStore(); storeFile = g_config->CreateStoreFile(); - storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX, pendingMaxBlockSize); struct SolidSyslogSecurityPolicy* policy = CreateSecurityPolicy(); currentPolicy = policy; diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index c6a11de6..bce1a3a1 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -221,7 +221,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetOptions* optio { storeFile = SolidSyslogPosixFile_Create(); - storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX, options->MaxBlockSize); static size_t capacityThreshold; capacityThreshold = options->CapacityThreshold; diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index e0e8349a..423327b9 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -284,7 +284,7 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetWindowsOptions { storeFile = SolidSyslogWindowsFile_Create(); - storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX); + storeBlockDevice = SolidSyslogFileBlockDevice_Create(storeFile, STORE_PATH_PREFIX, options->MaxBlockSize); static size_t capacityThreshold; capacityThreshold = options->CapacityThreshold; diff --git a/Core/Interface/SolidSyslogFileBlockDevice.h b/Core/Interface/SolidSyslogFileBlockDevice.h index 626bf1aa..e0e4ea7d 100644 --- a/Core/Interface/SolidSyslogFileBlockDevice.h +++ b/Core/Interface/SolidSyslogFileBlockDevice.h @@ -1,6 +1,8 @@ #ifndef SOLIDSYSLOGFILEBLOCKDEVICE_H #define SOLIDSYSLOGFILEBLOCKDEVICE_H +#include + #include "ExternC.h" struct SolidSyslogBlockDevice; @@ -12,10 +14,15 @@ EXTERN_C_BEGIN * targeted blockIndex changes — same-block runs of Read and Append (and Append-then-WriteAt * during MarkSent) share the handle. The single-handle-per-path invariant the storage layer * depends on (E27 #345 / S27.01) is enforced by construction here: the driver physically - * holds one file. */ + * holds one file. + * + * blockSize is the per-block capacity the device reports via + * SolidSyslogBlockDevice_GetBlockSize. Pass SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE for the + * default. */ struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create( struct SolidSyslogFile * file, - const char* pathPrefix + const char* pathPrefix, + size_t blockSize ); void SolidSyslogFileBlockDevice_Destroy(struct SolidSyslogBlockDevice * base); diff --git a/Core/Source/SolidSyslogFileBlockDevice.c b/Core/Source/SolidSyslogFileBlockDevice.c index fb66c5cb..0f9498fb 100644 --- a/Core/Source/SolidSyslogFileBlockDevice.c +++ b/Core/Source/SolidSyslogFileBlockDevice.c @@ -64,6 +64,7 @@ static bool FileBlockDevice_WriteAt( ); // NOLINTEND(bugprone-easily-swappable-parameters) static size_t FileBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t blockIndex); +static size_t FileBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice* base); static inline struct SolidSyslogFileBlockDevice* FileBlockDevice_SelfFromBase(struct SolidSyslogBlockDevice* base); static inline void FileBlockDevice_CloseIfOpen(struct OpenHandle* handle); @@ -71,7 +72,8 @@ static inline void FileBlockDevice_CloseIfOpen(struct OpenHandle* handle); void FileBlockDevice_Initialise( struct SolidSyslogBlockDevice* base, struct SolidSyslogFile* file, - const char* pathPrefix + const char* pathPrefix, + size_t blockSize ) { struct SolidSyslogFileBlockDevice* self = FileBlockDevice_SelfFromBase(base); @@ -82,8 +84,10 @@ void FileBlockDevice_Initialise( self->Base.Append = FileBlockDevice_Append; self->Base.WriteAt = FileBlockDevice_WriteAt; self->Base.Size = FileBlockDevice_Size; + self->Base.GetBlockSize = FileBlockDevice_GetBlockSize; self->Handle = (struct OpenHandle) {.File = file, .BlockIndex = 0, .IsOpen = false}; self->PathPrefix = pathPrefix; + self->BlockSize = blockSize; } void FileBlockDevice_Cleanup(struct SolidSyslogBlockDevice* base) @@ -361,3 +365,8 @@ static size_t FileBlockDevice_Size(struct SolidSyslogBlockDevice* base, size_t b return size; } + +static size_t FileBlockDevice_GetBlockSize(struct SolidSyslogBlockDevice* base) +{ + return FileBlockDevice_SelfFromBase(base)->BlockSize; +} diff --git a/Core/Source/SolidSyslogFileBlockDevicePrivate.h b/Core/Source/SolidSyslogFileBlockDevicePrivate.h index c59f7ea5..b426d97c 100644 --- a/Core/Source/SolidSyslogFileBlockDevicePrivate.h +++ b/Core/Source/SolidSyslogFileBlockDevicePrivate.h @@ -29,12 +29,14 @@ struct SolidSyslogFileBlockDevice struct SolidSyslogBlockDevice Base; struct OpenHandle Handle; const char* PathPrefix; + size_t BlockSize; }; void FileBlockDevice_Initialise( struct SolidSyslogBlockDevice* base, struct SolidSyslogFile* file, - const char* pathPrefix + const char* pathPrefix, + size_t blockSize ); void FileBlockDevice_Cleanup(struct SolidSyslogBlockDevice* base); diff --git a/Core/Source/SolidSyslogFileBlockDeviceStatic.c b/Core/Source/SolidSyslogFileBlockDeviceStatic.c index 3257496c..ce793854 100644 --- a/Core/Source/SolidSyslogFileBlockDeviceStatic.c +++ b/Core/Source/SolidSyslogFileBlockDeviceStatic.c @@ -26,13 +26,17 @@ static struct SolidSyslogPoolAllocator FileBlockDevice_Allocator = { SOLIDSYSLOG_FILE_BLOCK_DEVICE_POOL_SIZE }; -struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create(struct SolidSyslogFile* file, const char* pathPrefix) +struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create( + struct SolidSyslogFile* file, + const char* pathPrefix, + size_t blockSize +) { struct SolidSyslogBlockDevice* result = SolidSyslogNullBlockDevice_Get(); size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&FileBlockDevice_Allocator); if (SolidSyslogPoolAllocator_IndexIsValid(&FileBlockDevice_Allocator, index)) { - FileBlockDevice_Initialise(&FileBlockDevice_Pool[index].Base, file, pathPrefix); + FileBlockDevice_Initialise(&FileBlockDevice_Pool[index].Base, file, pathPrefix, blockSize); result = &FileBlockDevice_Pool[index].Base; } else diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 78c6234d..34fe8efb 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -118,7 +118,7 @@ TEST_BASE(DrainTestFixtureBase) void setupBlockDeviceAndPolicy() { file = FileFake_Create(&fileStorage); - device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); policy = SolidSyslogNullSecurityPolicy_Get(); } diff --git a/Tests/SolidSyslogBlockStorePosixTest.cpp b/Tests/SolidSyslogBlockStorePosixTest.cpp index 7f08a42a..eff52421 100644 --- a/Tests/SolidSyslogBlockStorePosixTest.cpp +++ b/Tests/SolidSyslogBlockStorePosixTest.cpp @@ -46,7 +46,7 @@ TEST_GROUP(SolidSyslogBlockStorePosix) { CleanStoreFiles(); file = SolidSyslogPosixFile_Create(); - device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); std::memset(maxMsg, 'A', sizeof(maxMsg)); } diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index 911c9ebe..26c206a5 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -75,7 +75,7 @@ TEST_BASE(BlockDeviceTestBase) void setupBlockDeviceFakes() { file = FileFake_Create(&storage); - device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); } void teardownBlockDeviceFakes() const @@ -490,7 +490,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) void CreateWithPathPrefix(const char* prefix) { SolidSyslogFileBlockDevice_Destroy(device); - device = SolidSyslogFileBlockDevice_Create(file, prefix); + device = SolidSyslogFileBlockDevice_Create(file, prefix, 4096); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); store = SolidSyslogBlockStore_Create(&config); } diff --git a/Tests/SolidSyslogFileBlockDeviceTest.cpp b/Tests/SolidSyslogFileBlockDeviceTest.cpp index d9943cfe..abaf2a94 100644 --- a/Tests/SolidSyslogFileBlockDeviceTest.cpp +++ b/Tests/SolidSyslogFileBlockDeviceTest.cpp @@ -37,7 +37,7 @@ TEST_GROUP(SolidSyslogFileBlockDevice) void setup() override { file = FileFake_Create(&fileStorage); - device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); } void teardown() override @@ -57,6 +57,11 @@ TEST(SolidSyslogFileBlockDevice, CreateReturnsNonNull) CHECK_TRUE(device != nullptr); } +TEST(SolidSyslogFileBlockDevice, GetBlockSizeReturnsCreatedSize) +{ + LONGS_EQUAL(4096, SolidSyslogBlockDevice_GetBlockSize(device)); +} + TEST(SolidSyslogFileBlockDevice, ExistsReturnsFalseOnFreshSlate) { CHECK_FALSE(SolidSyslogBlockDevice_Exists(device, 0)); @@ -308,7 +313,7 @@ TEST_GROUP(SolidSyslogFileBlockDevicePool) [[nodiscard]] struct SolidSyslogBlockDevice* MakeDevice() const { - return SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX); + return SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); } void FillPool() From 9b522e68e8658dd85a62cb576b977a8c5f5920a8 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 16:44:43 +0100 Subject: [PATCH 3/7] refactor: S21.04 source block size from the device, drop MaxBlockSize from BlockStoreConfig BlockStore_Create now reads SolidSyslogBlockDevice_GetBlockSize() instead of config.MaxBlockSize, which is removed from SolidSyslogBlockStoreConfig (API break). The min-block-size clamp is unchanged. Size-varying tests re-point the fixture device via an idempotent EnsureDeviceBlockSize helper (same-size calls reuse the device so corruption-recovery's persistent file handle survives). BDD targets route their runtime size into the device instead of the store config. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Common/BddTargetFreeRtosPipeline.c | 1 - Bdd/Targets/Linux/main.c | 1 - Bdd/Targets/Windows/BddTargetWindows.c | 1 - Core/Interface/SolidSyslogBlockStore.h | 1 - Core/Source/SolidSyslogBlockStoreStatic.c | 6 ++- ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 17 +++++++- Tests/SolidSyslogBlockStorePosixTest.cpp | 7 +++- Tests/SolidSyslogBlockStoreTest.cpp | 40 ++++++++++++++----- 8 files changed, 56 insertions(+), 18 deletions(-) diff --git a/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c b/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c index c2050edc..a45485ec 100644 --- a/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c +++ b/Bdd/Targets/Common/BddTargetFreeRtosPipeline.c @@ -565,7 +565,6 @@ static bool RebuildWithFileStore(void) currentPolicy = policy; struct SolidSyslogBlockStoreConfig storeConfig = { .BlockDevice = storeBlockDevice, - .MaxBlockSize = pendingMaxBlockSize, .MaxBlocks = pendingMaxBlocks, .DiscardPolicy = MapDiscardPolicy(pendingDiscardPolicy), .SecurityPolicy = policy, diff --git a/Bdd/Targets/Linux/main.c b/Bdd/Targets/Linux/main.c index bce1a3a1..5491c89f 100644 --- a/Bdd/Targets/Linux/main.c +++ b/Bdd/Targets/Linux/main.c @@ -228,7 +228,6 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetOptions* optio securityPolicy = CreateSecurityPolicy(options); static struct SolidSyslogBlockStoreConfig storeConfig = {0}; storeConfig.BlockDevice = storeBlockDevice; - storeConfig.MaxBlockSize = options->MaxBlockSize; storeConfig.MaxBlocks = options->MaxBlocks; storeConfig.DiscardPolicy = MapDiscardPolicy(options->DiscardPolicy); storeConfig.SecurityPolicy = securityPolicy; diff --git a/Bdd/Targets/Windows/BddTargetWindows.c b/Bdd/Targets/Windows/BddTargetWindows.c index 423327b9..43ddd5f2 100644 --- a/Bdd/Targets/Windows/BddTargetWindows.c +++ b/Bdd/Targets/Windows/BddTargetWindows.c @@ -291,7 +291,6 @@ static struct SolidSyslogStore* CreateStore(const struct BddTargetWindowsOptions securityPolicy = CreateSecurityPolicy(options); static struct SolidSyslogBlockStoreConfig storeConfig = {0}; storeConfig.BlockDevice = storeBlockDevice; - storeConfig.MaxBlockSize = options->MaxBlockSize; storeConfig.MaxBlocks = options->MaxBlocks; storeConfig.DiscardPolicy = MapDiscardPolicy(options->DiscardPolicy); storeConfig.SecurityPolicy = securityPolicy; diff --git a/Core/Interface/SolidSyslogBlockStore.h b/Core/Interface/SolidSyslogBlockStore.h index 279a4774..1061a117 100644 --- a/Core/Interface/SolidSyslogBlockStore.h +++ b/Core/Interface/SolidSyslogBlockStore.h @@ -35,7 +35,6 @@ EXTERN_C_BEGIN /* Required. Caller-owned: must outlive the BlockStore. SolidSyslogBlockStore_Destroy * does NOT destroy the block device — that is the integrator's responsibility. */ struct SolidSyslogBlockDevice* BlockDevice; - size_t MaxBlockSize; size_t MaxBlocks; enum SolidSyslogDiscardPolicy DiscardPolicy; struct SolidSyslogSecurityPolicy* SecurityPolicy; diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c index cba0b28b..e6c188c8 100644 --- a/Core/Source/SolidSyslogBlockStoreStatic.c +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -6,6 +6,7 @@ #include "BlockSequencePrivate.h" #include "RecordStorePrivate.h" +#include "SolidSyslogBlockDevice.h" #include "SolidSyslogBlockStoreErrors.h" #include "SolidSyslogBlockStorePrivate.h" #include "SolidSyslogError.h" @@ -88,8 +89,9 @@ static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( const struct RecordStore* recordStore ) { - size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); - size_t maxBlockSize = (config->MaxBlockSize < minBlockSize) ? minBlockSize : config->MaxBlockSize; + size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); + size_t deviceBlockSize = SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice); + size_t maxBlockSize = (deviceBlockSize < minBlockSize) ? minBlockSize : deviceBlockSize; struct BlockSequenceConfig blockConfig = { .BlockDevice = config->BlockDevice, diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 34fe8efb..99b7b867 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -19,6 +19,7 @@ extern "C" { #include "FileFake.h" #include "SolidSyslog.h" +#include "SolidSyslogBlockDevice.h" #include "SolidSyslogBlockStore.h" #include "SolidSyslogBuffer.h" #include "SolidSyslogCircularBuffer.h" @@ -122,6 +123,18 @@ TEST_BASE(DrainTestFixtureBase) policy = SolidSyslogNullSecurityPolicy_Get(); } + /* Block size is a property of the device; re-point it (pool size 1, so + * destroy-then-recreate on the same FileFake) at the scenario's size. + * Idempotent — unchanged size reuses the existing device. */ + void ensureDeviceBlockSize(size_t blockSize) + { + if (SolidSyslogBlockDevice_GetBlockSize(device) != blockSize) + { + SolidSyslogFileBlockDevice_Destroy(device); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, blockSize); + } + } + void teardownBlockDeviceAndPolicy() const { SolidSyslogFileBlockDevice_Destroy(device); @@ -152,9 +165,9 @@ TEST_GROUP_BASE(BlockStoreDrainOrdering, DrainTestFixtureBase) void CreateStore(const DrainTestConfig& cfg) { + ensureDeviceBlockSize(cfg.MaxBlockSize); struct SolidSyslogBlockStoreConfig config = {}; config.BlockDevice = device; - config.MaxBlockSize = cfg.MaxBlockSize; config.MaxBlocks = cfg.MaxBlocks; config.DiscardPolicy = cfg.DiscardPolicy; config.SecurityPolicy = policy; @@ -248,9 +261,9 @@ TEST_GROUP_BASE(ServiceDrainInterleave, DrainTestFixtureBase) /* Build BlockStore + wire SolidSyslog facade with buffer + store + spy. */ void Setup(const DrainTestConfig& cfg) { + ensureDeviceBlockSize(cfg.MaxBlockSize); struct SolidSyslogBlockStoreConfig storeCfg = {}; storeCfg.BlockDevice = device; - storeCfg.MaxBlockSize = cfg.MaxBlockSize; storeCfg.MaxBlocks = cfg.MaxBlocks; storeCfg.DiscardPolicy = cfg.DiscardPolicy; storeCfg.SecurityPolicy = policy; diff --git a/Tests/SolidSyslogBlockStorePosixTest.cpp b/Tests/SolidSyslogBlockStorePosixTest.cpp index eff52421..ceec7a33 100644 --- a/Tests/SolidSyslogBlockStorePosixTest.cpp +++ b/Tests/SolidSyslogBlockStorePosixTest.cpp @@ -4,6 +4,7 @@ #include #include "CppUTest/TestHarness.h" +#include "SolidSyslogBlockDevice.h" #include "SolidSyslogFileBlockDevice.h" #include "SolidSyslogBlockStore.h" #include "SolidSyslogPosixFile.h" @@ -61,9 +62,13 @@ TEST_GROUP(SolidSyslogBlockStorePosix) // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- test helper mirrors rotation test group signature void CreateStore(size_t maxBlockSize = ONE_MAX_MSG_RECORD, size_t maxBlocks = 2) { + if (SolidSyslogBlockDevice_GetBlockSize(device) != maxBlockSize) + { + SolidSyslogFileBlockDevice_Destroy(device); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, maxBlockSize); + } struct SolidSyslogBlockStoreConfig config = {}; config.BlockDevice = device; - config.MaxBlockSize = maxBlockSize; config.MaxBlocks = maxBlocks; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST; store = SolidSyslogBlockStore_Create(&config); diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index 26c206a5..ecd02ac3 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -3,6 +3,7 @@ #include #include "CppUTest/TestHarness.h" +#include "SolidSyslogBlockDevice.h" #include "SolidSyslogBlockStore.h" #include "SolidSyslogCrc16Policy.h" #include "SolidSyslogFile.h" @@ -39,7 +40,6 @@ enum static const struct SolidSyslogBlockStoreConfig DEFAULT_CONFIG = { nullptr, - TEST_MAX_BLOCK_SIZE, TEST_MAX_BLOCKS, SOLIDSYSLOG_DISCARD_POLICY_OLDEST, nullptr, @@ -75,7 +75,7 @@ TEST_BASE(BlockDeviceTestBase) void setupBlockDeviceFakes() { file = FileFake_Create(&storage); - device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 4096); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, TEST_MAX_BLOCK_SIZE); } void teardownBlockDeviceFakes() const @@ -86,6 +86,21 @@ TEST_BASE(BlockDeviceTestBase) } FileFake_Destroy(); } + + /* Block size is now a property of the device, not the store config. Tests that + * exercise a specific block size re-point the fixture device at it (pool size 1, + * so destroy-then-recreate on the same FileFake, mirroring CreateWithPathPrefix). + * Idempotent: when the size is unchanged the existing device is reused, so tests + * that rebuild the store on the same device (e.g. corruption recovery) keep their + * single persistent file handle. */ + void EnsureDeviceBlockSize(size_t blockSize) + { + if (SolidSyslogBlockDevice_GetBlockSize(device) != blockSize) + { + SolidSyslogFileBlockDevice_Destroy(device); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, blockSize); + } + } }; // clang-format on @@ -482,8 +497,8 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) void CreateWithMaxBlockSize(size_t maxBlockSize) { + EnsureDeviceBlockSize(maxBlockSize); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - config.MaxBlockSize = maxBlockSize; store = SolidSyslogBlockStore_Create(&config); } @@ -703,9 +718,9 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreRotation, BlockDeviceTestBase) size_t maxBlocks = 2, SolidSyslogStoreFullCallback onStoreFull = nullptr, void* storeFullContext = nullptr) { + EnsureDeviceBlockSize(maxBlockSize); struct SolidSyslogBlockStoreConfig config = DEFAULT_CONFIG; config.BlockDevice = device; - config.MaxBlockSize = maxBlockSize; config.MaxBlocks = maxBlocks; config.DiscardPolicy = policy; config.OnStoreFull = onStoreFull; @@ -1685,9 +1700,9 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCorruptionRecovery, BlockDeviceTestBase) // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- maxBlockSize and maxBlocks have distinct semantics void CreateWithMaxBlockSize(size_t maxBlockSize, size_t maxBlocks = 2) { + EnsureDeviceBlockSize(maxBlockSize); struct SolidSyslogBlockStoreConfig config = DEFAULT_CONFIG; config.BlockDevice = device; - config.MaxBlockSize = maxBlockSize; config.MaxBlocks = maxBlocks; config.SecurityPolicy = policy; store = SolidSyslogBlockStore_Create(&config); @@ -1787,8 +1802,8 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreCapacity, BlockDeviceTestBase) void CreateWithCapacity(size_t maxBlockSize, size_t maxBlocks, enum SolidSyslogDiscardPolicy policy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST) { + EnsureDeviceBlockSize(maxBlockSize); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - config.MaxBlockSize = maxBlockSize; config.MaxBlocks = maxBlocks; config.DiscardPolicy = policy; store = SolidSyslogBlockStore_Create(&config); @@ -1817,6 +1832,13 @@ TEST(SolidSyslogBlockStoreCapacity, GetTotalBytesScalesWithConfig) LONGS_EQUAL(3 * 10000, SolidSyslogStore_GetTotalBytes(store)); } +TEST(SolidSyslogBlockStoreCapacity, TotalBytesDerivesFromDeviceBlockSize) +{ + EnsureDeviceBlockSize(8192); + CreateDefault(); + LONGS_EQUAL(TEST_MAX_BLOCKS * 8192, SolidSyslogStore_GetTotalBytes(store)); +} + /* Given an empty store, * When GetUsedBytes is queried, * Then it returns 0. */ @@ -1964,8 +1986,8 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, ReArmsAfterFallingEdgeOnDiscardOlde char maxMsg[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; memset(maxMsg, 'A', sizeof(maxMsg)); + EnsureDeviceBlockSize(TWO_RECORDS); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - config.MaxBlockSize = TWO_RECORDS; config.MaxBlocks = 2; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_OLDEST; config.GetCapacityThreshold = ReturnsConfiguredThreshold; @@ -2076,8 +2098,8 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, AtFullCapacityWithHaltThresholdFire thresholdFireOrder = 0; storeFullFireOrder = 0; + EnsureDeviceBlockSize(MAX_MSG_RECORD + SLACK); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - config.MaxBlockSize = MAX_MSG_RECORD + SLACK; config.MaxBlocks = 2; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_HALT; config.OnStoreFull = RecordStoreFullFireOrder; @@ -2107,8 +2129,8 @@ TEST(SolidSyslogBlockStoreCapacityThreshold, StickyHundredPercentDoesNotRefireTh char maxMsg[SOLIDSYSLOG_MAX_MESSAGE_SIZE]; memset(maxMsg, 'A', sizeof(maxMsg)); + EnsureDeviceBlockSize(MAX_MSG_RECORD + SLACK); struct SolidSyslogBlockStoreConfig config = MakeConfig(device); - config.MaxBlockSize = MAX_MSG_RECORD + SLACK; config.MaxBlocks = 2; config.DiscardPolicy = SOLIDSYSLOG_DISCARD_POLICY_HALT; config.GetCapacityThreshold = ReturnsConfiguredThreshold; From 90dc5184f48a1f15c525d9b03179d047e8db213a Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 16:54:40 +0100 Subject: [PATCH 4/7] feat: S21.04 SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE tunable + device 0-uses-default FileBlockDevice_Create resolves a blockSize of 0 to SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE (default 8192, floored at one worst-case record). FatFs adapter gains a compile-time FF_MAX_SS floor on the default. Renames the zero-size store test to reflect that 0 now selects the device default rather than clamping to the minimum. Co-Authored-By: Claude Opus 4.8 (1M context) --- Core/Interface/SolidSyslogTunablesDefaults.h | 22 +++++++++++++++++++ .../Source/SolidSyslogFileBlockDeviceStatic.c | 3 ++- Platform/FatFs/Source/SolidSyslogFatFsFile.c | 8 +++++++ Tests/SolidSyslogBlockStoreTest.cpp | 2 +- Tests/SolidSyslogFileBlockDeviceTest.cpp | 7 ++++++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index a3398f25..c68156cf 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -81,6 +81,28 @@ #error "SOLIDSYSLOG_MAX_INTEGRITY_SIZE must be >= 4" #endif +/* + * Default per-block capacity (bytes) for file-backed block devices + * (SolidSyslogFileBlockDevice and any FatFs / FreeRTOS-Plus-FAT-backed + * equivalent). Supplied to SolidSyslogFileBlockDevice_Create when the + * integrator has no specific size in mind; passing 0 to _Create selects + * this default. A larger block holds more records before rotating to a + * fresh file; a smaller block rotates (and fsyncs) more often. + * + * Floor: one worst-case record — the RFC 5424 max message plus the widest + * integrity tag plus the 5-byte record framing (2 magic + 2 length + + * 1 sent-flag). Below that a block could not hold a single record, so the + * default must clear it for every SecurityPolicy. Sub-floor values + * rejected at compile time. + */ +#ifndef SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE +#define SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE 8192U +#endif + +#if SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE < (SOLIDSYSLOG_MAX_MESSAGE_SIZE + SOLIDSYSLOG_MAX_INTEGRITY_SIZE + 5) +#error "SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE must hold one worst-case record (MAX_MESSAGE_SIZE + MAX_INTEGRITY_SIZE + 5 framing bytes)" +#endif + /* * Number of SolidSyslog instances the library's internal static pool * can simultaneously hold. Each instance is a small bookkeeping struct diff --git a/Core/Source/SolidSyslogFileBlockDeviceStatic.c b/Core/Source/SolidSyslogFileBlockDeviceStatic.c index ce793854..a3f03d2b 100644 --- a/Core/Source/SolidSyslogFileBlockDeviceStatic.c +++ b/Core/Source/SolidSyslogFileBlockDeviceStatic.c @@ -33,10 +33,11 @@ struct SolidSyslogBlockDevice* SolidSyslogFileBlockDevice_Create( ) { struct SolidSyslogBlockDevice* result = SolidSyslogNullBlockDevice_Get(); + size_t resolvedBlockSize = (blockSize == 0U) ? (size_t) SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE : blockSize; size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&FileBlockDevice_Allocator); if (SolidSyslogPoolAllocator_IndexIsValid(&FileBlockDevice_Allocator, index)) { - FileBlockDevice_Initialise(&FileBlockDevice_Pool[index].Base, file, pathPrefix, blockSize); + FileBlockDevice_Initialise(&FileBlockDevice_Pool[index].Base, file, pathPrefix, resolvedBlockSize); result = &FileBlockDevice_Pool[index].Base; } else diff --git a/Platform/FatFs/Source/SolidSyslogFatFsFile.c b/Platform/FatFs/Source/SolidSyslogFatFsFile.c index e60de6bb..c4d8c895 100644 --- a/Platform/FatFs/Source/SolidSyslogFatFsFile.c +++ b/Platform/FatFs/Source/SolidSyslogFatFsFile.c @@ -8,8 +8,16 @@ #include "SolidSyslogFatFsFilePrivate.h" #include "SolidSyslogFileDefinition.h" #include "SolidSyslogNullFile.h" +#include "SolidSyslogTunables.h" #include "ff.h" +/* The shared file-block-size default must clear one FatFs sector — a block + * smaller than the underlying sector cannot back a coherent on-disk record + * layout. FF_MAX_SS comes from the integrator's ffconf.h. */ +#if SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE < FF_MAX_SS +#error "SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE must be >= FF_MAX_SS (one FatFs sector)" +#endif + const struct SolidSyslogErrorSource FatFsFileErrorSource = {"FatFsFile"}; #define READ_WRITE_OR_CREATE (FA_READ | FA_WRITE | FA_OPEN_ALWAYS) diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index ecd02ac3..93f25397 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -540,7 +540,7 @@ TEST(SolidSyslogBlockStoreConfig, MaxBlocksHundredClampedToMaximum) VerifyWriteAndReadBack(); } -TEST(SolidSyslogBlockStoreConfig, MaxBlockSizeZeroClampedToMinimum) +TEST(SolidSyslogBlockStoreConfig, ZeroBlockSizeUsesDeviceDefault) { CreateWithMaxBlockSize(0); VerifyWriteAndReadBack(); diff --git a/Tests/SolidSyslogFileBlockDeviceTest.cpp b/Tests/SolidSyslogFileBlockDeviceTest.cpp index abaf2a94..2e9e1772 100644 --- a/Tests/SolidSyslogFileBlockDeviceTest.cpp +++ b/Tests/SolidSyslogFileBlockDeviceTest.cpp @@ -62,6 +62,13 @@ TEST(SolidSyslogFileBlockDevice, GetBlockSizeReturnsCreatedSize) LONGS_EQUAL(4096, SolidSyslogBlockDevice_GetBlockSize(device)); } +TEST(SolidSyslogFileBlockDevice, ZeroBlockSizeUsesDefault) +{ + SolidSyslogFileBlockDevice_Destroy(device); + device = SolidSyslogFileBlockDevice_Create(file, TEST_PATH_PREFIX, 0); + LONGS_EQUAL(SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE, SolidSyslogBlockDevice_GetBlockSize(device)); +} + TEST(SolidSyslogFileBlockDevice, ExistsReturnsFalseOnFreshSlate) { CHECK_FALSE(SolidSyslogBlockDevice_Exists(device, 0)); From 3969a92e06ba731edac57691ab7e53f374754315 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 18:01:46 +0100 Subject: [PATCH 5/7] feat: S21.04 reject a device block too small for one record (BAD_CONFIG) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design Y: when the device's block size cannot hold one worst-case record, BlockStore_Create reports SOLIDSYSLOG_CAT_BAD_CONFIG / BLOCKSTORE_ERROR_BLOCK_TOO_SMALL and falls back to NullStore instead of silently growing the device's reported size. The old max(deviceSize, minRecord) clamp is gone — the device is the single source of truth. Drain-ordering tests that leaned on the silent clamp now size blocks explicitly to one near-max record. Co-Authored-By: Claude Opus 4.8 (1M context) --- Core/Interface/SolidSyslogBlockStoreErrors.h | 1 + Core/Source/SolidSyslogBlockStoreStatic.c | 56 +++++++++++++------ ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 17 +++++- Tests/SolidSyslogBlockStoreTest.cpp | 17 +++++- 4 files changed, 69 insertions(+), 22 deletions(-) diff --git a/Core/Interface/SolidSyslogBlockStoreErrors.h b/Core/Interface/SolidSyslogBlockStoreErrors.h index 025b1cb7..0753a928 100644 --- a/Core/Interface/SolidSyslogBlockStoreErrors.h +++ b/Core/Interface/SolidSyslogBlockStoreErrors.h @@ -11,6 +11,7 @@ EXTERN_C_BEGIN { BLOCKSTORE_ERROR_POOL_EXHAUSTED, BLOCKSTORE_ERROR_UNKNOWN_DESTROY, + BLOCKSTORE_ERROR_BLOCK_TOO_SMALL, BLOCKSTORE_ERROR_MAX }; diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c index e6c188c8..6a27b84a 100644 --- a/Core/Source/SolidSyslogBlockStoreStatic.c +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -23,7 +23,8 @@ struct SolidSyslogStore; static inline size_t BlockStore_IndexFromHandle(const struct SolidSyslogStore* base); static inline void BlockStore_CleanupAtIndex(size_t index, void* context); static struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy(struct SolidSyslogSecurityPolicy* configured); -static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig(const struct SolidSyslogBlockStoreConfig* config); +static bool BlockStore_DeviceCanHoldOneRecord( const struct SolidSyslogBlockStoreConfig* config, const struct RecordStore* recordStore ); @@ -35,6 +36,7 @@ static struct SolidSyslogPoolAllocator BlockStore_Allocator = {BlockStore_InUse, struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBlockStoreConfig* config) { struct SolidSyslogStore* result = SolidSyslogNullStore_Get(); + bool blockTooSmall = false; size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&BlockStore_Allocator); if (SolidSyslogPoolAllocator_IndexIsValid(&BlockStore_Allocator, index)) @@ -44,16 +46,25 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl if (recordStore != NULL) { - struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config, recordStore); - struct BlockSequence* blockSequence = BlockSequence_Create(&blockConfig); - - if (blockSequence != NULL) + if (BlockStore_DeviceCanHoldOneRecord(config, recordStore)) { - BlockStore_Initialise(&BlockStore_Pool[index].Base, recordStore, blockSequence, config); - result = &BlockStore_Pool[index].Base; + struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config); + 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); + (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); + } } else { + blockTooSmall = true; RecordStore_Destroy(recordStore); (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); } @@ -64,7 +75,11 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl } } - if (result == SolidSyslogNullStore_Get()) + if (blockTooSmall) + { + BlockStore_Report(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_CAT_BAD_CONFIG, BLOCKSTORE_ERROR_BLOCK_TOO_SMALL); + } + else if (result == SolidSyslogNullStore_Get()) { BlockStore_Report(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_CAT_POOL_EXHAUSTED, BLOCKSTORE_ERROR_POOL_EXHAUSTED); } @@ -84,18 +99,13 @@ static struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy(struct return resolved; } -static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( - const struct SolidSyslogBlockStoreConfig* config, - const struct RecordStore* recordStore -) +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig(const struct SolidSyslogBlockStoreConfig* config) { - size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); - size_t deviceBlockSize = SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice); - size_t maxBlockSize = (deviceBlockSize < minBlockSize) ? minBlockSize : deviceBlockSize; - + /* The device is the single source of truth for block size; Create has already + * rejected a device whose block is too small to hold one record. */ struct BlockSequenceConfig blockConfig = { .BlockDevice = config->BlockDevice, - .MaxBlockSize = maxBlockSize, + .MaxBlockSize = SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice), .MaxBlocks = config->MaxBlocks, .DiscardPolicy = config->DiscardPolicy, .OnStoreFull = config->OnStoreFull, @@ -107,6 +117,18 @@ static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( return blockConfig; } +/* The device owns its block size; a block too small to hold one worst-case record + * (max message + the active policy's trailer + record framing) cannot store anything, + * so Create rejects it rather than silently growing the device's reported size. */ +static bool BlockStore_DeviceCanHoldOneRecord( + const struct SolidSyslogBlockStoreConfig* config, + const struct RecordStore* recordStore +) +{ + size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); + return SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice) >= minBlockSize; +} + void SolidSyslogBlockStore_Destroy(struct SolidSyslogStore* base) { size_t index = BlockStore_IndexFromHandle(base); diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 99b7b867..cee1b1bb 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -41,6 +41,16 @@ extern "C" static const char* const TEST_PATH_PREFIX = "/tmp/draintest_"; +/* The store rejects a device whose block cannot hold one worst-case record + * (max message + framing). Sized just over that minimum and paired with a + * near-max payload, each block holds exactly one record — the calibration the + * drain-ordering scenarios need to span multiple blocks with only a handful of + * writes. (Previously these tests passed a tiny size and leaned on a silent + * clamp; the device is now the source of truth and an undersized block is a + * hard error, so the size is explicit here.) */ +static const size_t ONE_RECORD_BLOCK_SIZE = SOLIDSYSLOG_MAX_MESSAGE_SIZE + 16U; +static const size_t NEAR_MAX_PAYLOAD = SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U; + /* SenderSpy — sticky outage mode (every Send returns false until cleared) * and a vector of every *successful* send. Bigger than SenderFake's * last-only capture and one-shot FailNextSend; the BDD reproducer needs @@ -311,8 +321,8 @@ TEST(ServiceDrainInterleave, DiscardNewestDoesNotLetNewestBypassOldestOnRecovery * with just a couple of messages. */ DrainTestConfig cfg = { /*maxBlocks=*/2, - /*maxBlockSize=*/200 /*will clamp up*/, - /*payloadSize=*/SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U, + /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, + /*payloadSize=*/NEAR_MAX_PAYLOAD, SOLIDSYSLOG_DISCARD_POLICY_NEWEST }; Setup(cfg); @@ -385,7 +395,8 @@ TEST(ServiceDrainInterleave, DiscardNewestDoesNotLetNewestBypassOldestOnRecovery TEST(BlockStoreDrainOrdering, OutageDrainProducesAscendingSequenceIds) { DrainTestConfig cfg = - {/*maxBlocks=*/2, /*maxBlockSize=*/200, /*payloadSize=*/64, SOLIDSYSLOG_DISCARD_POLICY_NEWEST}; + {/*maxBlocks=*/2, /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, /*payloadSize=*/NEAR_MAX_PAYLOAD, + SOLIDSYSLOG_DISCARD_POLICY_NEWEST}; CreateStore(cfg); /* Pre-outage send + drain — mirrors `When the client sends a message` diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index 93f25397..45b71e7c 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -5,13 +5,17 @@ #include "CppUTest/TestHarness.h" #include "SolidSyslogBlockDevice.h" #include "SolidSyslogBlockStore.h" +#include "SolidSyslogBlockStoreErrors.h" #include "SolidSyslogCrc16Policy.h" +#include "SolidSyslogError.h" +#include "SolidSyslogErrorCategory.h" #include "SolidSyslogFile.h" #include "SolidSyslogFileBlockDevice.h" #include "SolidSyslogNullStore.h" #include "SolidSyslogSecurityPolicyDefinition.h" #include "SolidSyslogStore.h" #include "SolidSyslogTunables.h" +#include "ErrorHandlerFake.h" #include "FileFake.h" #include "TestUtils.h" @@ -484,6 +488,7 @@ TEST_GROUP_BASE(SolidSyslogBlockStoreConfig, BlockDeviceTestBase) void teardown() override { + SolidSyslog_SetErrorHandler(nullptr, nullptr); SolidSyslogBlockStore_Destroy(store); teardownBlockDeviceFakes(); } @@ -546,10 +551,18 @@ TEST(SolidSyslogBlockStoreConfig, ZeroBlockSizeUsesDeviceDefault) VerifyWriteAndReadBack(); } -TEST(SolidSyslogBlockStoreConfig, MaxBlockSizeOneClampedToMinimum) +TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeRejectsToNullStore) { CreateWithMaxBlockSize(1); - VerifyWriteAndReadBack(); + POINTERS_EQUAL(SolidSyslogNullStore_Get(), store); +} + +TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeReportsBadConfig) +{ + ErrorHandlerFake_Install(nullptr); + CreateWithMaxBlockSize(1); + UNSIGNED_LONGS_EQUAL(SOLIDSYSLOG_CAT_BAD_CONFIG, ErrorHandlerFake_LastCategory()); + UNSIGNED_LONGS_EQUAL(BLOCKSTORE_ERROR_BLOCK_TOO_SMALL, ErrorHandlerFake_LastDetail()); } TEST(SolidSyslogBlockStoreConfig, FilenameExactlyAtMaxPath) From 0e028bfcb353a52a1792f1650839f4e205524b4f Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 18:19:17 +0100 Subject: [PATCH 6/7] =?UTF-8?q?chore:=20S21.04=20trailing=20housekeeping?= =?UTF-8?q?=20=E2=80=94=20docs,=20MISRA=2015.7/20.9,=20format,=20suppressi?= =?UTF-8?q?ons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md: GetBlockSize on the BlockDevice rows; FileBlockDevice_Create gains blockSize - DEVLOG: S21.04 entry (Design Y, the load-bearing-clamp discovery) - MISRA 15.7: terminal else on the BlockStore_Create report chain - MISRA 20.9: defined() guard on the FatFs FF_MAX_SS floor - misra_suppressions.txt: renumber 3 shifted findings (FileBlockDevice/FatFs 11.3, BlockStoreStatic 11.8) - clang-format reflow Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 6 +-- Core/Interface/SolidSyslogTunablesDefaults.h | 3 +- Core/Source/SolidSyslogBlockStoreStatic.c | 4 ++ DEVLOG.md | 46 +++++++++++++++++++ Platform/FatFs/Source/SolidSyslogFatFsFile.c | 6 ++- ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 11 +++-- misra_suppressions.txt | 6 +-- 7 files changed, 69 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e7f32a77..3a1cac0b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -404,10 +404,10 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogFileDefinition.h` | File implementors (extension point) | `SolidSyslogFile` vtable struct | | `SolidSyslogFile.h` | Any code using the file abstraction | `SolidSyslogFile_Open`, `_Close`, `_IsOpen`, `_Read`, `_Write`, `_SeekTo`, `_Size`, `_Truncate` | | `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) | +| `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`, `_GetBlockSize` (block-indexed I/O; Acquire makes a block ready for fresh writes, Dispose releases it; `_Size(blockIndex)` is a block's current occupancy, `_GetBlockSize()` is the device-wide per-block capacity — the device is the single source of truth a BlockStore reads at construction) | | `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`. | +| `SolidSyslogBlockDeviceDefinition.h` | BlockDevice implementors (extension point) | `SolidSyslogBlockDevice` vtable struct (`Acquire`, `Dispose`, `Exists`, `Read`, `Append`, `WriteAt`, `Size`, `GetBlockSize`) | +| `SolidSyslogFileBlockDevice.h` | System setup code wiring a file-backed block device | `SolidSyslogFileBlockDevice_Create(file, pathPrefix, blockSize)` (sequence-numbered filenames `.log`, one cached file handle; `blockSize` is the per-block capacity reported via `GetBlockSize`, `0` selects the `SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE` tunable), `_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`. | | `SolidSyslogFileBlockDeviceErrors.h` | Any code installing an error handler that wants to react to FileBlockDevice-specific events (pointer-identity match on `FileBlockDeviceErrorSource`, switch on `enum SolidSyslogFileBlockDeviceErrors`) | `enum SolidSyslogFileBlockDeviceErrors` (`FILEBLOCKDEVICE_ERROR_*` codes + `FILEBLOCKDEVICE_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource FileBlockDeviceErrorSource`. Integrators ignore if not handling errors per source. | | `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`. | | `SolidSyslogBlockStoreErrors.h` | Any code installing an error handler that wants to react to BlockStore-specific events (pointer-identity match on `BlockStoreErrorSource`, switch on `enum SolidSyslogBlockStoreErrors`) | `enum SolidSyslogBlockStoreErrors` (`BLOCKSTORE_ERROR_*` codes + `BLOCKSTORE_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource BlockStoreErrorSource`. Integrators ignore if not handling errors per source. | diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index c68156cf..c939e5c4 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -100,7 +100,8 @@ #endif #if SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE < (SOLIDSYSLOG_MAX_MESSAGE_SIZE + SOLIDSYSLOG_MAX_INTEGRITY_SIZE + 5) -#error "SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE must hold one worst-case record (MAX_MESSAGE_SIZE + MAX_INTEGRITY_SIZE + 5 framing bytes)" +#error \ + "SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE must hold one worst-case record (MAX_MESSAGE_SIZE + MAX_INTEGRITY_SIZE + 5 framing bytes)" #endif /* diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c index 6a27b84a..c451d76d 100644 --- a/Core/Source/SolidSyslogBlockStoreStatic.c +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -83,6 +83,10 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl { BlockStore_Report(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_CAT_POOL_EXHAUSTED, BLOCKSTORE_ERROR_POOL_EXHAUSTED); } + else + { + /* Store created successfully — no diagnostic to emit. */ + } return result; } diff --git a/DEVLOG.md b/DEVLOG.md index c380ac29..15b2c97d 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,51 @@ # Dev Log +## 2026-06-04 — S21.04 BlockDevice owns its block size + +Closes the last open thread under E21 (Port-Time Configurability). Block size moves +from `SolidSyslogBlockStoreConfig.MaxBlockSize` to a property of the +`SolidSyslogBlockDevice` — the device becomes the single source of truth, queried +once at `BlockStore_Create`. An API break, deliberately landed in the 0.x beta +window so 1.0.0 stays additive-only. + +### Decisions + +- **Device is authoritative; the store validates, it does not clamp (Design Y).** + The previous `max(configuredSize, oneMaxRecord)` clamp silently grew an + undersized block. That silently betrayed "device is the source of truth," so it's + replaced by an honest reject: a device whose block cannot hold one worst-case + record (max message + framing + the active policy's trailer) → `BlockStore_Create` + reports `SOLIDSYSLOG_CAT_BAD_CONFIG` / `BLOCKSTORE_ERROR_BLOCK_TOO_SMALL` and + falls back to `NullStore` (the established bad-setup contract). +- **`_Create(file, prefix, blockSize)` takes the size at construction**, preserving + the BDD targets' runtime `--max-block-size` knob (they route the parsed value into + the device instead of the store config). +- **`SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE` (8192), `0` selects it.** Floored at one + worst-case record so passing 0 always clears the reject; FatFs adapter adds a + compile-time `FF_MAX_SS` floor on the default. + +### Sliced (5 commits, strict red/green) + +1. `GetBlockSize` on the contract → 2. FileBlockDevice takes the size → 3. store +reads the device, `MaxBlockSize` dropped from config → 4. default tunable + `0→default` +→ 5. Design Y reject. 1396 host tests green throughout. + +### Worth remembering + +- The old clamp was **load-bearing in a documented way**: a drain-ordering test set + `maxBlockSize=200` and commented *"will clamp up so each block holds exactly one + record."* Design Y made that an error, so those tests now size blocks explicitly to + one near-max record. Real implication for integrators: a block smaller than one + max-size message is now rejected, not silently grown. +- Size-varying BlockStore tests re-point the fixture device through an **idempotent** + `EnsureDeviceBlockSize` helper — same-size calls reuse the device so corruption- + recovery's single persistent file handle survives a destroy/recreate. + +### Not verified locally (CI's job) + +- FatFs `FF_MAX_SS` floor and the Windows / FreeRTOS BDD-target callers — the gcc + lane can't build them; their edits are symmetric to the green Linux target. + ## 2026-06-04 — S29.05 PlusFAT media driver + Plus-TCP target swap The last functional story of E29: a FreeRTOS-Plus-FAT `FF_Disk_t` semihosting diff --git a/Platform/FatFs/Source/SolidSyslogFatFsFile.c b/Platform/FatFs/Source/SolidSyslogFatFsFile.c index c4d8c895..4d21a031 100644 --- a/Platform/FatFs/Source/SolidSyslogFatFsFile.c +++ b/Platform/FatFs/Source/SolidSyslogFatFsFile.c @@ -13,8 +13,10 @@ /* The shared file-block-size default must clear one FatFs sector — a block * smaller than the underlying sector cannot back a coherent on-disk record - * layout. FF_MAX_SS comes from the integrator's ffconf.h. */ -#if SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE < FF_MAX_SS + * layout. FF_MAX_SS comes from the integrator's ffconf.h; guard with defined() + * so the check is skipped (rather than evaluating an undefined identifier) on + * any configuration that does not expose it. */ +#if defined(FF_MAX_SS) && (SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE < FF_MAX_SS) #error "SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE must be >= FF_MAX_SS (one FatFs sector)" #endif diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index cee1b1bb..1acf4dba 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -49,7 +49,7 @@ static const char* const TEST_PATH_PREFIX = "/tmp/draintest_"; * clamp; the device is now the source of truth and an undersized block is a * hard error, so the size is explicit here.) */ static const size_t ONE_RECORD_BLOCK_SIZE = SOLIDSYSLOG_MAX_MESSAGE_SIZE + 16U; -static const size_t NEAR_MAX_PAYLOAD = SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U; +static const size_t NEAR_MAX_PAYLOAD = SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U; /* SenderSpy — sticky outage mode (every Send returns false until cleared) * and a vector of every *successful* send. Bigger than SenderFake's @@ -394,9 +394,12 @@ TEST(ServiceDrainInterleave, DiscardNewestDoesNotLetNewestBypassOldestOnRecovery * we have the bug in our hands. */ TEST(BlockStoreDrainOrdering, OutageDrainProducesAscendingSequenceIds) { - DrainTestConfig cfg = - {/*maxBlocks=*/2, /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, /*payloadSize=*/NEAR_MAX_PAYLOAD, - SOLIDSYSLOG_DISCARD_POLICY_NEWEST}; + DrainTestConfig cfg = { + /*maxBlocks=*/2, + /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, + /*payloadSize=*/NEAR_MAX_PAYLOAD, + SOLIDSYSLOG_DISCARD_POLICY_NEWEST + }; CreateStore(cfg); /* Pre-outage send + drain — mirrors `When the client sends a message` diff --git a/misra_suppressions.txt b/misra_suppressions.txt index b55acc09..9cf70ed5 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -26,7 +26,7 @@ misra-c2012-11.2:Platform/Windows/Source/SolidSyslogWinsockAddressPrivate.h:30 misra-c2012-11.3:Core/Interface/SolidSyslogFormatter.h:28 misra-c2012-11.3:Core/Source/SolidSyslogBlockStore.c:73 misra-c2012-11.3:Core/Source/SolidSyslogCircularBuffer.c:91 -misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:102 +misra-c2012-11.3:Core/Source/SolidSyslogFileBlockDevice.c:106 misra-c2012-11.3:Core/Source/SolidSyslogFormatter.c:79 misra-c2012-11.3:Core/Source/SolidSyslogMetaSd.c:55 misra-c2012-11.3:Core/Source/SolidSyslogOriginSd.c:73 @@ -36,7 +36,7 @@ misra-c2012-11.3:Core/Source/SolidSyslogSwitchingSender.c:63 misra-c2012-11.3:Core/Source/SolidSyslogTimeQualitySd.c:63 misra-c2012-11.3:Core/Source/SolidSyslogUdpSender.c:100 misra-c2012-11.3:Platform/Atomics/Source/SolidSyslogStdAtomicCounter.c:29 -misra-c2012-11.3:Platform/FatFs/Source/SolidSyslogFatFsFile.c:49 +misra-c2012-11.3:Platform/FatFs/Source/SolidSyslogFatFsFile.c:59 misra-c2012-11.3:Platform/PlusFat/Source/SolidSyslogPlusFatFile.c:47 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:29 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:36 @@ -116,7 +116,7 @@ misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:59 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:61 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:63 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:67 -misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:41 +misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:44 misra-c2012-11.8:Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:159 misra-c2012-11.8:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:97 From 8e19c8957e8cf5622ce42e37e8b40e6e821ff2ff Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 4 Jun 2026 19:32:49 +0100 Subject: [PATCH 7/7] fix: S21.04 grow-and-warn instead of reject for an undersized block (Design Z) CI revealed the max(deviceSize, oneRecord) clamp was load-bearing: a documented, cross-target calibration the drain tests and the store_capacity/capacity_threshold/ block_lifecycle BDD suites depend on. Rejecting (Design Y, ERROR -> NullStore) broke them and tripped the BDD target's _Exit(3)-on-ERROR handler. Keep the clamp, but emit a WARNING (BAD_CONFIG / BLOCK_TOO_SMALL) so it is no longer silent. Reverts the drain-test recalibration back to the proven 200/64 + 200/(MAX-100) values; converts the reject tests to clamp-works + reports-warning. cppcheck-misra green (11.8 suppressed at the two new GetBlockSize sites, same const-config-member pattern already deviated in this file). Co-Authored-By: Claude Opus 4.8 (1M context) --- Core/Source/SolidSyslogBlockStoreStatic.c | 72 ++++++++++--------- DEVLOG.md | 43 ++++++----- ...SolidSyslogBlockStoreDrainOrderingTest.cpp | 22 ++---- Tests/SolidSyslogBlockStoreTest.cpp | 8 ++- misra_suppressions.txt | 4 +- 5 files changed, 77 insertions(+), 72 deletions(-) diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c index c451d76d..176f711c 100644 --- a/Core/Source/SolidSyslogBlockStoreStatic.c +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -23,7 +23,10 @@ struct SolidSyslogStore; static inline size_t BlockStore_IndexFromHandle(const struct SolidSyslogStore* base); static inline 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); +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( + const struct SolidSyslogBlockStoreConfig* config, + const struct RecordStore* recordStore +); static bool BlockStore_DeviceCanHoldOneRecord( const struct SolidSyslogBlockStoreConfig* config, const struct RecordStore* recordStore @@ -36,7 +39,6 @@ static struct SolidSyslogPoolAllocator BlockStore_Allocator = {BlockStore_InUse, struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBlockStoreConfig* config) { struct SolidSyslogStore* result = SolidSyslogNullStore_Get(); - bool blockTooSmall = false; size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&BlockStore_Allocator); if (SolidSyslogPoolAllocator_IndexIsValid(&BlockStore_Allocator, index)) @@ -46,25 +48,29 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl if (recordStore != NULL) { - if (BlockStore_DeviceCanHoldOneRecord(config, recordStore)) + if (!BlockStore_DeviceCanHoldOneRecord(config, recordStore)) + { + /* The device's block is smaller than one worst-case record. The store + * still works — BuildBlockSequenceConfig grows the block to the minimum so + * a record always fits — but the device was configured below a usable size, + * so surface it as a WARNING (delivered, degraded) rather than failing. */ + BlockStore_Report( + SOLIDSYSLOG_SEVERITY_WARNING, + SOLIDSYSLOG_CAT_BAD_CONFIG, + BLOCKSTORE_ERROR_BLOCK_TOO_SMALL + ); + } + + struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config, recordStore); + struct BlockSequence* blockSequence = BlockSequence_Create(&blockConfig); + + if (blockSequence != NULL) { - struct BlockSequenceConfig blockConfig = BlockStore_BuildBlockSequenceConfig(config); - 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); - (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); - } + BlockStore_Initialise(&BlockStore_Pool[index].Base, recordStore, blockSequence, config); + result = &BlockStore_Pool[index].Base; } else { - blockTooSmall = true; RecordStore_Destroy(recordStore); (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); } @@ -75,18 +81,10 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl } } - if (blockTooSmall) - { - BlockStore_Report(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_CAT_BAD_CONFIG, BLOCKSTORE_ERROR_BLOCK_TOO_SMALL); - } - else if (result == SolidSyslogNullStore_Get()) + if (result == SolidSyslogNullStore_Get()) { BlockStore_Report(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_CAT_POOL_EXHAUSTED, BLOCKSTORE_ERROR_POOL_EXHAUSTED); } - else - { - /* Store created successfully — no diagnostic to emit. */ - } return result; } @@ -103,13 +101,21 @@ static struct SolidSyslogSecurityPolicy* BlockStore_ResolveSecurityPolicy(struct return resolved; } -static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig(const struct SolidSyslogBlockStoreConfig* config) +static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig( + const struct SolidSyslogBlockStoreConfig* config, + const struct RecordStore* recordStore +) { - /* The device is the single source of truth for block size; Create has already - * rejected a device whose block is too small to hold one record. */ + /* The device is the single source of truth for block size, but a block smaller than + * one worst-case record is grown to that floor so a single record always fits (Create + * has already emitted a WARNING for that case). */ + size_t minBlockSize = RecordStore_RecordSize(recordStore, SOLIDSYSLOG_MAX_MESSAGE_SIZE); + size_t deviceBlockSize = SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice); + size_t maxBlockSize = (deviceBlockSize < minBlockSize) ? minBlockSize : deviceBlockSize; + struct BlockSequenceConfig blockConfig = { .BlockDevice = config->BlockDevice, - .MaxBlockSize = SolidSyslogBlockDevice_GetBlockSize(config->BlockDevice), + .MaxBlockSize = maxBlockSize, .MaxBlocks = config->MaxBlocks, .DiscardPolicy = config->DiscardPolicy, .OnStoreFull = config->OnStoreFull, @@ -121,9 +127,9 @@ static struct BlockSequenceConfig BlockStore_BuildBlockSequenceConfig(const stru return blockConfig; } -/* The device owns its block size; a block too small to hold one worst-case record - * (max message + the active policy's trailer + record framing) cannot store anything, - * so Create rejects it rather than silently growing the device's reported size. */ +/* True when the device's block can hold one worst-case record (max message + the active + * policy's trailer + record framing). When false the block is grown to that floor and a + * WARNING is emitted — the store works, but the device's configured size was degraded. */ static bool BlockStore_DeviceCanHoldOneRecord( const struct SolidSyslogBlockStoreConfig* config, const struct RecordStore* recordStore diff --git a/DEVLOG.md b/DEVLOG.md index 15b2c97d..9dc8c8cb 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -10,33 +10,42 @@ window so 1.0.0 stays additive-only. ### Decisions -- **Device is authoritative; the store validates, it does not clamp (Design Y).** - The previous `max(configuredSize, oneMaxRecord)` clamp silently grew an - undersized block. That silently betrayed "device is the source of truth," so it's - replaced by an honest reject: a device whose block cannot hold one worst-case - record (max message + framing + the active policy's trailer) → `BlockStore_Create` - reports `SOLIDSYSLOG_CAT_BAD_CONFIG` / `BLOCKSTORE_ERROR_BLOCK_TOO_SMALL` and - falls back to `NullStore` (the established bad-setup contract). +- **Device is the single source of truth, but the store keeps a usable floor and warns + (Design Z).** The block-size source moves from config to the device; what changes vs + the old code is only the *too-small* case. A first attempt (Design Y) rejected an + undersized block outright (`ERROR` → `NullStore`). CI showed the old + `max(deviceSize, oneRecord)` clamp was far more load-bearing than realised — it was a + *deliberate, documented* cross-target calibration mechanism (block size MAX-coupled + via the clamp), relied on by the drain-ordering tests **and** the + `store_capacity` / `capacity_threshold` / `block_lifecycle` BDD suites across host and + freertos-cross. Rejecting broke all of them, and re-deriving the per-target message-fit + math to keep them working is exactly the fragile calculation that has bitten this repo + before. So the clamp stays — a device whose block can't hold one worst-case record is + grown to that floor — but now emits a `WARNING` (`SOLIDSYSLOG_CAT_BAD_CONFIG` / + `BLOCKSTORE_ERROR_BLOCK_TOO_SMALL`): delivered, degraded, no longer silent. The store + still works; `GetBlockSize` reports the device's requested value. - **`_Create(file, prefix, blockSize)` takes the size at construction**, preserving the BDD targets' runtime `--max-block-size` knob (they route the parsed value into the device instead of the store config). - **`SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE` (8192), `0` selects it.** Floored at one - worst-case record so passing 0 always clears the reject; FatFs adapter adds a - compile-time `FF_MAX_SS` floor on the default. + worst-case record; FatFs adapter adds a `defined()`-guarded compile-time `FF_MAX_SS` + floor on the default. -### Sliced (5 commits, strict red/green) +### Sliced (strict red/green) 1. `GetBlockSize` on the contract → 2. FileBlockDevice takes the size → 3. store -reads the device, `MaxBlockSize` dropped from config → 4. default tunable + `0→default` -→ 5. Design Y reject. 1396 host tests green throughout. +reads the device, `MaxBlockSize` dropped from config (clamp retained) → 4. default +tunable + `0→default` → 5. WARNING on clamp. 1396 host tests green throughout. ### Worth remembering -- The old clamp was **load-bearing in a documented way**: a drain-ordering test set - `maxBlockSize=200` and commented *"will clamp up so each block holds exactly one - record."* Design Y made that an error, so those tests now size blocks explicitly to - one near-max record. Real implication for integrators: a block smaller than one - max-size message is now rejected, not silently grown. +- **The clamp is a load-bearing, documented feature, not an accident.** + `syslog_steps.py` literally comments that the store_capacity scenarios *depend* on the + clamp making block size MAX-coupled. Design Y (reject) is wrong here precisely because + it removes that. Keep the clamp; make it loud, not silent. +- A `WARNING` is safe in the BDD targets: their stderr handler `_Exit(3)`s only on + `Severity <= ERROR`; a `WARNING` just prints and the process survives — which is why + the Y reject (ERROR) killed the BDD targets and Z (WARNING) does not. - Size-varying BlockStore tests re-point the fixture device through an **idempotent** `EnsureDeviceBlockSize` helper — same-size calls reuse the device so corruption- recovery's single persistent file handle survives a destroy/recreate. diff --git a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp index 1acf4dba..fb6c6fde 100644 --- a/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp +++ b/Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp @@ -41,16 +41,6 @@ extern "C" static const char* const TEST_PATH_PREFIX = "/tmp/draintest_"; -/* The store rejects a device whose block cannot hold one worst-case record - * (max message + framing). Sized just over that minimum and paired with a - * near-max payload, each block holds exactly one record — the calibration the - * drain-ordering scenarios need to span multiple blocks with only a handful of - * writes. (Previously these tests passed a tiny size and leaned on a silent - * clamp; the device is now the source of truth and an undersized block is a - * hard error, so the size is explicit here.) */ -static const size_t ONE_RECORD_BLOCK_SIZE = SOLIDSYSLOG_MAX_MESSAGE_SIZE + 16U; -static const size_t NEAR_MAX_PAYLOAD = SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U; - /* SenderSpy — sticky outage mode (every Send returns false until cleared) * and a vector of every *successful* send. Bigger than SenderFake's * last-only capture and one-shot FailNextSend; the BDD reproducer needs @@ -321,8 +311,8 @@ TEST(ServiceDrainInterleave, DiscardNewestDoesNotLetNewestBypassOldestOnRecovery * with just a couple of messages. */ DrainTestConfig cfg = { /*maxBlocks=*/2, - /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, - /*payloadSize=*/NEAR_MAX_PAYLOAD, + /*maxBlockSize=*/200 /*grown to one record by the store*/, + /*payloadSize=*/SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100U, SOLIDSYSLOG_DISCARD_POLICY_NEWEST }; Setup(cfg); @@ -394,12 +384,8 @@ TEST(ServiceDrainInterleave, DiscardNewestDoesNotLetNewestBypassOldestOnRecovery * we have the bug in our hands. */ TEST(BlockStoreDrainOrdering, OutageDrainProducesAscendingSequenceIds) { - DrainTestConfig cfg = { - /*maxBlocks=*/2, - /*maxBlockSize=*/ONE_RECORD_BLOCK_SIZE, - /*payloadSize=*/NEAR_MAX_PAYLOAD, - SOLIDSYSLOG_DISCARD_POLICY_NEWEST - }; + DrainTestConfig cfg = + {/*maxBlocks=*/2, /*maxBlockSize=*/200, /*payloadSize=*/64, SOLIDSYSLOG_DISCARD_POLICY_NEWEST}; CreateStore(cfg); /* Pre-outage send + drain — mirrors `When the client sends a message` diff --git a/Tests/SolidSyslogBlockStoreTest.cpp b/Tests/SolidSyslogBlockStoreTest.cpp index 45b71e7c..93d4b0ce 100644 --- a/Tests/SolidSyslogBlockStoreTest.cpp +++ b/Tests/SolidSyslogBlockStoreTest.cpp @@ -551,16 +551,18 @@ TEST(SolidSyslogBlockStoreConfig, ZeroBlockSizeUsesDeviceDefault) VerifyWriteAndReadBack(); } -TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeRejectsToNullStore) +TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeIsGrownAndStoreWorks) { CreateWithMaxBlockSize(1); - POINTERS_EQUAL(SolidSyslogNullStore_Get(), store); + CHECK(store != SolidSyslogNullStore_Get()); + VerifyWriteAndReadBack(); } -TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeReportsBadConfig) +TEST(SolidSyslogBlockStoreConfig, BelowMinimumBlockSizeReportsWarning) { ErrorHandlerFake_Install(nullptr); CreateWithMaxBlockSize(1); + LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_WARNING, ErrorHandlerFake_LastSeverity()); UNSIGNED_LONGS_EQUAL(SOLIDSYSLOG_CAT_BAD_CONFIG, ErrorHandlerFake_LastCategory()); UNSIGNED_LONGS_EQUAL(BLOCKSTORE_ERROR_BLOCK_TOO_SMALL, ErrorHandlerFake_LastDetail()); } diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 9cf70ed5..3cbc2454 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -116,7 +116,9 @@ misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:59 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:61 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:63 misra-c2012-11.8:Core/Source/SolidSyslogMessageFormatter.c:67 -misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:44 +misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:46 +misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:113 +misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:139 misra-c2012-11.8:Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:159 misra-c2012-11.8:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:97