Skip to content

ci: S10.20 flip MISRA + naming gates to error mode and close E10#438

Merged
DavidCozens merged 1 commit into
mainfrom
ci/s10-20-error-mode
May 23, 2026
Merged

ci: S10.20 flip MISRA + naming gates to error mode and close E10#438
DavidCozens merged 1 commit into
mainfrom
ci/s10-20-error-mode

Conversation

@DavidCozens

@DavidCozens DavidCozens commented May 23, 2026

Copy link
Copy Markdown
Owner

Purpose

Closes the E10 enforcement-mode flip and tidies the audit-phase artefacts
that were always slated for removal at the close-out. With S10.12–S10.19
done and main carrying 0 cppcheck-misra findings and 0 clang-tidy
naming warnings, the gates can flip.

This is the last story under E10 — merging closes both the story (#437)
and the epic (#12).

Change Description

CI gate flips

  • analyze-cppcheck job — cppcheck-misra --error-exitcode=01 on
    both the "Run cppcheck-misra" and "Generate cppcheck-misra XML report"
    steps. The soft-freeze comment block (referenced the old "S10.18 will
    flip" target) replaced with a one-paragraph current-state note.
  • Root .clang-tidyWarningsAsErrors: '*,-readability-identifier-naming'
    '*'. Stale paragraph pruned to a single-line note.

Scope additions

  • Platform/MbedTls/Source/ added to both cppcheck-misra invocations
    (was absent — adapter landed in S08.07 against local-only checks).
  • Non-MISRA cppcheck XML pass scope widened from Core/Source/ only to
    Core/Source/ + Platform/*/Source/ (matches cppcheck-misra scope).
    Build-time cppcheck via CMAKE_C_CPPCHECK already gated Platform/;
    this widens the diagnostic artefact upload to match.
  • analyze-format find roots widened to include Platform/.

MbedTls verification → small cleanup

The MbedTls inclusion surfaced 7 findings (the "verification step only"
framing in the issue body was wrong). All in SolidSyslogMbedTlsStream.c,
all reviewed per-site per the S10.12 conformance recipe:

Site Rule Disposition
line 14 — anonymous enum block 5.7 D.009 extension
line 60 — SelfFromBase downcast 11.3 D.002 extension
lines 190, 202 — BIO callback void* ctx casts 11.5 D.002 extension
line 211 — if/else if chain missing trailing else 15.7 Real per-site fix — added explicit else { /* n<0 → keep -1 */ }. No behaviour change.
lines 227, 246 — (unsigned char*) buffer at mbedTLS byte-buffer APIs 11.5 New deviation D.013

New deviation D.013

void* ↔ unsigned char* at third-party byte-buffer API boundaries.
mbedTLS types its byte buffers as unsigned char*; the project's
SolidSyslogStream::Send/Read use the conventional void* byte-buffer
contract. OpenSSL (SSL_write/SSL_read) takes void* so this never
came up for TlsStream.c. Refactoring the Stream API to unsigned char*
would propagate to every Stream impl + StreamSender — wrong direction
for one third-party API's typing choice. Cast is well-defined under
C99 §6.5 ¶7 (unsigned char may alias any object type). Full rationale
in docs/misra-deviations.md#d013.

Audit doc deletion

docs/misra-conformance.md removed (frozen since S10.12, slated for
deletion at the gate flip). Live truth lives in docs/misra-deviations.md
(rationales) and misra_suppressions.txt (per-site state). DEVLOG
history retains references — point-in-time records, not rewriting.

Mop-up

Tests/DatagramFake.h:18-21 comment from the S10.22 deferred list
reworded to disambiguate the per-slot default vs the
out-of-DATAGRAMFAKE_MAX_SEND_CALLS fallback.

Test Evidence

Local verification in the gcc dev container:

  • cppcheck --addon=misra --error-exitcode=1 over full Core +
    Platform/{Atomics,Posix,Windows,OpenSsl,MbedTls,FreeRtos,FatFs}/Source/
    with new suppressions → EXIT=0.
  • cmake --build --preset tidy (which exercises
    readability-identifier-naming under the new WarningsAsErrors: '*')
    EXIT=0, no warnings.
  • clang-format --dry-run --Werror over the widened
    Core/Interface Core/Source Platform Tests Bdd/Targets tree →
    EXIT=0.

Re-proving that the gates fail on a deliberate violation was considered
and skipped — they fired dozens of times during S10.07–S10.19, so
re-proving in this PR adds churn without information.

Areas Affected

  • CI.github/workflows/ci.yml: analyze-cppcheck,
    analyze-format.
  • Root config.clang-tidy: WarningsAsErrors.
  • MISRA artefactsmisra_suppressions.txt (+9 lines),
    docs/misra-deviations.md (+D.013 section), docs/misra-conformance.md
    (deleted).
  • MbedTls adapterPlatform/MbedTls/Source/SolidSyslogMbedTlsStream.c:
    trailing else { } in MbedTlsStream_BioRecv, no behaviour change.
  • Test fakesTests/DatagramFake.h: comment-only.
  • DEVLOGDEVLOG.md: 2026-05-23 S10.20 entry.

No production behaviour change in any shipped artefact.

Closes #437
Closes #12

Summary by CodeRabbit

  • Bug Fixes

    • Fixed non-blocking transport error handling in MbedTLS stream operations to ensure proper failure signaling.
  • Chores

    • Strengthened code quality enforcement: stricter clang-tidy warnings configuration and expanded MISRA/cppcheck coverage.
    • Updated CI workflows for comprehensive quality analysis and error detection.
  • Documentation

    • Added MISRA C:2012 deviation documentation for third-party API compatibility.
    • Reorganized conformance tracking and developer logs.

Review Change Stack

- cppcheck-misra: --error-exitcode=0 → 1 on both Run and Generate-XML steps.
- Add Platform/MbedTls/Source/ to cppcheck-misra invocation.
- Widen non-MISRA cppcheck XML pass scope to Core/ + Platform/*/Source/.
- Widen analyze-format scope to include Platform/.
- clang-tidy: WarningsAsErrors '*,-readability-identifier-naming' → '*'.
- Delete docs/misra-conformance.md (audit-phase doc, live truth lives in
  docs/misra-deviations.md + misra_suppressions.txt).
- Tests/DatagramFake.h: reword per-slot vs out-of-range comment.
- MbedTls verification surfaced 7 findings: 4 existing-deviation
  extensions (D.002, D.009), 1 trailing-else fix (rule 15.7), 2 sites
  under new D.013 (void* ↔ unsigned char* at third-party byte-buffer
  API boundaries — mbedtls_ssl_write / mbedtls_ssl_read take
  unsigned char* whereas SolidSyslogStream::Send/Read use void*).

Closes #437 and #12.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e06fe691-1e86-4d6f-86cc-e58ecf083e3c

📥 Commits

Reviewing files that changed from the base of the PR and between a727353 and 2970a9a.

📒 Files selected for processing (8)
  • .clang-tidy
  • .github/workflows/ci.yml
  • DEVLOG.md
  • Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c
  • Tests/DatagramFake.h
  • docs/misra-conformance.md
  • docs/misra-deviations.md
  • misra_suppressions.txt
💤 Files with no reviewable changes (1)
  • docs/misra-conformance.md

📝 Walkthrough

Walkthrough

This PR closes epic E10 by flipping static-analysis gates from warning to error mode and verifying Platform/MbedTls is conformant. It widens CI scope to include the MbedTls adapter, applies a transport-error handling fix, documents a new MISRA deviation for byte-buffer API casts, and removes the frozen conformance audit document.

Changes

S10.20 MISRA and Naming Enforcement Close-out

Layer / File(s) Summary
CI gate flips and scope widening
.clang-tidy, .github/workflows/ci.yml
Flip clang-tidy WarningsAsErrors to enforce naming on all checks; flip cppcheck-misra from warning to error-exit mode; expand both MISRA and non-MISRA cppcheck scopes to Platform/MbedTls; widen clang-format to include Platform; delete frozen docs/misra-conformance.md.
MbedTls code fix and D.013 deviation
Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c, docs/misra-deviations.md, misra_suppressions.txt
Add explicit else branch to MbedTlsStream_BioRecv for transport-level error handling; document new MISRA C:2012 Rule 11.5 deviation D.013 for void*unsigned char* casting at third-party byte-buffer API boundaries; register suppressions for the affected MbedTls adapter call sites.
DEVLOG entry and supporting clarifications
DEVLOG.md, Tests/DatagramFake.h
Document S10.20 close-out in DEVLOG including gate flips, scope widening, MbedTls verification findings, and D.013 introduction; clarify DatagramFake.h SendTo result semantics documentation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • DavidCozens/solid-syslog#358: Foundational PR introducing initial MISRA deviations framework and docs/misra-deviations.md; this PR adds deviation D.013 and documents the final enforcement close-out.
  • DavidCozens/solid-syslog#360: Modifies clang-tidy readability-identifier-naming warning configuration; this PR flips the same check into error-gating mode to complete enforcement.
  • DavidCozens/solid-syslog#382: Updates .clang-tidy enum-constant naming rules; this PR completes the naming enforcement by removing the soft-freeze exception for identifier-naming checks.

Poem

🐰 The gates flip shut with a satisfying click—
MISRA errors now fail the build, quick!
Platform is verified, deviations declared,
E10's enforcement is finally prepared.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: flipping MISRA and naming gates to error mode and closing the E10 epic/story, which aligns with the PR's primary objectives.
Description check ✅ Passed The description is comprehensive and well-structured, covering purpose, detailed change description, test evidence, and areas affected. All required template sections are present and substantive.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from issue #437 (#437.1#437.7) and meets the E10 epic's enforcement close-out objectives: gates flipped to error mode, Platform/MbedTls scope added, format scope widened, deviations documented, audit doc deleted.
Out of Scope Changes check ✅ Passed All file changes are directly scoped to issue #437 requirements: CI gate flips, scope additions, MISRA deviations documentation, suppressions, audit doc deletion, and a minor code fix with no behavior change in MbedTlsStream.c.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/s10-20-error-mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1300 passed, 🙈 2 skipped)
   🚦   build-freertos-host-tdd: 100% successful (✔️ 1524 passed, 🙈 2 skipped)
   🚦   build-linux-clang: 100% successful (✔️ 1252 passed, 🙈 2 skipped)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1252 passed, 🙈 2 skipped)
   🚦   integration-linux-openssl: 100% successful (✔️ 9 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 7 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 9 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 46 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 90% successful (✔️ 44 passed, 🙈 5 skipped)
   🚦   bdd-freertos-qemu: 86% successful (✔️ 42 passed, 🙈 7 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1136 passed, 🙈 1 skipped)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1252 passed, 🙈 2 skipped)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: 1 warning (low: 1)


Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

@DavidCozens DavidCozens merged commit e254400 into main May 23, 2026
21 checks passed
@DavidCozens DavidCozens deleted the ci/s10-20-error-mode branch May 23, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

S10.20: Flip MISRA + naming gates to error mode and close E10 E10: Static Analysis and MISRA

1 participant