refactor: S10.10 mechanical MISRA sweep (10 rules, ~130 sites)#376
Merged
Conversation
Append __LINE__ via two-step concat so each typedef expansion produces a unique name within a translation unit, satisfying MISRA C:2012 Rule 5.6 (typedef name shall be a unique identifier). cppcheck-misra: rule 5.6 findings 5 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add U suffix to integer literals (and a handful of anonymous-enum initializers and named tunable constants) so each operator's operands share the same MISRA essential-type category. Two byte-comparison sites in SolidSyslog.c (BOM strip) and SolidSyslogFormatter.c (UTF-8 lead range) refactored to use `unsigned char` comparisons with hex literals where the (char) cast did not satisfy cppcheck-misra's essential-type tracking. SOLIDSYSLOG_FORMATTER_STORAGE_SIZE picks up U on the literal inside the macro so every expansion is uniform. cppcheck-misra: rule 10.4 findings 92 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rations Bitwise operators (& == << ^=) on plain char are inappropriate per MISRA Rule 10.1 — chars are essentially-character, the bitwise operators require essentially-unsigned operands. Cast char operands to unsigned char and U-suffix the hex literal masks they are tested against. Affected sites: - Core/Source/SolidSyslogUtf8.h: five top-bit classifiers (IsAsciiByte, IsContinuationByte, IsTwoByteLead, IsThreeByteLead, IsFourByteLead). - Core/Source/SolidSyslogFormatter.c: IsOverlongTwoByteLead, IsOverlongThreeByteEncoding, IsUtf16SurrogateEncoding, IsOverlongFourByteEncoding, IsAboveUnicodeMaxEncoding. - Core/Source/SolidSyslogCrc16.c: the data-byte shift inside SolidSyslogCrc16_Compute now uses a literal 8U (the BITS_PER_BYTE anonymous-enum constant remained essentially-signed for cppcheck-misra even after U-suffixing the initializer); the unrelated enum initializers picked up matching U suffixes for visual consistency inside the same block. cppcheck-misra: rule 10.1 findings 11 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…SRA 15.7) Add empty else clauses with explanatory comments to ten if/else-if chains spanning Core/Source/ and Platform/*/Source/. The else bodies hold no code — they document the residual case the existing branches do not cover, which is what Rule 15.7 asks for. cppcheck-misra: rule 15.7 findings 10 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add parentheses around sub-expressions mixing comparison/equality with arithmetic or logical operators to make precedence explicit per MISRA Rule 12.1 (advisory). One ternary in SolidSyslogUdpPayload_FromMtu refactored to a small if/return — cppcheck-misra continued to flag the return-of-ternary form even after bracketing both arms. cppcheck-misra: rule 12.1 findings 5 → 0 (audit baseline; +3 newly surfaced sites that fell out of the 10.4 sweep were swept along). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…10.8) Cast absoluteMinutes to uint32_t once at the top of SolidSyslog_FormatNonZeroUtcOffset and use uint32_t arithmetic for the hours/minutes split. Previous form cast each composite (absoluteMinutes / 60) and (absoluteMinutes % 60) — a different essential-type category on a composite expression, which violates MISRA Rule 10.8. cppcheck-misra: rule 10.8 findings 2 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…acros
The two SOLIDSYSLOG_CIRCULARBUFFER_STORAGE_SIZE[_BYTES] macros are part
of the public API: integrator code under Tests/ and Bdd/Targets/ calls
them to size caller-supplied storage. cppcheck-misra runs only over
Core/Source/ + Platform/*/Source/, so from its scope the macros look
unused. Annotate with an inline `cppcheck-suppress misra-c2012-2.5`
naming the consumers, so the rule keeps catching genuinely-unused
macros elsewhere.
Also U-suffix a literal inside SOLIDSYSLOG_CIRCULARBUFFER_STORAGE_SIZE_BYTES
to match the Rule 10.4 sweep style — the macro is not currently expanded
inside any cppcheck-scanned TU, so this is preemptive consistency.
The audit's original verdict ("unused — delete them") was wrong: the
macros are used, just outside cppcheck's scope.
cppcheck-misra: rule 2.5 findings 2 → 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(MISRA 14.4) GetComputerNameExA returns BOOL (typedef int in Win32). The controlling expression of an if shall have essentially boolean type (MISRA 14.4); a bare BOOL value does not. Compare against FALSE so the != yields a real boolean result. cppcheck-misra: rule 14.4 findings 1 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…MISRA 7.1) MISRA 7.1 prohibits octal literals — Posix permission bits are traditionally written in octal (0600 = owner read/write), but the project has agreed to avoid octal in production code. Replace with a named hex constant OWNER_READ_WRITE = 0x180U inside a local anonymous enum so the literal is named at the call site and the value documents what each bit represents. cppcheck-misra: rule 7.1 findings 1 → 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (MISRA 3.1) The CRC-16 banner comment contained a https:// URL — the // inside a /* */ block is a nested comment-start sequence, which MISRA 3.1 prohibits. Drop the protocol prefix and rephrase the citation; the host + path are still navigable. cppcheck-misra: rule 3.1 findings 1 → 0. All ten S10.10-scope rules now report zero findings: 5.6 (5→0), 10.4 (92→0), 10.1 (11→0), 15.7 (10→0), 12.1 (5→0+drift→0), 10.8 (2→0), 2.5 (2→0), 14.4 (1→0), 7.1 (1→0), 3.1 (1→0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d D.011/D.012 Two related changes for cleaner MISRA conformance: 1. **STATIC_ASSERT polyfill (Rule 5.6 follow-up).** The negative-array- size form added earlier in this story used a __LINE__-concatenated typedef name to satisfy Rule 5.6 (typedef uniqueness). cppcheck-misra then surfaced 5 new Rule 2.3 findings (typedef declared but never used — by design for the assertion mechanism) and the ## operator itself was a Rule 20.10 violation. Net regression vs. baseline. Switch to a direct C11 _Static_assert wrapper that stringifies the msg argument. The library already compiles at --std=c11; this avoids the typedef entirely. New deviation D.011 covers the unavoidable # operator in the stringify wrapper. 2. **CircularBuffer 2.5 suppression (Rule 2.5 follow-up).** Move the inline cppcheck-suppress on the public API macros to a txt-file suppression paired with D.012, matching the project's structural deviation pattern. Inline rationale removed. Also incidental: - `SOLIDSYSLOG_STATIC_ASSERT` call sites in `SolidSyslogFormatter.c` and `SolidSyslogCircularBuffer.c` gain explicit parens around the `*` sub-expression — cppcheck-misra now analyses inside the `_Static_assert` condition and surfaces Rule 12.1. - `OWNER_READ_WRITE` constant in `SolidSyslogPosixMessageQueueBuffer.c` moved from a function-local anonymous enum to the file-scope one, silencing a Rule 5.7 finding on the second anonymous enum block. - `Formatter_IsAboveUnicodeMaxEncoding` gains a NOLINT for bugprone-easily-swappable-parameters — the Rule 10.1 refactor shifted the function body so that `lead` and `continuation1` are each used exactly once, tripping clang-tidy's heuristic. cppcheck-misra: rules 5.6, 20.10, 2.3, 2.5, 5.7, 12.1 all 0 → unchanged or → 0; no regressions in any sweep target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mark the ten sweep-target rule rows in misra-conformance.md as landed in S10.10 with the count → 0 annotation matching the S10.07/S10.08/S10.09 style. Update the MISRA totals table to add the S10.10 row, move rule 2.5 from Fix to Deviate (D.012), and add the D.011 entry. Renumber the per-component sweep target table from S10.10..S10.17 to S10.11..S10.18 to reflect the actual numbering after S10.10 took the slot. Mark the Mechanical MISRA sweep table as landed with audit / post-S10.10 columns plus the two new deviations introduced as byproducts. DEVLOG entry records: - Story selection rationale and slicing approach - Three audit verdict re-categorisations (rule 2.5, 5.6, 12.1) - Quirks encountered (anonymous-enum essential-type tracking, (char) cast vs unsigned char, return-of-ternary, clang-tidy swappable-parameters sensitivity to body shape, clang-format re-flow on suppression comments) - The auditor-view discussion on txt-file vs inline suppressions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis pull request implements the S10.10 mechanical MISRA sweep: 125+ uniform fixes across compile-time constants, runtime arithmetic, UTF-8/formatter safety, control flow, and static assertions. Changes add explicit ChangesUnsigned Literal and Type Safety Fixes
Control Flow Clarification: Explicit Else Branches
Static Assertion Modernization
Documentation and Deviation Records
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1126 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/misra-deviations.md (1)
819-825: ⚡ Quick winAdd language identifier to fenced code block.
The code fence displaying grep results should specify a language identifier (e.g.,
textorshell) to satisfy markdownlint MD040.📝 Proposed fix
-``` +```text Tests/SolidSyslogCircularBufferTest.cpp — both macros Tests/SolidSyslogTest.cpp — _BYTES form Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp — _SIZE form Bdd/Targets/Windows/BddTargetWindows.c — _SIZE form Bdd/Targets/FreeRtos/main.c — _SIZE form</details> As per coding guidelines, markdownlint flagged this as MD040 (fenced-code-language). <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/misra-deviations.mdaround lines 819 - 825, The fenced code block in
docs/misra-deviations.md containing the grep results lacks a language identifier
and triggers markdownlint MD040; update that block by adding a language tag (for
exampletext) immediately after the opening triple backticks so the block
becomes ```text and retains the same contents (the lines listing
Tests/SolidSyslogCircularBufferTest.cpp, Tests/SolidSyslogTest.cpp,
Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp,
Bdd/Targets/Windows/BddTargetWindows.c, Bdd/Targets/FreeRtos/main.c) to satisfy
the linter.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Nitpick comments:
In@docs/misra-deviations.md:
- Around line 819-825: The fenced code block in docs/misra-deviations.md
containing the grep results lacks a language identifier and triggers
markdownlint MD040; update that block by adding a language tag (for example
text) immediately after the opening triple backticks so the block becomesTests/SolidSyslogCircularBufferTest.cpp, Tests/SolidSyslogTest.cpp, Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp, Bdd/Targets/Windows/BddTargetWindows.c, Bdd/Targets/FreeRtos/main.c) to satisfy the linter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID:
d119fc2b-79fd-478d-aba5-d013c466690f📒 Files selected for processing (48)
Core/Interface/SolidSyslogBlockStore.hCore/Interface/SolidSyslogCircularBuffer.hCore/Interface/SolidSyslogFileBlockDevice.hCore/Interface/SolidSyslogFormatter.hCore/Interface/SolidSyslogSecurityPolicyDefinition.hCore/Interface/SolidSyslogStreamSender.hCore/Interface/SolidSyslogTunablesDefaults.hCore/Source/BlockSequence.cCore/Source/RecordStore.hCore/Source/SolidSyslog.cCore/Source/SolidSyslogAtomicCounter.cCore/Source/SolidSyslogCircularBuffer.cCore/Source/SolidSyslogCrc16.cCore/Source/SolidSyslogFileBlockDevice.cCore/Source/SolidSyslogFormatter.cCore/Source/SolidSyslogMacros.hCore/Source/SolidSyslogMetaSd.cCore/Source/SolidSyslogOriginSd.cCore/Source/SolidSyslogTimeQualitySd.cCore/Source/SolidSyslogUdpPayload.cCore/Source/SolidSyslogUdpSender.cCore/Source/SolidSyslogUtf8.hDEVLOG.mdPlatform/FatFs/Interface/SolidSyslogFatFsFile.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosDatagram.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosMutex.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosStaticResolver.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosTcpStream.hPlatform/OpenSsl/Interface/SolidSyslogTlsStream.hPlatform/OpenSsl/Source/SolidSyslogTlsStream.cPlatform/Posix/Interface/SolidSyslogPosixFile.hPlatform/Posix/Interface/SolidSyslogPosixMutex.hPlatform/Posix/Interface/SolidSyslogPosixTcpStream.hPlatform/Posix/Source/SolidSyslogPosixDatagram.cPlatform/Posix/Source/SolidSyslogPosixFile.cPlatform/Posix/Source/SolidSyslogPosixHostname.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.cPlatform/Posix/Source/SolidSyslogPosixTcpStream.cPlatform/Windows/Interface/SolidSyslogWindowsFile.hPlatform/Windows/Interface/SolidSyslogWindowsMutex.hPlatform/Windows/Interface/SolidSyslogWinsockTcpStream.hPlatform/Windows/Source/SolidSyslogWindowsFile.cPlatform/Windows/Source/SolidSyslogWindowsHostname.cPlatform/Windows/Source/SolidSyslogWinsockDatagram.cPlatform/Windows/Source/SolidSyslogWinsockTcpStream.cdocs/misra-conformance.mddocs/misra-deviations.mdmisra_suppressions.txt
This was referenced May 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Closes #375. Lands the cross-cutting mechanical MISRA sweep specified in
docs/misra-conformance.md("Mechanical MISRA sweep — hybrid split"), plus the Rule 5.6 polyfill fix carried forward from S10.06. Finishes the cross-cutting batch begun by S10.07/S10.08/S10.09 before the per-component conformance sweeps begin.Change Description
Per-rule landing, one commit per rule, in rule-size-descending order:
SOLIDSYSLOG_STATIC_ASSERTpolyfill to C11_Static_assert(the typedef-trick form duplicated identifiersolidsyslog_static_assert_across every TU)Core/+Platform/*/— literals at flagged sites,SOLIDSYSLOG_MAX_MESSAGE_SIZE/SOLIDSYSLOG_MAX_INTEGRITY_SIZE/SOLIDSYSLOG_FORMATTER_OVERHEADand theSTORAGE_SIZEmacros. Two byte-comparison sites (BOM strip, UTF-8 lead range) refactored tounsigned char+ hex literalschartounsigned charfor bitwise operators inSolidSyslogUtf8.h(5 sites),SolidSyslogFormatter.c(5 sites), and inlined theBITS_PER_BYTEenum constant as a literal8UinSolidSyslogCrc16.celse { /* explanatory */ }on ten if/else-if chains across Core and Platform(uint32_t)cast above the composite expression inSolidSyslog_FormatNonZeroUtcOffsetTests/andBdd/Targets/. Captured as D.012 inmisra_suppressions.txt+docs/misra-deviations.mdBOOLfromGetComputerNameExAcompared with!= FALSE0600octal → named hex constantOWNER_READ_WRITE = 0x180UinSolidSyslogPosixMessageQueueBuffer.chttps://prefix from a URL in theSolidSyslogCrc16.cbanner — the//inside/* */was a nested comment-startNew deviations introduced as byproducts of the sweep:
#operator in the_Static_assertwrapper is unavoidable for stringifying the msg argument; documented per MISRA Compliance:2020 §4.2 deviation-record format.Both are line-specific in
misra_suppressions.txtso genuinely unused macros and stray#/##use elsewhere still surface as fresh findings.Auditor view of suppressions placement discussed during the story: structural deviations live in
misra_suppressions.txt+docs/misra-deviations.md(the project's existing D.001–D.010 pattern) rather than inlinecppcheck-suppresscomments. Reasoning maps to MISRA Compliance:2020 §4.2: centralised, listable, auditable, and each D.XXX carries an explicit "Risk and mitigation" path-to-elimination section.Verdict re-categorisations from the audit (three rules had wrong or incomplete verdicts):
_Static_assertcondition. All resolved.Test Evidence
cppcheck --addon=misrare-run in the gcc container to confirm the per-rule count went to 0. No behaviour-change commits (every fix is structural or essential-type-mechanical), so existing tests verify behaviour preservation.build-linux-gcc,build-linux-clang,sanitize-linux-gcc,coverage-linux-gcc(99.5% line, 98.9% functions — unchanged),analyze-tidy,analyze-cppcheck(both passes),analyze-format(clang-format dry-run clean after the macro-rewrite reflow).bugprone-easily-swappable-parameterswarning surfaced after the Rule 10.1 refactor changedFormatter_IsAboveUnicodeMaxEncoding's body shape; suppressed withNOLINTBEGIN/ENDmatching the existing pattern onFormatter_WriteContext.Areas Affected
Core/Source/—SolidSyslogMacros.h(polyfill switch),SolidSyslogFormatter.c,SolidSyslog.c,SolidSyslogCrc16.c,SolidSyslogUtf8.h,SolidSyslogFileBlockDevice.c,BlockSequence.c,RecordStore.h,SolidSyslogUdpPayload.c,SolidSyslogUdpSender.c,SolidSyslogStreamSender.c,SolidSyslogCircularBuffer.c,SolidSyslogMetaSd.c,SolidSyslogOriginSd.c,SolidSyslogTimeQualitySd.c,SolidSyslogAtomicCounter.c.Core/Interface/—SolidSyslogTunablesDefaults.h,SolidSyslogSecurityPolicyDefinition.h,SolidSyslogBlockStore.h,SolidSyslogFileBlockDevice.h,SolidSyslogStreamSender.h,SolidSyslogFormatter.h,SolidSyslogCircularBuffer.h.Platform/*/—Platform/Posix/Source/{SolidSyslogPosixFile,SolidSyslogPosixHostname,SolidSyslogPosixMessageQueueBuffer,SolidSyslogPosixDatagram,SolidSyslogPosixTcpStream}.c;Platform/Windows/Source/{SolidSyslogWindowsFile,SolidSyslogWindowsHostname,SolidSyslogWinsockDatagram,SolidSyslogWinsockTcpStream}.c;Platform/OpenSsl/Source/SolidSyslogTlsStream.c; allPlatform/*/Interface/Solid*.hstorage-size enum +slots[]arrays.docs/misra-conformance.md(rows marked landed, totals updated, sweep-volume table renumbered to S10.11..S10.18),docs/misra-deviations.md(D.011, D.012 added),misra_suppressions.txt(line-specific entries for D.011 / D.012),DEVLOG.md(session entry),CLAUDE.mdunchanged.Tests/orBdd/changes — the sweep is structural and the existing test suite verifies behaviour preservation.Closes #375
Summary by CodeRabbit
Refactor
Documentation
Chores