diff --git a/Core/Source/BlockSequenceStatic.c b/Core/Source/BlockSequenceStatic.c index 7c6c1212..ee0bfab3 100644 --- a/Core/Source/BlockSequenceStatic.c +++ b/Core/Source/BlockSequenceStatic.c @@ -34,7 +34,8 @@ void BlockSequence_Destroy(struct BlockSequence* blockSequence) size_t index = BlockSequence_IndexFromHandle(blockSequence); if (SolidSyslogPoolAllocator_IndexIsValid(&BlockSequence_Allocator, index)) { - SolidSyslogPoolAllocator_FreeIfInUse(&BlockSequence_Allocator, index, BlockSequence_CleanupAtIndex, NULL); + (void + ) SolidSyslogPoolAllocator_FreeIfInUse(&BlockSequence_Allocator, index, BlockSequence_CleanupAtIndex, NULL); } } } diff --git a/Core/Source/RecordStore.c b/Core/Source/RecordStore.c index 06cf0689..1c8c7e44 100644 --- a/Core/Source/RecordStore.c +++ b/Core/Source/RecordStore.c @@ -122,9 +122,15 @@ static inline void RecordStore_AssembleRecord(struct RecordStore* recordStore, c RecordStore_MagicAddress(recordStore)[0] = MAGIC_BYTE_0; RecordStore_MagicAddress(recordStore)[1] = MAGIC_BYTE_1; + /* Length is packed little-endian into the byte buffer — the on-disk + * format is LE regardless of host (every supported target is LE; an + * explicit pack keeps that invariant readable and side-steps MISRA + * 21.15 which forbids memcpy between incompatible essential types). */ uint16_t length = (uint16_t) size; - memcpy(RecordStore_LengthAddress(recordStore), &length, RECORD_LENGTH_SIZE); - memcpy(RecordStore_MessageAddress(recordStore), data, size); + uint8_t* lengthBytes = RecordStore_LengthAddress(recordStore); + lengthBytes[0] = (uint8_t) (length & 0xFFU); + lengthBytes[1] = (uint8_t) ((length >> 8) & 0xFFU); + (void) memcpy(RecordStore_MessageAddress(recordStore), data, size); recordStore->SecurityPolicy->ComputeIntegrity( RecordStore_IntegrityRegionAddress(recordStore), @@ -246,9 +252,9 @@ static inline bool RecordStore_IsMagicValid(struct RecordStore* recordStore) static inline uint16_t RecordStore_RecordLength(struct RecordStore* recordStore) { - uint16_t length = 0; - memcpy(&length, RecordStore_LengthAddress(recordStore), RECORD_LENGTH_SIZE); - return length; + /* Little-endian unpack — see AssembleRecord for the format invariant. */ + const uint8_t* lengthBytes = RecordStore_LengthAddress(recordStore); + return (uint16_t) (((uint16_t) lengthBytes[0]) | (((uint16_t) lengthBytes[1]) << 8)); } static inline bool RecordStore_IsValidLength(uint16_t length) @@ -329,7 +335,7 @@ static inline void RecordStore_CopyRecordData( ) { size_t copySize = RecordStore_BoundedSize(length, maxSize); - memcpy(dst, RecordStore_MessageAddress(recordStore), copySize); + (void) memcpy(dst, RecordStore_MessageAddress(recordStore), copySize); *bytesRead = copySize; } @@ -474,7 +480,7 @@ static inline bool RecordStore_IsRecordSent( ) { uint8_t flag = SENT_FLAG_SENT; - SolidSyslogBlockDevice_Read( + (void) SolidSyslogBlockDevice_Read( blockDevice, blockIndex, RecordStore_SentFlagOffset(recordStore, recordStart, length), diff --git a/Core/Source/RecordStoreStatic.c b/Core/Source/RecordStoreStatic.c index 8165b3c2..bde72a87 100644 --- a/Core/Source/RecordStoreStatic.c +++ b/Core/Source/RecordStoreStatic.c @@ -33,7 +33,8 @@ void RecordStore_Destroy(struct RecordStore* recordStore) size_t index = RecordStore_IndexFromHandle(recordStore); if (SolidSyslogPoolAllocator_IndexIsValid(&RecordStore_Allocator, index)) { - SolidSyslogPoolAllocator_FreeIfInUse(&RecordStore_Allocator, index, RecordStore_CleanupAtIndex, NULL); + (void + ) SolidSyslogPoolAllocator_FreeIfInUse(&RecordStore_Allocator, index, RecordStore_CleanupAtIndex, NULL); } } } diff --git a/Core/Source/SolidSyslogBlockStoreStatic.c b/Core/Source/SolidSyslogBlockStoreStatic.c index 62244abd..c751a891 100644 --- a/Core/Source/SolidSyslogBlockStoreStatic.c +++ b/Core/Source/SolidSyslogBlockStoreStatic.c @@ -52,12 +52,12 @@ struct SolidSyslogStore* SolidSyslogBlockStore_Create(const struct SolidSyslogBl else { RecordStore_Destroy(recordStore); - SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); + (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); } } else { - SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); + (void) SolidSyslogPoolAllocator_FreeIfInUse(&BlockStore_Allocator, index, NULL, NULL); } } diff --git a/DEVLOG.md b/DEVLOG.md index 4ac0c42d..ae973042 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,185 @@ # Dev Log +## 2026-05-22 — S10.18 Storage stack conformance + +Closes S10.18 (#430). Seventh per-group conformance story in E10. Storage +stack cluster: `Store`, `BlockStore`, `RecordStore`, `BlockSequence`, +`BlockDevice`, `FileBlockDevice`, `File`, plus the POSIX / Windows / FatFs +file impls. Started from the CI report at run 26309037474 (`main@a062d39`, +post-S10.17), which surfaced 10 unsuppressed cppcheck-misra findings in +scope; clang-tidy was clean. + +### Headline + +Four code-touching commits (Patterns A, B, C, E), two pure-review +patterns (D and F), one clang-format follow-up. No new tree-wide +findings; 10 in-scope findings cleared; one new deviation D.012 +authorising the FILE_EXTENSION 8.9 tracker false positive. + +### Pattern A — `(void)` cast on `PoolAllocator_FreeIfInUse` (MISRA 17.7) + +`SolidSyslogPoolAllocator_FreeIfInUse` returns `bool` (false when the +slot wasn't ours / already free). Four call sites in pool-backed +`*Static.c` Destroy and Create-rollback paths discard the return — +the cleanup is best-effort. Explicit `(void)` cast at each site: +`BlockSequenceStatic.c:37`, `RecordStoreStatic.c:36`, +`SolidSyslogBlockStoreStatic.c:55/:60`. + +### Pattern B — discarded returns in `RecordStore.c` (MISRA 17.7 + 21.15) + +Five 17.7 sites in `RecordStore.c`. Treated per-site: + +- Lines 127, 332: `(void) memcpy(...)` — copy into a byte buffer from + an opaque source (`const void* data` / `void* dst`). +- Line 477: `(void) SolidSyslogBlockDevice_Read(...)` — + `RecordStore_IsRecordSent` deliberately falls through to + `flag == SENT_FLAG_SENT` on a read failure during the + corruption-recovery scan. The function doc-comment already documents + the fail-safe; a skipped record surfaces downstream as a sequenceId + gap. +- Lines 126, 250: the bare `(void) memcpy` cast didn't satisfy MISRA — + it unmasked rule 21.15 ("pointer arguments shall be to compatible + essential types") because the writes/reads went between `uint8_t*` + and `uint16_t*`. Refactored to explicit little-endian byte + pack/unpack: + + lengthBytes[0] = (uint8_t) (length & 0xFFU); + lengthBytes[1] = (uint8_t) ((length >> 8) & 0xFFU); + + Every supported target is little-endian (POSIX x86/x64/ARM, Windows + x86/x64, FreeRTOS-Cortex-M3 on QEMU); the existing + `SolidSyslogBlockStoreTest.cpp:1555` truncated-header fixture + already bakes in LE layout (`{0xA5, 0x5A, 0x05}` — length 5 packed + as `0x05 0x00`). Pure refactor on every host; the explicit shape + locks the on-disk format invariant in code rather than leaving it + implicit. + +### Pattern C — D.012 deviation for `FILE_EXTENSION` 8.9 + +`Core/Source/SolidSyslogFileBlockDevice.c:20` declares +`static const char FILE_EXTENSION[] = ".log"`. The constant is +referenced from **two** sites in the TU — a file-scope enum +`sizeof()` at line 25 (contributing to `FILENAME_SUFFIX` and the +derived compile-time `MAX_PREFIX_LENGTH`) and `_FormatBlockFilename` +at line 214. cppcheck-misra's 8.9 tracker counts only function-scope +references, so the enum site is invisible and the rule fires +spuriously. + +Three alternatives were investigated before recording D.012: + +1. **Inline the literal at both call sites** — DRY violation for a + single-source-of-truth on-disk constant. +2. **Promote the dependent enum entries to file-scope + `static const size_t`** — *verified experimentally during S10.18 + that this does not satisfy 8.9*; instead surfaces a second 8.9 + false positive on the new constant for the same reason + (function-scope tracker treats all file-scope references + uniformly). The fix path amplifies the problem rather than + resolving it. +3. **Promote to `#define FILE_EXTENSION ".log"`** — side-steps 8.9 + (macros are not objects) at the cost of introducing a string + macro inconsistent with the file-scope-const pattern used + elsewhere in storage code. + +New deviation D.012 in `docs/misra-deviations.md`; single-site +line-anchored suppression in `misra_suppressions.txt`. The story +body specified "no new deviation rows in `docs/misra-deviations.md`" +— D.012 is a divergence from that, raised as work per the per-group +conformance workflow's "raise as work" clause. The per-site review +surfaced a genuine cppcheck-misra tracker false positive rather than +a fixable code defect. + +### Pattern D — D.002 11.3 vtable downcasts (no work) + +Five sites — `BlockStore.c:69`, `FileBlockDevice.c:102`, +`FatFsFile.c:45`, `PosixFile.c:66`, `WindowsFile.c:73` — are all +textbook `*_SelfFromBase` vtable downcasts established by S10.11. +Anchors are correct, rationale holds, no code change. + +### Pattern E — D.003 → D.009 migration of 8 anonymous-enum 5.7 sites + +Per-site verification confirmed every D.003 / 5.7 entry in storage +scope is an anonymous-enum site, not the struct-tag pattern D.003 +originally covered. All 8 migrated to the D.009 block. No 2.4 +entries surfaced after the migration — the cppcheck dedup pattern +that unmasked `UdpPayload.h:15` in S10.17 doesn't fire here (no +inclusion-chain narrowing). + +Out-of-scope D.003 5.7 entries kept where they are: `CircularBuffer.c` +(closed S10.12), `Crc16.c` / `Crc16Policy.c` (closed S10.13), and +the SolidSyslog.c + various platform impls deferred to S10.19. Same +fix-when-we-see-it precedent S10.16 set. + +**Side observation captured during the migration.** I tried +typedef-ing one of the anonymous enums (`BlockSequence.c:13`, +`MIN_MAX_BLOCKS` etc.) as `typedef enum { … } BlockSequenceLimits` +to see whether the typedef satisfied 5.7. It did — the 5.7+2.4 +findings on the declaration vanished. But it immediately surfaced +**two new 10.4 findings** at the arithmetic use sites (`current + 1U` +and `+ SEQUENCE_MODULUS` arithmetic mixing an `enum BlockSequenceLimits` +member with `uint8_t` / unsigned-int operands violates 10.4's +essential-type-category rule). Net trade: 2-per-declaration becomes +N-per-use-site, where N is typically larger. The D.009 deviation +implicitly captures this — the anonymous-enum-as-constant-container +pattern is the cheaper shape under MISRA. + +### Pattern F — D.004 / D.006 / D.008 per-site re-justification (no work) + +Seven sites: + +- **D.004 / 18.4** — `RecordStore.c:37/:42/:47/:52`: textbook + field-offset pointer arithmetic chain inside the contiguous + record buffer (`MagicAddress + MAGIC_SIZE` → `LengthAddress` → + `MessageAddress` → `IntegrityChecksumAddress` → `SentFlagAddress`). + Exactly what D.004 authorises. +- **D.006 / 11.8** — `BlockSequence.c:465` (const-receiver path + strips through to `BlockDevice_Size`'s non-const param) and + `BlockStoreStatic.c:39` (`config->SecurityPolicy` assigned to + non-const local). Both real false positives, same shape as + documented. +- **D.008 / 21.6** — `WindowsFile.c:8` `#include ` for + `SEEK_SET` / `SEEK_END` only. + +All hold. No anchor drift after Patterns A/B/C. + +### clang-format follow-up + +`(void) SolidSyslogPoolAllocator_FreeIfInUse(...)` exceeds line +length even with arguments inline; clang-format splits between +`(void` and `)`. Ugly but mechanically what the formatter chose; +accepted rather than adding helper scaffolding to dodge it. + +### Acceptance + +- Zero in-scope cppcheck-misra unsuppressed findings (10 cleared). +- Zero in-scope `analyze-tidy` warnings. +- Tree-wide unsuppressed total: 65 → 65 (no new findings outside scope). +- 1290 / 1290 tests pass on `debug` and `sanitize` (ASan + UBSan). +- Tree-wide coverage 99.9% (2925 / 2929 lines, 602 / 602 functions); + +1 line covered vs. main as a side effect of Pattern B's byte-pack + rewrite exercising paths the old `memcpy`-into-buffer path skipped. +- clang-format clean tree-wide (within the CI scope: + `Core/Interface Core/Source Tests Bdd/Targets`). +- One new deviation row D.012; no other changes to + `docs/misra-deviations.md`. + +### Carry-forward for S10.19 / S10.20 + +- Engine + Formatter cluster owns 38 remaining unsuppressed findings + in `Core/Source/SolidSyslog.c`, `Core/Source/SolidSyslogFormatter.c`, + `Core/Source/SolidSyslogErrorMessages.h`, `Core/Source/SolidSyslogStatic.c` + — all S10.19's job. +- 2 × 8.7 findings in `Platform/Atomics/Source/SolidSyslog{Std,Windows}AtomicCounter.c` + are orphans from closed S10.13. Fix-when-touched, or sweep at S10.20. +- D.003 → D.009 migration still has out-of-scope sister sites to + visit: `CircularBuffer.c:15`, `Crc16.c:12`, `Crc16Policy.c:10` + (closed S10.12/S10.13), plus `SolidSyslog.c:30` and the platform + Hostname / SysUpTime / Clock impls — these belong to S10.19 (engine) + and the closed S10.14 (config + platform helpers); same + fix-when-we-see-it precedent. + +--- + ## 2026-05-22 — S10.17 Network primitives conformance Closes S10.17 (#428). Sixth per-group conformance story in E10. Network diff --git a/docs/misra-deviations.md b/docs/misra-deviations.md index c7e928db..cae92cc1 100644 --- a/docs/misra-deviations.md +++ b/docs/misra-deviations.md @@ -828,3 +828,86 @@ The alternatives all regress: Project owner — David Cozens. Recorded under [S10.10](https://github.com/DavidCozens/solid-syslog/issues/375). +--- + +## D.012 — Rule 8.9: file-scope `static const` referenced from a file-scope enum + one function + +### Rule + +> **Rule 8.9 (Advisory)** — An object should be defined at block scope +> if its identifier only appears in a single function. + +### Deviation + +`Core/Source/SolidSyslogFileBlockDevice.c:20` declares +`static const char FILE_EXTENSION[] = ".log"`. The constant is the +single source of truth for the on-disk filename extension and is +referenced from two places in the translation unit: + +1. The file-scope enum at line 25 — `sizeof(FILE_EXTENSION) - 1U` + contributes to `FILENAME_SUFFIX`, which in turn computes + `MAX_PREFIX_LENGTH` (an integer constant expression consumed by + the formatter at the call site). +2. `FileBlockDevice_FormatBlockFilename` at line 214 — both the + bytes pointer and the runtime length are derived from the same + constant. + +cppcheck-misra's 8.9 tracker counts only function-scope references. +The file-scope enum reference at line 25 is invisible to it, so it +sees a single function reference (line 214) and reports the constant +as having "block-scope-only" usage — even though moving it into the +function would break the enum's compile-time `sizeof()` evaluation. + +### Scope + +`Core/Source/SolidSyslogFileBlockDevice.c:20` — one declaration. + +A future per-component sweep may surface this shape on other +file-scope `static const` objects whose identifier is used by a +file-scope enum's `sizeof()`/value initialiser **and** exactly one +function. When that happens, the deviation extends to those files; +the rule still catches genuinely single-function-scoped objects. + +### Rationale + +The constant cannot move to block scope without regressing the code. +Three alternatives were considered and rejected: + +- Inlining the literal at the enum site and at the call site would + introduce a second copy of `".log"`, violating DRY for what is + effectively a single on-disk format invariant. +- Promoting the constant's dependents from enum entries to file-scope + `static const size_t` (verified experimentally during S10.18) does + **not** satisfy the rule — the tracker treats file-scope references + uniformly, so a new `static const size_t FILENAME_SUFFIX` trips a + *second* 8.9 finding for the same reason. The fix path amplifies + the problem rather than resolving it. +- Promoting to `#define FILE_EXTENSION ".log"` would side-step rule + 8.9 (macros are not objects) at the cost of introducing a string + macro where the codebase otherwise uses `static const`. Rejected + to keep the file-scope-const idiom consistent across the tree. + +Summary: + +| Alternative | Why rejected | +|-------------|--------------| +| Inline `cppcheck-suppress misra-c2012-8.9` at the declaration | Inline suppressions are weaker by MISRA Compliance:2020 §4.2 (rationale scattered, not centrally auditable). Project preference is structural deviations in this document. | +| Inline the `".log"` literal at both use sites | DRY violation for a single-source-of-truth on-disk constant. | +| Promote the dependent enum entries to `static const size_t` | Verified to **not** satisfy 8.9; instead surfaces a second false positive on the new constant. | +| Promote to `#define FILE_EXTENSION ".log"` | Introduces a string macro inconsistent with the file-scope-const pattern used elsewhere in storage code. | + +### Risk and mitigation + +- **Genuinely single-function-scoped constants.** A new file-scope + `static const` whose identifier really appears in only one + function would still be a defect; this deviation is line-specific, + so a fresh 8.9 finding elsewhere still surfaces in CI. +- **Elimination path.** If a future cppcheck-misra version teaches + its 8.9 tracker to count file-scope-initialiser references, the + suppression becomes unnecessary and can be removed. + +### Approval + +Project owner — David Cozens. Recorded under +[S10.18](https://github.com/DavidCozens/solid-syslog/issues/430). + diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 3d653bb2..d26744fd 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -80,25 +80,17 @@ misra-c2012-5.7:Core/Interface/SolidSyslogFormatter.h:14 misra-c2012-5.7:Core/Interface/SolidSyslogSecurityPolicyDefinition.h:13 misra-c2012-5.7:Core/Interface/SolidSyslogTimeQuality.h:12 misra-c2012-5.7:Core/Interface/SolidSyslogTransport.h:5 -misra-c2012-5.7:Core/Source/BlockSequence.c:13 -misra-c2012-5.7:Core/Source/BlockSequence.c:107 -misra-c2012-5.7:Core/Source/RecordStore.c:14 -misra-c2012-5.7:Core/Source/RecordStorePrivate.h:14 misra-c2012-5.7:Core/Source/SolidSyslog.c:30 misra-c2012-5.7:Core/Source/SolidSyslogCircularBuffer.c:15 misra-c2012-5.7:Core/Source/SolidSyslogCrc16.c:12 misra-c2012-5.7:Core/Source/SolidSyslogCrc16Policy.c:10 -misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:16 -misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:23 misra-c2012-5.7:Core/Source/SolidSyslogOriginSdPrivate.h:9 misra-c2012-5.7:Platform/FreeRtos/Source/SolidSyslogFreeRtosSysUpTime.c:7 -misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixFile.c:18 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixHostname.c:10 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c:17 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h:11 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixSysUpTime.c:6 misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWindowsClock.c:21 -misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWindowsFile.c:24 misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWindowsHostname.c:19 misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWindowsSysUpTime.c:18 @@ -163,6 +155,15 @@ misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixSleep.c:6 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixTcpStream.c:24 misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWinsockResolver.c:37 misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:100 +# Storage stack anonymous-enum 5.7 sites — migrated from D.003 by S10.18. +misra-c2012-5.7:Core/Source/BlockSequence.c:13 +misra-c2012-5.7:Core/Source/BlockSequence.c:107 +misra-c2012-5.7:Core/Source/RecordStore.c:14 +misra-c2012-5.7:Core/Source/RecordStorePrivate.h:14 +misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:16 +misra-c2012-5.7:Core/Source/SolidSyslogFileBlockDevice.c:23 +misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixFile.c:18 +misra-c2012-5.7:Platform/Windows/Source/SolidSyslogWindowsFile.c:24 # D.010 — Rule 20.10: # stringification in _Static_assert polyfill # See docs/misra-deviations.md#d010 @@ -209,3 +210,7 @@ misra-c2012-2.5:Core/Source/SolidSyslogErrorMessages.h:136 misra-c2012-2.5:Core/Source/SolidSyslogErrorMessages.h:138 misra-c2012-2.5:Core/Source/SolidSyslogErrorMessages.h:140 misra-c2012-2.5:Core/Source/SolidSyslogErrorMessages.h:142 + +# D.012 — Rule 8.9: file-scope static const referenced from a file-scope enum + one function +# See docs/misra-deviations.md#d012 +misra-c2012-8.9:Core/Source/SolidSyslogFileBlockDevice.c:20