diff --git a/.clang-tidy b/.clang-tidy index 837fbf13..88672787 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,11 +31,8 @@ Checks: > -modernize-deprecated-headers, -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -# Treat all clang-tidy warnings as errors EXCEPT the new naming check, -# which lands in warning mode for the duration of E10's soft-freeze -# (S10.02 through S10.18). S10.18 will flip it back to error mode once -# the sweeps in S10.07 onwards have cleared the backlog. -WarningsAsErrors: '*,-readability-identifier-naming' +# Treat all clang-tidy warnings as errors. +WarningsAsErrors: '*' # Only apply to project headers, not third-party or system headers. HeaderFilterRegex: '.*Interface/.*' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index afcf0bf6..452c85a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -486,7 +486,7 @@ jobs: - name: Check clang-format run: | - find Core/Interface Core/Source Tests Bdd/Targets -name '*.c' -o -name '*.cpp' -o -name '*.h' \ + find Core/Interface Core/Source Platform Tests Bdd/Targets -name '*.c' -o -name '*.cpp' -o -name '*.h' \ | xargs clang-format --dry-run --Werror analyze-cppcheck: @@ -517,8 +517,22 @@ jobs: --inline-suppr --std=c11 -ICore/Interface + -IPlatform/Atomics/Interface + -IPlatform/Posix/Interface + -IPlatform/Windows/Interface + -IPlatform/OpenSsl/Interface + -IPlatform/MbedTls/Interface + -IPlatform/FreeRtos/Interface + -IPlatform/FatFs/Interface --xml --xml-version=2 Core/Source/ + Platform/Atomics/Source/ + Platform/Posix/Source/ + Platform/Windows/Source/ + Platform/OpenSsl/Source/ + Platform/MbedTls/Source/ + Platform/FreeRtos/Source/ + Platform/FatFs/Source/ 2> build/cppcheck/cppcheck-report.xml - name: Upload cppcheck report @@ -530,26 +544,19 @@ jobs: retention-days: 1 # ---------------------------------------------------------------- - # cppcheck-misra addon — S10.03, warning mode. - # - # Runs the MISRA C:2012 addon over Strict + Pragmatic tiers - # (Core/Source/ + Platform/*/Source/). --error-exitcode=0 keeps - # the step non-fatal: diagnostics print to the CI log and an XML - # artifact uploads, but the job continues regardless. - # - # S10.18 will collapse this and the non-MISRA pass above into a - # single invocation when both reach the same weight (error mode); - # the existing pass's scope will widen to Platform/*/Source/ at - # that point — see project_e10_accumulated_scope.md. + # cppcheck-misra addon — runs the MISRA C:2012 addon over Strict + + # Pragmatic tiers (Core/Source/ + Platform/*/Source/). Findings + # fail CI (--error-exitcode=1); suppressions live in + # misra_suppressions.txt with rationales in docs/misra-deviations.md. # ---------------------------------------------------------------- - - name: Run cppcheck-misra (warning mode) + - name: Run cppcheck-misra if: success() || failure() run: > cppcheck --addon=misra --suppressions-list=misra_suppressions.txt - --error-exitcode=0 + --error-exitcode=1 --inline-suppr --std=c11 -ICore/Interface @@ -557,6 +564,7 @@ jobs: -IPlatform/Posix/Interface -IPlatform/Windows/Interface -IPlatform/OpenSsl/Interface + -IPlatform/MbedTls/Interface -IPlatform/FreeRtos/Interface -IPlatform/FatFs/Interface Core/Source/ @@ -564,6 +572,7 @@ jobs: Platform/Posix/Source/ Platform/Windows/Source/ Platform/OpenSsl/Source/ + Platform/MbedTls/Source/ Platform/FreeRtos/Source/ Platform/FatFs/Source/ @@ -574,7 +583,7 @@ jobs: cppcheck \ --addon=misra \ --suppressions-list=misra_suppressions.txt \ - --error-exitcode=0 \ + --error-exitcode=1 \ --inline-suppr \ --std=c11 \ -ICore/Interface \ @@ -582,6 +591,7 @@ jobs: -IPlatform/Posix/Interface \ -IPlatform/Windows/Interface \ -IPlatform/OpenSsl/Interface \ + -IPlatform/MbedTls/Interface \ -IPlatform/FreeRtos/Interface \ -IPlatform/FatFs/Interface \ --xml --xml-version=2 \ @@ -590,6 +600,7 @@ jobs: Platform/Posix/Source/ \ Platform/Windows/Source/ \ Platform/OpenSsl/Source/ \ + Platform/MbedTls/Source/ \ Platform/FreeRtos/Source/ \ Platform/FatFs/Source/ \ 2> build/cppcheck-misra/cppcheck-misra-report.xml diff --git a/DEVLOG.md b/DEVLOG.md index f0f412c7..47087376 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,124 @@ # Dev Log +## 2026-05-23 — S10.20 close-out: flip MISRA + naming gates to error mode + +Closes S10.20 (#437) and E10 (#12). Bundle of CI gate flips and audit- +phase paperwork now that S10.12–S10.19 have left the tree with zero +cppcheck-misra findings and zero clang-tidy naming warnings. + +### What landed + +1. **cppcheck-misra now gates CI.** `analyze-cppcheck` job — both the + "Run cppcheck-misra" and "Generate cppcheck-misra XML report" steps + flipped `--error-exitcode=0` → `1`. The stale "warning mode" comment + block was replaced with a one-paragraph note describing the current + contract. +2. **MbedTls added to the cppcheck-misra invocation.** + `-IPlatform/MbedTls/Interface` and `Platform/MbedTls/Source/` + appended to both invocations. +3. **Non-MISRA cppcheck XML pass widened.** Diagnostic XML pass scoped + from `Core/Source/` only to the full `Core/Source/` + + `Platform/*/Source/` tree (matches the cppcheck-misra scope). + Build-time cppcheck via `CMAKE_C_CPPCHECK` already gated Platform/; + this widens the artefact upload to match. +4. **clang-tidy `readability-identifier-naming` now gates CI.** Root + `.clang-tidy` flipped `WarningsAsErrors: '*,-readability-identifier-naming'` + → `'*'`. The soft-freeze paragraph (which referenced the wrong story + number — S10.18 vs S10.20) was replaced with a single-line current- + state comment. +5. **`analyze-format` scope widened** to include `Platform/`. The + find-root list went from `Core/Interface Core/Source Tests Bdd/Targets` + to `Core/Interface Core/Source Platform Tests Bdd/Targets`. +6. **`docs/misra-conformance.md` deleted.** Audit working doc, frozen + since S10.12, slated for removal at the gate flip. Live truth lives + in `docs/misra-deviations.md` (rationales) and + `misra_suppressions.txt` (per-site state). DEVLOG history retains + references — those are point-in-time records and don't need rewriting. +7. **DatagramFake.h comment nit** from the S10.22 deferred list + reworded for clarity (`Tests/DatagramFake.h:18-21`). + +### MbedTls verification turned into a small cleanup + +The MbedTls inclusion was framed in the story body as a verification +step — the assumption was that the adapter had been developed against +the local cppcheck-misra check and would land clean. Re-running +revealed 7 findings, all in `SolidSyslogMbedTlsStream.c`: + +| Site | Rule | Disposition | +|------|------|-------------| +| line 14 — anon `enum { HANDSHAKE_TIMEOUT_MILLISECONDS, … }` | 5.7 | D.009 extension (anonymous-enum named-constant container; identical shape to `PosixSleep.c:6`). | +| line 60 — `(struct SolidSyslogMbedTlsStream*) base` in SelfFromBase | 11.3 | D.002 extension (vtable downcast). | +| line 190 — `(struct SolidSyslogMbedTlsStream*) ctx` in BioSend | 11.5 | D.002 extension (opaque-handle downcast — mbedTLS BIO `void*` ctx carries our `self`). | +| line 202 — same in BioRecv | 11.5 | D.002 extension. | +| line 211 — `if (n > 0) { … } else if (n == 0) { … }` no trailing `else` | 15.7 | **Real per-site fix.** Added `else { /* n < 0 — transport-level error; keep result = -1 to signal a hard failure to mbedTLS … */ }`. No behaviour change — the `int result = -1;` initialiser already covered the n<0 path; the trailing `else` just makes it explicit. Same shape as the ~10 sites cleared in S10.10. | +| line 227 — `(const unsigned char*) buffer` in MbedTlsStream_Send | 11.5 | **New deviation D.013** (see below). | +| line 246 — `(unsigned char*) buffer` in MbedTlsStream_Read | 11.5 | New D.013. | + +### New deviation D.013 + +`void* ↔ unsigned char*` at third-party byte-buffer API boundaries. + +`SolidSyslogStream::Send`/`Read` use `void*` byte buffers (the +conventional C transport-API idiom — matches POSIX `send`/`recv` and +OpenSSL `SSL_write`/`SSL_read`). mbedTLS types its byte buffers as +`const unsigned char*` / `unsigned char*`, so the implementation cast +bridging the two is unavoidable at the API boundary. Rule 11.5 fires +on each. + +Alternatives considered and rejected: + +- Refactor `SolidSyslogStream` to use `unsigned char*` — ABI change + propagating to every Stream impl and `SolidSyslogStreamSender`; + wrong direction (one third-party API's typing choice shouldn't + drive a public-interface change). +- `memcpy` shim — runtime cost on the hot send/receive path. +- Inline suppressions — weaker than a centrally-auditable deviation + per MISRA Compliance:2020 §4.2. + +The cast is well-defined under C99 §6.5 ¶7 (`unsigned char` may alias +any object type), so reinterpreting a `void*` byte buffer as +`unsigned char*` at the third-party API boundary is a no-op at the +abstract-machine level. + +Scope: 2 sites today in `SolidSyslogMbedTlsStream.c`. Deviation +extends to any future Stream / Datagram / hash / MAC implementation +wrapping a byte-typed third-party C API. OpenSSL's `TlsStream.c` +explicitly **doesn't** fall under D.013 because `SSL_write`/`SSL_read` +already take `void*`. + +### Decisions + +- **Did not split out the MbedTls cleanup as a separate pre-PR.** + The work is small (6 suppression lines + 1 trailing-else + 1 new + deviation section) and the per-group conformance workflow that + S10.12 established is the standard recipe for exactly this case + (extend existing deviations, fix per-site, document). David + approved rolling into S10.20 after a per-site walkthrough. +- **Did not re-prove the gates fail on a deliberate violation.** The + gates fired dozens of times during S10.07–S10.19; re-proving here + would add PR churn without information. +- **Kept the non-MISRA XML pass scope-widen in scope** even though + build-time cppcheck already covers Platform/ via + `CMAKE_C_CPPCHECK`. The XML artefact should report the same surface + the gates do. +- **Kept the `analyze-format` Platform/ widening in scope** (epic + body listed it as "optional"). E10 close-out is the natural moment + and there's nothing pushing it back. + +### Deferred / follow-ups + +- **None for E10.** This closes the epic. +- A possible E10-successor refactor that adopts `unsigned char*` in + `SolidSyslogStream::Send`/`Read` would retire D.013, but is not + scheduled and the trade-offs (see D.013 rationale) suggest leaving + the `void*` contract in place. + +### Open questions + +None. + +--- + ## 2026-05-23 — S10.19 Engine + Formatter conformance Closes S10.19 (#432). Last per-group conformance story in E10. Engine + diff --git a/Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c b/Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c index e54e44ce..78632c53 100644 --- a/Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c +++ b/Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c @@ -210,6 +210,11 @@ static int MbedTlsStream_BioRecv(void* ctx, unsigned char* buf, size_t len) { result = MBEDTLS_ERR_SSL_WANT_READ; } + else + { + /* n < 0 — transport-level error; keep result = -1 to signal a + hard failure to mbedTLS so the handshake / read aborts. */ + } return result; } diff --git a/Tests/DatagramFake.h b/Tests/DatagramFake.h index 3eb35294..a788e57c 100644 --- a/Tests/DatagramFake.h +++ b/Tests/DatagramFake.h @@ -15,8 +15,10 @@ EXTERN_C_BEGIN struct SolidSyslogDatagram* DatagramFake_Create(void); void DatagramFake_Destroy(struct SolidSyslogDatagram * datagram); - /* Per-call SendTo result (0-based). Calls beyond the configured slots - * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT by default. */ + /* Per-call SendTo result (0-based). Configured slots default to + * SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT until overridden; SendTo + * calls beyond DATAGRAMFAKE_MAX_SEND_CALLS return + * SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED. */ void DatagramFake_SetSendResult( struct SolidSyslogDatagram * datagram, int callIndex, diff --git a/docs/misra-conformance.md b/docs/misra-conformance.md deleted file mode 100644 index 149c7fc4..00000000 --- a/docs/misra-conformance.md +++ /dev/null @@ -1,272 +0,0 @@ -# Conformance audit and backlog - -> **Frozen — audit complete (S10.05–S10.11 + S10.21 landed, per-group -> phase began with S10.12).** This document is the working record of -> the E10 audit. With the cross-cutting sweeps done and per-group -> conformance underway, the audit phase is closed. The live truth is -> CI plus the two source-of-truth documents: -> -> **S11.11 update.** Deviation D.006-original (rule 1.4 / C11 -> ``) was retired — cppcheck 2.10 no longer flags the -> site. The remaining deviations were renumbered down by one -> (old D.007–D.012 → new D.006–D.011). Cross-references in the -> tables below have been updated to point at the new numbers; the -> historical narrative (counts, original audit verdicts) is left -> intact. -> -> - `docs/misra-deviations.md` — the deviation rationales and -> approvals. New per-site suppressions land here, not in this doc. -> - `misra_suppressions.txt` — the cppcheck-misra suppressions list. -> -> No further per-rule updates are expected here. Expect to delete -> this file at S10.20 when the gates flip from warning to error and -> the suppressions list becomes the single live record. - -Working document tracking the naming and MISRA C:2012 violations -currently surfaced by clang-tidy and cppcheck-misra against the -SolidSyslog tree, with a per-rule proposed verdict that feeds the -rest of E10: - -- **S10.06** consumes this doc and applies the configuration - changes (which rules to disable, which deviations to add to - `docs/misra-deviations.md`, which need `misra_suppressions.txt` - entries). -- **S10.07–S10.17** consume the per-rule `Fix` verdicts and - execute the sweeps. -- **S10.18** flips the warning-mode gates back to error-mode once - the sweeps and deviations have cleared the backlog. - -## Snapshot - -- Source: `main` at commit `51196e2` (post-S10.04 reformat). -- Naming report regenerated by `cmake --preset tidy && cmake - --build --preset tidy` in the gcc dev container. -- MISRA report regenerated by `cppcheck --addon=misra ...` over - `Core/Source/` + `Platform/*/Source/`. -- Raw reports are not committed; the CI runs from S10.02 / S10.03 - upload `cppcheck-misra-report.xml` as artifacts on each build, - and the naming warnings live in the tidy build log. - -## Verdict definitions - -Each rule with non-zero findings carries exactly one of the -following verdicts: - -- **Fix** — the violation is genuine; the code is wrong relative to - the rule. Sweep target picked up by an S10.07+ story. -- **Deviate** — the violation is structural to a project-wide - convention (no-typedef-struct, vtable + caller-supplied storage, - flexible array members, etc.). The rule stays enabled but the - convention is documented in `docs/misra-deviations.md` (extended - in S10.06) with rationale. No per-site code change. -- **Disable** — the rule is structurally inappropriate for the - codebase (e.g. `cppcoreguidelines-pro-type-cstyle-cast` on C - code, disabled in S10.04). S10.06 applies the configuration - change. -- **Re-categorise** — the violation indicates that - `docs/NAMING.md` or the .clang-tidy configuration needs - adjustment, not the code. (Example: vtable function-pointer - members named in PascalCase to mirror the function they hold — - a deliberate convention NAMING.md has not yet acknowledged.) -- **Mixed** — some findings are **Fix**, others are **Deviate**. - The per-site split is deferred to S10.06 review rather than - forced into a single bucket here. (Example: rule 11.8 const-strip - casts — the Winsock platform-API site is Deviate; the Core - sites are Fix.) -- **Investigate** — the addon report needs further inspection - before a verdict can be assigned. Usually a "is this a transitive - include we can't help vs a direct one we can?" question. S10.06 - resolves to one of the verdicts above. - -## Headline counts - -| Source | Snapshot (pre-S10.05 decisions) | Post-S10.05 decisions | Distinct rules / kinds | -|--------|--------------------------------:|----------------------:|----------------------:| -| `clang-tidy readability-identifier-naming` | 404 | **446** *(Tier 4 PascalCase re-statement: +186 data-member findings, -128 vtable findings; Tier 1 second-shape addition: -12 library-top-level findings; Tier 2 tag-rule clarification: -4 file-scope tag findings)* | 5 identifier kinds | -| `cppcheck-misra` | 575 | 575 *(no MISRA rule changes in S10.05; verdict assignments only)* | 29 rules | -| **Grand total** | 979 | **1021** | — | - ---- - -## Naming — `readability-identifier-naming` - -| Kind | Count | Top sites | Verdict | Rationale | Owner | -|------|------:|-----------|---------|-----------|-------| -| **enum constant** | 252 → 101 *(named enums landed in S10.07; anonymous-enum idiom unchanged)* | `SolidSyslogPrival.h` (128 — `SOLIDSYSLOG_FACILITY_*`, `SOLIDSYSLOG_SEVERITY_*`), `SolidSyslogTransport.h` (24), `SolidSyslogBlockStore.h` (12) | **Mixed** | `docs/NAMING.md` already specifies `SolidSyslogClass_Constant` form (e.g. `SOLIDSYSLOG_SEVERITY_EMERGENCY`) for *tagged* enums — renamed across the tree in five per-enum slices on `feat/s10-07-enum-rename`. The remaining 101 findings are the anonymous-`enum { … };` named-constant idiom (`SOLIDSYSLOG_ADDRESS_SIZE`, `SOLIDSYSLOG_MAX_HOST_SIZE`, `INVALID_FD`, …) — functionally macros, currently classified by clang-tidy as enum constants and so flagged by the prefix/case rules. Needs its own verdict: either re-classify as macro-equivalent and tighten the relevant `.clang-tidy` regex, or rename to a class-prefixed PascalCase form. Loose thread surfaced during S10.07 verification; carried into the S10.07-sibling or its own ticket. | **S10.07** (named enums); follow-up for anonymous-enum idiom | -| **member** | 128 → 186 → 0 *(post Tier 4 re-statement; sweep landed in S10.09)* | Pre-change: `SolidSyslog*Definition.h` headers (vtable function-pointer members in PascalCase). Post-change: ~186 data members in lowerCamelCase across `SolidSyslog*Config`, impl structs, helper structs, and test fakes. | **Disable original + Fix — landed in S10.09** | The original 128 findings were all vtable function-pointer members in PascalCase, flagged because Tier 4 was previously `lowerCamelCase`. **Tier 4 was re-stated in S10.05 to `PascalCase` for all members** — a single rule with no member-kind exception. `.clang-tidy` flipped to `MemberCase: CamelCase`. The 128 vtable warnings disappeared and **S10.09 cleared the 186 lowerCamelCase data-member warnings** with a per-cluster sweep (Formatter → storage → buffer → sender impls → sender Configs + fakes → SD → sync/atomics → top-level public structs + impl `struct SolidSyslog`). `analyze-tidy` reports zero `readability-identifier-naming` member findings post-S10.09. | landed in **S10.09** | -| **global function** | 12 → 0 *(post Tier 1 second-shape addition)* | `SolidSyslogError.h` (8 — `SolidSyslog_SetErrorHandler`, `SolidSyslog_Error`), `SolidSyslog.h` (2 — `SolidSyslog_Log`, `SolidSyslog_Service`), `SolidSyslogConfig.h` (2 — `SolidSyslog_Create`, `SolidSyslog_Destroy`) | **Disable + landed in S10.05** | All six unique function names follow the form `SolidSyslog_` — the library-level API entry points. **NAMING.md Tier 1 was re-stated in this story** to recognise `SolidSyslog_` as a first-class shape for whole-library operations (alongside `SolidSyslog_` for class-scoped ones). `.clang-tidy` gained a `GlobalFunctionIgnoredRegexp` of `^SolidSyslog_[A-Z][A-Za-z0-9]*$` to accept the shape. The 12 warnings vanished in this commit. | NAMING.md + `.clang-tidy` landed here | -| **enum** | 8 → 0 *(landed in S10.07)* | `SolidSyslogPrival.h` (8 — `SolidSyslog_Facility`, `SolidSyslog_Severity`) | **Fix — landed in S10.07** | Enum tag names should be `SolidSyslogFacility` and `SolidSyslogSeverity` (no underscore) per NAMING.md Tier 1 struct/enum rule. The underscore is residue. Renamed alongside the enum constants in the same per-enum slices. | **S10.07** | -| **struct** | 4 → 0 *(post Tier 2 tag-rule clarification)* | `SolidSyslog.c` (`SolidSyslog` — opaque-impl), `SolidSyslogFormatter.c` (`EscapedContext`), `BlockSequence.c` (`BlockPresence`), `SolidSyslogFileBlockDevice.c` (`OpenHandle`) | **Disable + landed in S10.05** | Three are Tier 2 file-scope helper tags (`EscapedContext`, `BlockPresence`, `OpenHandle`) and one is the opaque-impl case (`struct SolidSyslog` matches the public opaque tag verbatim). **NAMING.md Tier 2 was extended in this story** to state that file-scope struct tags use bare PascalCase (no `SolidSyslog` prefix), and the opaque-impl exception was documented. `.clang-tidy` gained a `StructIgnoredRegexp` with negative lookahead — accepts `SolidSyslog` exactly, accepts any PascalCase tag that does NOT start with `SolidSyslog`, keeps Tier 1 `SolidSyslog` tags subject to the StructPrefix rule. The 4 warnings vanished in this commit. | NAMING.md + `.clang-tidy` landed here | - -### Naming totals by verdict (post-S10.05 decisions) - -| Verdict | Count | What's covered | -|---------|------:|----------------| -| **Fix — landed in S10.07** — enum-tag rename + named enum-constant rename | 260 → 101 | SCREAMING_SNAKE enum constants (252 → 101 — anonymous-`enum { … };` idiom carried over for separate verdict) + `SolidSyslog_` enum tags (8 → 0) | -| **Fix — landed in S10.09** — data-member PascalCase rename | 186 → 0 | All lowerCamelCase struct data members (Config + impl + test-fake) cleared by per-cluster sweep | -| **Disable + landed in S10.05** — vtable function-pointer members | 128 → 0 | Tier 4 re-stated to PascalCase for all members; `MemberCase: CamelCase` | -| **Disable + landed in S10.05** — library-top-level functions | 12 → 0 | Tier 1 re-stated to two shapes (`SolidSyslog_` and `SolidSyslog_`); `GlobalFunctionIgnoredRegexp` accepts the second | -| **Disable + landed in S10.05** — file-scope Tier 2 struct tags | 4 → 0 | Tier 2 extended to cover struct tags (bare PascalCase, no prefix); `StructIgnoredRegexp` whitelists the 4 known sites | -| **Total post-S10.05 snapshot** | **446** | All 446 are Fix targets; no remaining Re-categorise items on the naming side | - -The single biggest unit of work is now the **enum-constant + data-member rename: 446 sites combined**, scheduled under S10.07 (existing — 260 sites) and its new S10.07-sibling (186 sites). - ---- - -## MISRA C:2012 — `cppcheck-misra` - -Each row carries the cppcheck addon's count. The verdict reflects how the rule should be handled across the codebase; per-site exemptions (where needed) live in `misra_suppressions.txt` and are populated by S10.06. - -| Rule | Count | What it asks | Top sites | Verdict | Rationale | Owner | -|------|------:|--------------|-----------|---------|-----------|-------| -| **5.9** (advisory) | 168 → 0 *(landed in S10.08)* | Identifiers with internal linkage shall be unique | `SolidSyslogWinsockTcpStream.c` (21), `SolidSyslogPosixTcpStream.c` (16), `SolidSyslogUdpSender.c` (12) | **Fix — landed in S10.08** | `static` helpers reusing common names (`Write`, `Read`, `Open`, etc.) across TUs were renamed `Class_Function` per NAMING.md Tier 2 using a strip-only-`SolidSyslog` rule (`SolidSyslogWinsockTcpStream.c` → `WinsockTcpStream_*` etc.). `Core/Source/SolidSyslog.c` got `SolidSyslog_` per the documented exception. Sweep was wide — applied to all ~811 statics across Core/Source + Platform/\*/Source, not just the 168 collisions; 5.9 cleared as a structural consequence. | **S10.08** | -| **11.3** | 95 | Cast between pointer to object types of different kinds | `SolidSyslogPosixFile.c` (10), `SolidSyslogWindowsFile.c` (10), `SolidSyslogTlsStream.c` (6) | **Deviate** | Vtable + caller-supplied-storage is the project's foundational OO-in-C mechanism: `(struct Impl*) storage` is unavoidable at every `_Create`. Replacing it would require dynamic allocation or copy-by-value, both rejected. Document as project-wide deviation. | **S10.06** (deviation D.002 in `docs/misra-deviations.md`) | -| **10.4** | 92 → 0 *(landed in S10.10)* | Both operands of operator shall be in same essential type category | `SolidSyslogFormatter.c` (15), `SolidSyslog.c` (6), `SolidSyslogFileBlockDevice.c` (6) | **Fix — landed in S10.10** | Mostly `size_t + 1` where `1` is `int`. Mechanical U-suffix sweep across `Core/Source/`, `Core/Interface/`, and `Platform/*/`. Two byte-comparison sites (BOM strip in `SolidSyslog.c`, UTF-8 lead range in `SolidSyslogFormatter.c`) refactored to `unsigned char` + hex-literal comparisons where the `(char)` cast did not satisfy cppcheck-misra's essential-type tracking. `SOLIDSYSLOG_FORMATTER_STORAGE_SIZE` picked up U on the literal inside the macro so every expansion is uniform. | **S10.10** | -| **8.9** (advisory) | 56 | Object referenced in one function should have block scope | `SolidSyslogFormatter.c` (8), `SolidSyslogMetaSd.c` (5), `SolidSyslogOriginSd.c` (5) | **Fix** | File-scope statics that should be locals or `static const` block-scope. Per-site review. | **S10.09** (if "abbreviation purge" widens to "code-shape hygiene") or **new sweep story** | -| **5.7** | 54 | Tag names unique | `BlockSequence.c` (2), `SolidSyslogFileBlockDevice.c` (2), `SolidSyslogBlockStore.h` (1), and so on across many forward-decl + definition pairs | **Deviate** | NAMING.md's no-typedef-struct convention deliberately repeats the same `struct SolidSyslogX` tag wherever it's used (forward-decl in headers, definition in source). 5.7 fires on every repetition. Document as deviation. | **S10.06** (deviation D.003) | -| **11.8** | 11 | Cast shall not remove const/volatile qualification | `SolidSyslog.c` (8), `BlockSequence.c` (1), `SolidSyslogBlockStore.c` (1), `SolidSyslogWinsockTcpStream.c` (1) | **Deviate** *(resolved in S10.06: all 11 sites)* | All 10 Core sites are field-access "false positives" — accessing a non-const-pointer field through a `const struct*` parameter; C standard §6.5.2.3 ¶3 says the member-access yields an unqualified pointer rvalue, so this is not a real const-strip. The Winsock `select()` site is the documented platform-API const-strip already commented in source. Captured as **D.006**. | landed in **S10.06** | -| **17.7** | 11 | Value returned by non-void function shall be used | `RecordStore.c` (5), `SolidSyslogCircularBuffer.c` (4), `SolidSyslog.c` (1) | **Fix** | Use the return value or cast to `(void)` with comment. | **New sweep story** or **S10.10** (Buffer pilot) where most concentrate | -| **10.1** | 11 → 0 *(landed in S10.10)* | Operands shall not be of inappropriate essential type | `SolidSyslogUtf8.h` (5), `SolidSyslogFormatter.c` (5), `SolidSyslogCrc16.c` (1) | **Fix — landed in S10.10** | Bitwise operators on plain `char` are inappropriate (char is essentially-character, bitwise operators require essentially-unsigned). Cast `char` operands to `unsigned char` and U-suffix the hex literal masks they are tested against. The Crc16 data-byte shift uses a literal `8U` instead of the `BITS_PER_BYTE` anonymous-enum constant (which cppcheck-misra still tracks as essentially-signed even with a U-suffixed initialiser). | **S10.10** | -| **15.7** | 10 → 0 *(landed in S10.10)* | All if/else-if shall be terminated with `else` | `BlockSequence.c` (3), `SolidSyslogFormatter.c` (2), `SolidSyslogTlsStream.c` (1) | **Fix — landed in S10.10** | Added trailing `else { /* explanatory comment */ }` to ten if/else-if chains spanning Core and Platform. Empty bodies document the residual case the existing branches do not cover. | **S10.10** | -| **11.2** | 10 | Conversions between pointer to incomplete type and other types | `SolidSyslogAddressInternal.h` (2 × 3 platforms) | **Deviate** | Same opaque-type pattern as 11.3 — `Address` is opaque. Document together. | **S10.06** (with D.002) | -| **8.4** | 8 | Compatible declaration shall be visible | `SolidSyslogStdAtomicU32.c` (4), `SolidSyslogWindowsAtomicU32.c` (4) | **Fix** | Atomics adapters define functions whose external prototype lives in a header that should be visible at definition. Add include or forward decl. | **S10.11** (security policies + CRC + sync primitives) | -| **12.1** (advisory) | 5 → 0 *(landed in S10.10; +3 sites surfaced after the 10.4 sweep were swept along; +2 surfaced after the STATIC_ASSERT polyfill switch were also cleared)* | Operator precedence should be explicit | `SolidSyslogCircularBuffer.c` (3), `SolidSyslogAtomicCounter.c` (1), `SolidSyslogTlsStream.c` (1) | **Fix — landed in S10.10** | Added explicit parens around sub-expressions mixing comparison with arithmetic or logical operators. One ternary in `SolidSyslogUdpPayload_FromMtu` refactored to an if/return — cppcheck-misra continued to flag the return-of-ternary form even after bracketing both arms. | **S10.10** | -| **17.8** | 5 | Function parameter should not be modified | `SolidSyslogFormatter.c` (3), `SolidSyslogError.c` (1), `SolidSyslogUdpPayload.c` (1) | **Fix** | Introduce a local copy. | **S10.17** for Formatter; mixed for the rest | -| **5.6** | 5 → 0 *(landed in S10.10)* | Typedef names unique | `SolidSyslogBlockStore.c` (1), `SolidSyslogCircularBuffer.c` (1), `SolidSyslogFileBlockDevice.c` (1) | **Fix — landed in S10.10** | Audit verdict had been "per-site review"; the actual cause was the project's `SOLIDSYSLOG_STATIC_ASSERT` polyfill emitting the same `typedef char solidsyslog_static_assert_[...]` in every TU. Replaced the negative-array polyfill with a C11 `_Static_assert` wrapper (the library compiles at `--std=c11`); the new form has no typedef and so cannot trigger 5.6. The unavoidable `#` stringify in the wrapper is captured by deviation **D.010** (Rule 20.10). | **S10.10** | -| **18.4** (advisory) | 4 | `+`, `-`, `+=`, `-=` shall not be applied to pointer types | `RecordStore.c` (4) | **Deviate** | RecordStore manipulates record buffers using pointer arithmetic by design (magic byte offset → length offset → message offset, etc.). This is the natural C expression for the algorithm. Document as deviation. | **S10.06** (D.004) | -| **15.5** (advisory) | 4 | Function should have single exit point | `SolidSyslogFormatter.c` (3), `SolidSyslog.c` (1) | **Fix** | Already a documented project preference ("Production code (Tier 1, Core/Source/) — Single return per function" in CLAUDE.md). Real drift. | **S10.17** (core message pipeline) | -| **11.5** | 4 | Conversion from pointer to void to pointer to object | `SolidSyslogWinsockTcpStream.c` (2), `SolidSyslogUdpSender.c` (1), `SolidSyslogWinsockDatagram.c` (1) | **Deviate** | Same opaque-type / vtable pattern as 11.3. | **S10.06** (with D.002) | -| **21.10** | 3 | `wchar.h` shall not be used | `SolidSyslogPosixClock.c` (1), `SolidSyslogPosixSleep.c` (1), `SolidSyslogPosixSysUpTime.c` (1) | **Deviate** *(resolved in S10.06)* | All three sites are `#include ` — glibc transitively pulls in `` via `bits/types/struct_tm.h`. No direct `` use anywhere in the project. Captured as **D.007**. | landed in **S10.06** | -| **22.10** | 3 | The value of `errno` shall be checked only after a function that may set it | `SolidSyslogPosixTcpStream.c` (2), `SolidSyslogPosixDatagram.c` (1) | **Fix** | We currently read `errno` in some places where the preceding function isn't documented to set it (or we check before re-calling). Per-site review. | **S10.15** (sender platform impls) | -| **8.6** | 3 | Identifier with external linkage shall have exactly one external definition | `SolidSyslogAddress.c` (1 each across Posix/Windows/FreeRtos) | **Fix** | Either missing definition or duplicate declaration of `Address` helpers — needs per-site investigation. | **S10.15** | -| **10.8** | 2 → 0 *(landed in S10.10)* | Composite-expression cast to wider essential type | `SolidSyslog.c` (2) | **Fix — landed in S10.10** | Hoisted the `(uint32_t)` cast to the top of `SolidSyslog_FormatNonZeroUtcOffset` so the hours/minutes split uses `uint32_t / 60U` and `% 60U` without casting a composite expression. | **S10.10** | -| **18.7** (advisory) | 2 | Flexible array members shall not be declared | `SolidSyslogCircularBuffer.c` (1), `SolidSyslogFormatter.c` (1) | **Deviate** | Flexible array members are the storage mechanism for `SolidSyslogFormatter` and `SolidSyslogCircularBuffer` — they hold caller-supplied storage of variable size. Removing them would require copying or fixed-size buffers. Document as deviation. | **S10.06** (D.005) | -| **1.4** (advisory) | 2 | Emergent language features shall not be used | `SolidSyslogStdAtomicU32.c` (1), `SolidSyslogWindowsAtomicU32.c` (1) | **Deviate** *(retired in S11.11)* | C11 `` is the project's atomic primitive on POSIX/clang/gcc/modern MSVC; we deliberately use C11 here and the alternative (`InterlockedCompareExchange`) is selected at link time on legacy MSVC. Use of C11 atomics is intentional, not accidental. | **S10.06** (D.006-original — retired in **S11.11**: cppcheck 2.10 no longer flags this site) | -| **2.5** (advisory) | 2 → 0 *(landed in S10.10)* | Unused macro | `SolidSyslogCircularBuffer.h` (2) | **Deviate — landed in S10.10** | Audit verdict had been "delete them" — that was wrong. The two `SOLIDSYSLOG_CIRCULAR_BUFFER_STORAGE_SIZE[_BYTES]` macros are part of the public API; their consumers live under `Tests/` (Consistency-only tier) and `Bdd/Targets/` (Out of scope), so cppcheck-misra cannot see the call sites. Captured as **D.011**. | **S10.10** | -| **3.1** | 1 → 0 *(landed in S10.10)* | `/* */` shall not contain comment-start sequence | `SolidSyslogCrc16.c` (1) | **Fix — landed in S10.10** | The Crc16 banner contained a `https://` URL — the `//` inside the `/* */` is a nested comment-start. Dropped the protocol prefix and rephrased the citation. | **S10.10** | -| **7.1** | 1 → 0 *(landed in S10.10)* | Octal constants shall not be used | `SolidSyslogPosixMessageQueueBuffer.c` (1) | **Fix — landed in S10.10** | `0600` (Posix file mode) replaced with a named hex enum `OWNER_READ_WRITE = 0x180U` in the file-scope anonymous-enum block. | **S10.10** | -| **21.6** | 1 | The Standard Library `stdio.h` shall not be used | `SolidSyslogWindowsFile.c` (1) | **Deviate** *(resolved in S10.06)* | `WindowsFile.c` includes `` solely for `SEEK_SET` / `SEEK_END` constants used by `_lseeki64` from ``; no stdio function or type is referenced. Captured as **D.008**. | landed in **S10.06** | -| **14.4** | 1 → 0 *(landed in S10.10)* | Controlling expression shall have essentially Boolean type | `SolidSyslogWindowsHostname.c` (1) | **Fix — landed in S10.10** | `GetComputerNameExA` returns `BOOL` (typedef `int`); the if now compares `!= FALSE` so the controlling expression is a real boolean result. | **S10.10** | -| **2.4** (advisory) | 1 → 6 *(resolved in S10.06)* | A project should not contain unused tag declarations | Re-run with D.002–D.008 suppressions surfaced 5 more anonymous-`enum { … };` sites previously hidden behind 5.7 — the audit's original count understated the rule. All 6 sites are the project's anonymous-`enum` named-constant idiom (≈31 such blocks tree-wide). | **Deviate** | Captured as **D.009**. The anonymous-`enum` idiom is project-wide and intentional (type-safe constants without `#define`). | landed in **S10.06** | - -### MISRA totals by verdict - -| Verdict | Count | Rules | -|---------|------:|-------| -| **Fix — landed in S10.08** — rule 5.9 (static-function `Class_` prefix sweep) | 168 → 0 | 5.9 | -| **Fix — landed in S10.10** — mechanical sweep (uniform U-suffixes, parens, trailing-else, …) + Rule 5.6 polyfill | 130 → 0 | 10.4 (92), 10.1 (11), 15.7 (10), 12.1 (5), 5.6 (5), 10.8 (2), 2.5 (2 — graduated to Deviate as D.011), 14.4 (1), 7.1 (1), 3.1 (1) | -| **Fix** — other rules (per-component sweeps S10.11+) | 90 | 8.9, 17.7, 8.4, 17.8, 15.5, 22.10, 8.6 | -| **Deviate** — landed in S10.06 (D.002–D.009; D.006-original / rule 1.4 retired in **S11.11**) | 187 | 11.3, 11.2, 11.5 *(D.002)*; 5.7 *(D.003)*; 18.4 *(D.004)*; 18.7 *(D.005)*; 1.4 *(retired S11.11)*; 11.8 *(D.006)*; 21.10 *(D.007)*; 21.6 *(D.008)*; 2.4 *(D.009)* | -| **Deviate** — landed in S10.10 (D.010, D.011) | 3 | 20.10 *(D.010, 1 site — `#` stringify in `_Static_assert` wrapper)*; 2.5 *(D.011, 2 sites — public API macros consumed under `Tests/` and `Bdd/Targets/`)* | -| **Disable** | 0 | — | -| **Total reconciled** | **388 Fix + 190 Deviate = 575** (rule 2.5 moved from Fix to Deviate per D.011; raw `Fix` reduced by 2, raw `Deviate` increased by 3 with the new D.010 = +1) | Raw count includes 5 dual-rule overlaps (same code site reported under two rules, e.g. 11.2 + 11.3 cast in `Address.c`). | - -The verdict surface evolved from the audit's headline: -- S10.06 added D.007 / D.008 (Investigate items) and D.009 (D.003 - suppression revealed 5 more 2.4 findings). -- S10.10 added D.010 (20.10 — stringify in the new `_Static_assert` - polyfill wrapper) and D.011 (2.5 — re-categorised from "delete - unused" when the audit's verdict was found to be wrong; the macros - are consumed outside the cppcheck-misra scope). - ---- - -## Cross-cutting patterns observed - -Recurring themes that recur across many rules — useful for **S10.06** when writing up the deviation document: - -1. **The opaque-impl + caller-supplied-storage pattern** drives rules 11.2, 11.3, 11.5 (109 findings combined). Single deviation document covers all three. The deviation note already lives in spirit in `docs/NAMING.md` (caller-supplied storage section). - -2. **The no-typedef-struct convention** drives rule 5.7 (54 findings). Single deviation. Already noted in `docs/NAMING.md` "No struct typedefs" section. - -3. **Pointer arithmetic on record buffers** drives rule 18.4 (4 findings, all in RecordStore). Documenting once covers the file. - -4. **Flexible array members** drive rule 18.7 (2 findings — Formatter and CircularBuffer). Two declared, one deviation. - -5. **C11 `` use** drives rule 1.4 (2 findings — Atomics adapters). One deviation. - -6. **`Class_Function` static helpers across TUs** drive rule 5.9 (168 findings) — these were the named S10.08 sweep target *and* an instance of the project's MISRA Tier 2 Class-prefix convention. **S10.08 landed wide** — applied the prefix to all ~811 statics across `Core/Source/` + `Platform/*/Source/` per the strip-only-`SolidSyslog` rule (and the `SolidSyslog_` exception for `Core/Source/SolidSyslog.c`), clearing 5.9 to **0**. No residual vtable-derived 5.9 findings; no follow-up deviation needed. - -These six concentrated buckets together account for **339 of the 575 findings (59%)** — the named S10.08 sweep on rule 5.9 (168), plus the five structural deviations (D.002 covers 11.3/11.2/11.5 = 109, D.003 = 54, D.004 = 4, D.005 = 2, D.006 = 2). The remainder — **236 findings spread across the other 23 rules** — is per-site cleanup, decomposed as: **221 Fix** target (mostly diffuse mechanical: U-suffix on literals, return-value use, trailing-else, explicit precedence parens), **11 Mixed** (rule 11.8 needs per-site split in S10.06), and **4 Investigate** (rules 21.10 / 21.6 likely transitive system-header includes). - ---- - -## Sweep volume estimate per S10.07+ story - -Rough order-of-magnitude for sweep planning. These are **upper bounds** (some Fix counts will be re-categorised down at the per-site review stage): - -| Story | Scope | Expected fix count | -|-------|-------|--------------------| -| S10.07 | Enum constants → `Class_PascalCase` + enum tags | 260 | -| S10.09 *(was the S10.07-sibling slot; named in S10.09 issue)* | Data members lowerCamelCase → PascalCase per Tier 4 re-statement — **landed** | 186 → 0 | -| S10.08 | Static functions → `Class_Function` (MISRA 5.9) | 168 | -| Abbreviation purge *(deferred — separate cross-cutting story, not yet raised)* | Locals/parameters and function-name abbreviations — different lens, scope still to be sized | TBD | -| **S10.10** *(was "Mechanical MISRA sweep" — picked up the next free story number)* | Tree-wide hybrid mechanical fixes — **landed**: 10.4 (92), 10.1 (11), 15.7 (10), 12.1 (5+drift), 5.6 (5), 10.8 (2), 2.5 (2, became D.011), 14.4 (1), 7.1 (1), 3.1 (1). +2 new deviations: D.010 (20.10), D.011 (2.5). | 130 → 0 | -| S10.11 *(was S10.10 in audit-era numbering)* | Buffers pilot — receives 8.9 + 17.7 + 17.8 fragments | small (~5–10) | -| S10.12 | SecurityPolicies + CRC + Sync — receives 8.4 + 2.5 fragments | ~10 | -| S10.13 | Config + platform helpers — receives 14.4 fragment | small (~5) | -| S10.14 | Structured data — receives 8.9 + 17.8 fragments | small (~5) | -| S10.15 | Stores + BlockDevice + File — receives 8.9 + 17.7 + 17.8 + 5.6 + 8.6 fragments | ~20 | -| S10.16 | Senders + transport extension points — receives 22.10 + 8.6 + 17.7 + 2.4 fragments | ~15 | -| S10.17 | Stream platform impls — receives 17.7 + 8.4 + 11.8 (decided in S10.06) fragments | small (~5) | -| S10.18 | Core message pipeline — receives 15.5 + 10.1 + 17.8 + 10.8 fragments | ~20 | - -The per-component story numbers above are advisory — actual numbers -are assigned when each story is raised. The audit-era numbering -(S10.10..S10.17) shifted by one because the mechanical sweep took -the S10.10 slot. - -### Mechanical MISRA sweep — hybrid split *(decided in S10.05, landed in S10.10)* - -The 221 Fix-target MISRA findings across 16 rules split into two -categories by whether the fix needs local code context: - -**Mechanical tree-wide sweep** *(landed in S10.10)* — truly uniform -fixes that benefit from being applied once across the whole tree -under a single review: - -| Rule | Audit count | After S10.10 | Fix shape | -|------|------------:|------------:|-----------| -| 10.4 | 92 | 0 | Add `U` suffix to integer literals; occasional cast | -| 12.1 | 5 | 0 | Add explicit precedence parentheses | -| 2.5 | 2 | 0 | Audit said "delete"; actually consumed outside cppcheck scope — became D.011 | -| 15.7 | 10 | 0 | Add trailing `else { /* exhaustive */ }` | -| 10.8 | 2 | 0 | Hoist cast above composite expression | -| 10.1 | 11 | 0 | Bool/char essential-type mismatch — cast to `unsigned char` + hex literals | -| 5.6 | 5 | 0 | Switch `SOLIDSYSLOG_STATIC_ASSERT` polyfill to C11 `_Static_assert` | -| 2.4 | 1 → graduated to Deviate in S10.06 (D.009) before S10.10 ran | — | — | -| 3.1 | 1 | 0 | Disambiguate `//` inside a `/* */` URL | -| 7.1 | 1 | 0 | Replace `0600` octal with named hex `OWNER_READ_WRITE = 0x180U` | -| 14.4 | 1 | 0 | `BOOL` comparison with `!= FALSE` instead of bare value | -| **S10.10 total** | **131** *(126 mechanical + 5 polyfill)* | **0** | | -| **New deviations introduced as byproducts** | — | — | D.010 (20.10, 1 site — stringify); D.011 (2.5, 2 sites — public API macros) | - -**Per-component sweep targets** *(absorbed into S10.10–S10.17)* — -fixes that need local file context to apply correctly: - -| Rule | Count | Fix shape | -|------|------:|-----------| -| 8.9 | 56 | Move file-scope statics to block scope — needs context | -| 17.7 | 11 | Use return value — depends on what the caller wants | -| 17.8 | 5 | Don't modify parameter — refactor needs context | -| 15.5 | 4 | Single exit point — refactor function shape | -| 5.6 | 5 | Typedef name collision — per-site review | -| 8.4 | 8 | Compatible declaration visible — needs header or include change | -| 22.10 | 3 | `errno` check after function-that-may-set-it — per-site review | -| 8.6 | 3 | Identifier with external linkage one definition — per-site review | -| **Per-component total** | **95** | | - -The 221 = 126 + 95 reconciles. The 168 findings of rule 5.9 stay -in the named S10.08 sweep (Tier 2 `Class_` prefix rename), as -already scheduled. The 171 Deviate findings still land in S10.06 -via D.002–D.006 in `docs/misra-deviations.md`. The 11 Mixed (rule -11.8) and 4 Investigate (rules 21.10 / 21.6) findings are still -held for per-site review in S10.06. - -#### Story naming for the mechanical sweep - -Decided in S10.05: **decide-in-principle, slot in S10.06**. S10.05's -audit records the work as "a new mechanical sweep story exists, -carries the 126 fixes listed above, hybrid scope per Q4 decision." -S10.06 picks the actual story number, writes the issue, and slots -it relative to the other E10 audit/decide work. diff --git a/docs/misra-deviations.md b/docs/misra-deviations.md index 00678773..23514451 100644 --- a/docs/misra-deviations.md +++ b/docs/misra-deviations.md @@ -861,3 +861,71 @@ Summary: Project owner — David Cozens. Recorded under [S10.18](https://github.com/DavidCozens/solid-syslog/issues/430). +--- + +## D.013 — Rule 11.5: `void*` ↔ `unsigned char*` at third-party byte-buffer API boundaries + +### Rule + +> **Rule 11.5 (Advisory)** — A conversion should not be performed from +> pointer to `void` into pointer to object. + +### Deviation + +`SolidSyslogStream::Send` takes `const void*` and `SolidSyslogStream::Read` +takes `void*` — the project-wide byte-buffer contract used by every +Stream implementation. Some third-party C libraries (notably mbedTLS) +type their byte buffers as `const unsigned char*` / `unsigned char*` +rather than `void*`. The implementation cast bridging the two is +unavoidable at the API boundary: + +```c +int rc = mbedtls_ssl_write(&self->SslContext, (const unsigned char*) buffer, size); +``` + +Rule 11.5 fires on each such adapter cast. + +### Scope + +`Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c` — two sites +(`MbedTlsStream_Send`, `MbedTlsStream_Read`). + +The deviation extends to any future Stream / Datagram / hash / MAC +implementation that wraps a byte-typed third-party C API +(`unsigned char*` rather than `void*`). The OpenSSL adapter +(`Platform/OpenSsl/Source/SolidSyslogTlsStream.c`) does **not** fall +under this deviation — `SSL_write` / `SSL_read` take `void*` and so no +cast is needed. + +### Rationale + +The alternatives all regress: + +| Alternative | Why rejected | +|-------------|--------------| +| Refactor `SolidSyslogStream::Send`/`Read` to use `unsigned char*` | Public-API ABI change that propagates to every Stream implementation (Posix TCP, Winsock TCP, FreeRTOS TCP, OpenSSL TLS, mbedTLS TLS, NullStream) and every Stream caller (`SolidSyslogStreamSender`). The `void*` byte-buffer contract is the conventional C idiom for transport interfaces and matches POSIX `send`/`recv`, OpenSSL `SSL_write`/`SSL_read`, etc. Changing it for the sake of one third-party API's typing choice is the wrong direction. | +| Copy through an `unsigned char` scratch buffer per call | Runtime cost on the hot send/receive path; adds a fixed-size scratch or a stack-allocated VLA in a critical-path function. Defeats the zero-copy intent of the Stream contract. | +| Inline `cppcheck-suppress misra-c2012-11.5` at each site | Inline suppressions are weaker by MISRA Compliance:2020 §4.2 (rationale scattered, not centrally auditable). Project preference is structural deviations in this document. | + +The cast is well-defined: `unsigned char` may alias any object type +(C99 §6.5 ¶7), so reinterpreting a `void*` byte buffer as +`unsigned char*` and back is a no-op at the abstract-machine level. + +### Risk and mitigation + +- **Alignment** — Both representations are byte-addressed; no + alignment promotion occurs. The cast targets `unsigned char*`, which + has the weakest alignment requirement of any object pointer. +- **Type safety** — The caller-supplied buffer originates as a + contiguous byte sequence (typically the formatted syslog record); + treating it as `unsigned char*` at the third-party API boundary is + the same byte sequence under a different pointer type. +- **Elimination path** — A future revision of the Stream API that + adopts `unsigned char*` directly would retire this deviation. + Tracked as a possible E10-successor refactor, not scheduled. + +### Approval + +Project owner — David Cozens. Recorded under +[S10.20](https://github.com/DavidCozens/solid-syslog/issues/437). + diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 1fdfa820..9db2e8df 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -44,6 +44,7 @@ misra-c2012-11.3:Platform/FreeRtos/Source/SolidSyslogFreeRtosDatagram.c:47 misra-c2012-11.3:Platform/FreeRtos/Source/SolidSyslogFreeRtosMutex.c:40 misra-c2012-11.3:Platform/FreeRtos/Source/SolidSyslogFreeRtosResolver.c:44 misra-c2012-11.3:Platform/FreeRtos/Source/SolidSyslogFreeRtosTcpStream.c:85 +misra-c2012-11.3:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:60 misra-c2012-11.3:Platform/OpenSsl/Source/SolidSyslogTlsStream.c:70 misra-c2012-11.3:Platform/Posix/Source/SolidSyslogPosixAddress.c:10 misra-c2012-11.3:Platform/Posix/Source/SolidSyslogPosixAddressPrivate.h:20 @@ -66,6 +67,8 @@ misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWindowsMutex.c:34 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:94 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:160 misra-c2012-11.5:Core/Source/SolidSyslogUdpSender.c:205 +misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:190 +misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:202 misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:138 misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:322 misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:342 @@ -115,6 +118,7 @@ misra-c2012-5.7:Core/Source/SolidSyslogStreamSender.c:19 misra-c2012-5.7:Core/Source/SolidSyslogUdpPayload.c:5 misra-c2012-5.7:Platform/FreeRtos/Source/SolidSyslogFreeRtosResolver.c:21 misra-c2012-5.7:Platform/FreeRtos/Source/SolidSyslogFreeRtosTcpStream.c:27 +misra-c2012-5.7:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:14 misra-c2012-5.7:Platform/OpenSsl/Source/SolidSyslogTlsStream.c:16 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogGetAddrInfoResolver.c:20 misra-c2012-5.7:Platform/Posix/Source/SolidSyslogPosixDatagram.c:21 @@ -162,3 +166,8 @@ misra-c2012-2.5:Core/Interface/SolidSyslogCircularBuffer.h:22 # 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 + +# D.013 — Rule 11.5: void* ↔ unsigned char* at third-party byte-buffer API boundaries +# See docs/misra-deviations.md#d013 +misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:227 +misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:246