docs: S23.22 give every documented fact one home, and check it stays there - #711
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request reorganises platform documentation into overview and setup pages. It adds MkDocs generation hooks and validates platform, role, navigation, manifest, and page-description consistency in CI. ChangesDocumentation architecture and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…atrix The header-field rows were each one subsection low, PRI cited the message-length section, and the three structured-data rows paired a 6.3.x citation with the correct 7.x one — 6.3.4 is Change Control and 6.3.5 is Examples, so neither defines origin or meta. Message size cited 8.1 (UNICODE) for what is 6.1, in both the section column and the note; PRINTUSASCII cited 9 (IANA Considerations) for what the section 6 ABNF defines. Verified against the RFC's own table of contents. Also state why non-transparent framing is excluded rather than calling octet counting "recommended" — RFC 6587 is Historic and recommends neither framing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sixteen jobs were missing from the table and three were named that no longer exist — build-freertos-host-tdd, build-freertos-target and bdd-freertos-qemu were each split into plustcp and lwip variants. Add a table saying which adapter each lane exercises and whether it runs against fakes or the real thing, since the lane names carry the platform but not the adapter. Correct two claims about what a required check buys: analyze-iwyu* run continue-on-error, and feeding the summary aggregator does not make a lane blocking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SBOM page said the top-level components array is empty; the template carries one entry, OpenSSL with scope: optional, which the same page and release-verification.md both describe correctly. State what the array holds, and record that the other optional upstreams the Platform packs bind to are not yet declared. The deviation register jumped D.003 to D.005. D.004 was Rule 18.4 pointer arithmetic in RecordStore.c, retired in S10.19 when the field helpers moved to &base[OFFSET]; restore it in the retired form D.014 already uses, so the register has no gap for an assessor to query. Say in the hardening path that per-stage figures are rounded up individually and so overshoot when summed, since a reader adding them finds ~13.9 KB against a measured 13.5 KB and concludes the headline is wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All 38 requirements read Supported or N/A, which reads as unaudited when the file's own notes already admit two limitations. The sequenceId row becomes Partial: uniqueness and gap detection hold, but under concurrent raise adjacent IDs may invert in transmission, which the note has always said. SD-NAME / SD-ID syntax validation was a deferred item buried in the escaping row's note; it is now its own Planned row against 6.3.2, tracked to Custom Structured Data (#64), leaving the escaping row Supported because escaping genuinely is. Widen the N/A key to cover a deliberate exclusion, which is what RFC 6587 3.4.2 is — applicable to a sender, and not implemented. Summary arithmetic recounted: RFC 5424 is now 18 rows, 16/1/1/0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-repudiation Both substrate bullets were labelled "Mutual TLS (non-repudiation, CR 2.12)", which contradicts the CR 2.12 row in the same file: mutual TLS authenticates the TLS peer and binds nothing about a given record to it, least of all across a relay. Label it as peer authentication and a partial contribution, and point at the row that explains why. Spell the crypto policy identifiers out. This page was the only one using the brace shorthand; at-rest-cryptography.md, both platform pages and hardening-path.md all use the full names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A tree-wide sweep for SL1-SL4 left three instances, all outside docs/ and so missed by earlier passes. The mTLS feature description claimed to satisfy CR 2.12 at SL3+, which is wrong three ways: Table B.1 puts CR 2.12 at SL1-4, mutual TLS authenticates the peer rather than evidencing non-repudiation, and a scenario proving mTLS works does not satisfy a control. A Gherkin description reads as an assertion about what the test proves, so the claim goes entirely. The API-audiences diagram used SL1 to mean a minimal build; it now says that. The IEC 62443 page's meta description still advertised a mapping by Security Level, which the page no longer does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The description still advertised 62443-3-3 coverage, which the page dropped when its two SR rows were renumbered to CRs and it became consistently 4-2. It also said the controls map to "SolidSyslog components", which now reads against the standard's own meaning of component — a software application or device, not a library inside one. Say what the page's columns actually say instead, mirroring the CRA page's phrasing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ucing them Twelve of the fourteen deviation entries quoted the guideline headline verbatim. MISRA C:2012 is copyrighted and not redistributable, and the project claims no compliance — the register exists to tell an integrator where our code knowingly departs, which does not require restating the standard. Each entry now names its guideline and category and points at a licensed copy; the rationale, risk and approval sections, which were always our own writing, are unchanged. A preamble section states the omission is deliberate, so a reader sees diligence rather than a gap. CLAUDE.md carried the Rule 15.6 headline in a parenthetical; it now describes what the formatter settings do instead. The suppressions files and source comments already carried no rule text and needed no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four references to MISRA Compliance:2020 cited a document the project does not hold, three of them to support a claim that inline tool suppressions are weaker than a central register — a sound engineering preference that had acquired an authority it cannot carry. All four go; the preference is stated as ours. D.006 attributed external-interface deviation guidance to Rule 11.8 that is not there, and rested fifteen of its seventeen sites on "the accepted interpretation in the MISRA community". Both are replaced by the argument from the C standard, which was already present and is stronger. Clause references are fixed to C99 and corrected: flexible array members are 6.7.2.1 paragraph 16, not 18, which is C11's numbering; the -> operator is 6.5.2.3 paragraph 4, not 3, in both editions. 6.7.2.1 renumbered when C11 added anonymous struct and union members, so mixing editions sends a reader to the wrong sentence — a new header fixes the edition and says why. D.010 claimed every build compiles the C11 branch, writing the C99 fallback and the C++ harness branch out of existence. All three are compiled and all three are named. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dates on every entry, taken from the issue each cites rather than invented: an issue link is traceable only while the tracker exists, and Compliance-style records carry raised and approved dates. D.002 had no Scope at all, on the largest deviation in the register — 80 line-specific suppressions across ten platform packs and Core. It now states them by tier, count and rule. D.011, D.012 and D.013 pre-authorised instances nobody had reviewed: a future sweep "extends the deviation to those files". Each now says a new instance is reviewed on its merits and either amends the entry or gets its own. D.002's alignment argument covered both sub-cases and was the wrong one for the vtable downcasts, where the guarantee is 6.7.2.1 paragraph 13 — a pointer to a struct converts to a pointer to its first member and back. Alignment is load-bearing only for the Formatter, whose storage is a caller-declared array. The lwIP const-strip cited "handoff design decision #1", which no reader can resolve; it now links the published PBUF_REF contract. Header gains four notes: the register is process evidence rather than a compliance submission, the MISRA trademark attribution, that misra_suppressions.txt is the authoritative instance trace, and that one person raises and approves every entry. D.014 was missing its separator, and its story numbers read backwards because they are numbered by epic; the dates now carry the order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
D.009 described the rule 5.7 suppression migration as half finished, so the record-to-suppression mapping read as ambiguous. It is finished: 44 of the 45 entries sit in the D.009 block and the one in D.003 is a struct tag, not an anonymous enum, so it belongs there. Both blocks carry 5.7 lines permanently and the rule deciding which is a standing convention, not a transitional state. D.005 claimed every toolchain supports flexible array members and that CI proves it on every push. MSVC reports C4200 for the construct and the build carries /wd4200 for exactly this reason — without it /WX would fail the Windows lane. IAR and Keil are not built at all. Say both. D.001 said MSVC 2015+ where D.005 said 2013+, and carried an IAR row whose claim could not be verified. The table now separates what CI proves from what rests on vendor documentation, and the two lead-in sentences that generalised over "every supported target" are reconciled with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… findings D.006 cited 6.5.2.3 paragraph 4 for half of what it says. The second half — that a pointer to a qualified type yields the so-qualified member type — reads against our conclusion until you notice the member is itself a pointer, so the qualification lands on the pointer and is discarded when it is copied to a parameter. Nothing is stripped from the pointed-to type. The paragraph is our strongest support once read whole, and it now says so. Not every entry is a deviation. Five are cases where the code complies and cppcheck-misra reports anyway. Each entry now classifies itself, and the header explains why both belong in one register: the suppression exists either way. Eight genuine departures, not fourteen. D.002 reported twelve rule 11.2 suppressions while describing only the Formatter as triggering it; eleven are the Address downcasts, which fire 11.2 because the public handle is deliberately an incomplete type. Said so. D.009 said "approximately 31" anonymous enums where the tree has 45 across 44 files. C99 as conformance baseline is now distinguished from C11 as default build configuration. Smaller fixes to D.001, D.011 and D.014. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The earlier pass removed every reference to Compliance:2020 because three of them supported a claim the document does not make. That took out the one accurate reference with them: the entries really do follow its deviation record structure, and saying so gives the register external structural authority rather than a shape we appear to have invented. Restored with the citation corrected — Section 4 for the deviation process, Appendix B for the example record — and framed as voluntary, since it is mandatory only from MISRA C:2023. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ding D.011 failed the test the header had just introduced. The macro is used — by four named consumers the entry itself lists — and cppcheck-misra cannot see them because they sit outside the scanned tier. That the invisibility comes from the scan configuration rather than from a tracker gap is a property of the scan, not of the code, so it is a tool limitation on the same reasoning as D.012. Seven departures, not eight. Every entry's body section is now Construct rather than Deviation: it describes the code either way, and a section headed Deviation inside an entry classified Tool limitation contradicted itself. D.006's rationale no longer calls its tool-limitation half a deviation, and the header says the word is used generically where the distinction does not matter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The c99 preset landed as a per-PR lane in #713, so the register's two present-tense claims that it is a pre-release check are stale. Both now name build-linux-c99, and the language-edition section adds what the lane actually proves: the claim is about Core, built alone at strict -std=c99, with the platform packs a best-effort drift check. docs/ci.md gains the lane, and its branch-protection paragraph is corrected. It said every job but deploy-docs-pages is required; the new lane is not, so a change breaking the C99 baseline can still merge with it red. Thirty of thirty-two, and the exception is named. local-checks.md and builds.md needed nothing — #713 moved that material itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-linux-c99 was running on every pull request without gating one, so a change breaking the C99 baseline could merge with the lane red. Its context is now on the required list — 33 contexts, verified against branch protection — leaving deploy-docs-pages as the only job in ci.yml that is not required, because it only runs on main. docs/ci.md and CLAUDE.md's required-checks list both updated; the CLAUDE.md list now matches branch protection exactly, in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-linux-c89-headers landed in #716 without gating a PR. Its context is now required — 34 contexts, verified against branch protection in both directions — so deploy-docs-pages remains the only job in ci.yml that is not required, because it only runs on main. docs/ci.md gains the lane, including what it proves beyond the language level: each header is compiled alone, so one that needs a companion included first fails here too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both called the c99 check pre-release and pointed at docs/local-checks.md. It has been a per-PR lane since #713, and that commit moved the C99 material to builds.md — CMakeLists.txt even contradicted itself, its second C99 comment already citing builds.md. SolidSyslogMacros.h also described C99 as "the optional portability target" where the project now calls it the conformance baseline. Comments only: clang-format clean, debug build green (1494 tests), and the line counts are unchanged so the misra-c2012-20.10 suppression still lands on the same line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DavidCozens
force-pushed
the
docs/s23-22-audit
branch
from
August 7, 2026 07:04
4b54de8 to
54223d8
Compare
Each platform becomes docs/platforms/<slug>/, holding index.md (what the pack ships) and, where one exists today, setup.md (how to wire it). The three integrating-*.md guides move in as those setup pages. Paths only — no content is rewritten. The docs slug for the lwIP pack becomes lwipraw, matching its Platform/LwipRaw directory and its LwipRaw registry token. A future lwIP sockets pack would be a sibling, not a rename, so the slug has to say which lwIP API this one wraps. The nav label stays "lwIP (Raw API)". Platforms leaves API reference for a tab of its own: it answers whether the library runs on a given target, which is an adoption question rather than a reference one. The porting guide sits at the end of that tab as the other half of it — nothing shipped fits, so write one. Published URLs move for the three setup pages. Nothing external links them; link stability begins at 0.1.0, which is a reason to land this before the release rather than after. Part of #708
Traversal between the code and the hand-written docs had one direction only, and no stable anchor at all: api/modules.md read "No modules found", and the directory pages sit at content-hash URLs that change whenever a directory moves. Each pack now defines one group in SolidSyslog<Pack>Platform.dox at its root, and every public header in the pack carries @InGroup. The group page lists the pack's headers with their briefs and links back to the platform's page; the platform's page gains a Source section linking forward to the group. A reader arriving at either end reaches the other. The group definition lives in a .dox rather than a header so nothing suggests it can be included, and install() globs *.h so none of them ship. The cost is one visible .dox entry per pack in the Headers index. mkdoxy now sweeps Platform whole rather than listing the ten Interface directories, so a new pack's headers and its group are picked up by existing there. EXCLUDE_PATTERNS keeps the private Source/ headers out, which the explicit list had been doing implicitly. Every Platform MISRA suppression is anchored under Source/, so adding lines to Interface/ headers shifts none of them. Part of #708
Establishes the platform page template — what it ships, requirements, security behaviour and obligations, source, setup — on the two packs that carry the real content. The obligations sections are what an integrator cannot derive without reading the adapter: that OpenSSL rejects a half-supplied mTLS credential at Open while Mbed TLS proceeds with server-authenticated TLS; that OpenSSL checks the key against the certificate locally and Mbed TLS does not; that ServerName has three distinct behaviours, one of them a silent opt-out; that neither adapter performs revocation; and that rotation is a file replacement on one and a recreate on the other. Corrects a claim carried in the IEC 62443 substrate prose: the Mbed TLS TLS 1.2 floor is pinned explicitly with mbedtls_ssl_conf_min_tls_version, not inherited from MBEDTLS_SSL_PRESET_DEFAULT. The code pins it precisely because the preset can negotiate down to TLS 1.0 or 1.1, so the old wording understated the adapter. OpenSSL gains the setup page the fixed folder shape calls for, wired from the BDD target that exercises both the server-authenticated and mutual-TLS paths. Part of #708
Nav. The Platforms sidebar was a list of expandable two-child sections, which read as clutter. It becomes the overview, then porting, then a flat alphabetical list of packs. Each pack's setup guide leaves the nav and is reached from the pack's own page instead — the two have different audiences. Adopt becomes Integrate. Traversal. The API reference could not answer "which platform is this and what does it need from my build". Every generated page for a header under Platform/<Pack>/ now carries a banner linking to that pack's page. The pack list is read from SOLIDSYSLOG_PLATFORM_REGISTRY and the display name from the platform page's own heading, so registering a platform is all it takes and the name is written once. Source links. The platform pages no longer link into the source tree, and neither does the compliance guide — a guide that names adapter internals is coupled to every pack that has them. Doxygen's verbatim source listings are off, so the reference documents the contract rather than shipping a second copy of the headers. Template. Bold lead-ins become subheadings, the ServerName tables go — that detail belongs on the field, where it cannot drift — and the two TLS pages no longer reference each other. A page that describes another platform is a page that has to be revisited when that platform changes, which is the coupling this restructure exists to remove. The description check now reads the page tree rather than the nav, since setup pages are deliberately off-nav and a page a search engine can reach still needs a snippet. It immediately found one published page that had never had a description. Part of #708
Option D for the platform pages, C for the API pages, from the mock. API reference and Setup were short sections at the foot of each platform page, where a reader looking for either had to scroll past everything else to find out they existed. They become two chips under the title. The banner on generated API pages becomes a chip in the same visual family, replacing a full-width admonition that used the whole column to say one thing — it is a signpost, not a caution. Both use the UML component symbol and the adapter blue of the post-it kit, where blue already means "a backend that realises a role" — which is what a platform adapter is. That borrows a vocabulary the site has rather than inventing one. All three are injected rather than written. Hand-written raw HTML was the first attempt and was wrong twice over: MkDocs rewrites and validates Markdown links only, so the hrefs pointed at .md files that do not exist in the built site, and the strict build could not see it. Generating them keeps the links inside MkDocs' own resolution, keeps the markup out of the pages, and means adding a platform needs no markup at all — the Setup chip appears when a setup page does. "Pack" is gone from the platform pages. It was jargon standing in for "platform" and earned nothing; it survives in six other documents and in CLAUDE.md, which is a separate sweep. Adopt becomes Integrate in the docs index too, matching the nav. Tests cover what the strict build cannot: that a Core header is not labelled with a platform, that the label is the platform page's own heading rather than the registry token, and that every registered platform has a docs folder. Part of #708
Every platform page now carries a Security behaviour and obligations section. The eight added here are shorter than the two TLS ones because there is less to say, not because less was looked for — what they state is what an integrator cannot infer from the class list. Some of it is uncomfortable and belongs on the page for that reason: the POSIX and Windows transports carry syslog in clear; leaving the FreeRTOS mutex unfilled is a silent data race on a multi-task target, undetectable by construction; an asynchronous lwIP marshal corrupts stack state rather than failing; FAT is not journalling, so f_sync and ff_fflush bound loss without guaranteeing the directory entry; and if the AtomicCounter pool is exhausted the Null counter returns 1 every time, so gap detection reports nothing wrong while proving nothing at all. Sibling-platform references are gone. Three pages named another platform to say where a capability comes from — which is the coupling that makes adding a platform expensive, since the eleventh would have to be added to each of them. They point at the capability matrix instead, which is the one place that already answers "who fills this role". The API-page chip drops the UML-class styling and becomes the same pill as the platform pages' own chips. One chip style across the site. Part of #708
Sets the pattern for the other eight: a setup page carries only what neither the platform page nor the header already owns. On that test most of the Mbed TLS page was duplication — 1,519 words to 663. Cut because the platform page now says it: the TLS 1.2 floor, the coexistence contract, certificate rotation, key custody, and what the adapter does not own. Cut because the config struct says it, on the fields themselves where it cannot drift: the whole "what you need to provide" table, whose ServerName row ran to nine lines. Cut because the example repositories are where working code is shown: the reference integrations table and the links into BDD targets. Cut because a page that names another platform has to be revisited when that platform changes: the opening comparison with the OpenSSL adapter. Kept, because it lives nowhere else and all of it was learned the hard way: the layering, the wiring itself, and the four failures that are almost unreadable from their symptoms — entropy sources that are not tagged STRONG, psa_crypto_init called before the DRBG is seeded, the external RNG hook on a target with no platform entropy, and Mbed TLS reaching newlib's syscall heap instead of the RTOS one. porting.md linked the coexistence contract by anchor; it now points at the platform page, where that section moved. Part of #708
The C11 atomics platform appeared under four names: "C11 atomics" in the sidebar and on its own page, "C11 atomics platform" on the chip, and "Group platform_atomics" on its generated reference page, beneath a breadcrumb reading "Modules". Group and Module are Doxygen's vocabulary. They appear nowhere else in this documentation and mean nothing to a reader who has not used Doxygen, so they are rewritten out: a group page is now titled after the platform, and its breadcrumb points at the Platforms overview. mkdoxy ignores the title the @defgroup declares and prints the group's name instead, so the rewrite takes the name from the platform page's own heading — the same source the chips already use. One place names a platform. The modules index leaves the navigation. It is Doxygen's own listing of groups and duplicates the Platforms tab, under a heading no reader asked for. The Mbed TLS setup page loses its FreeRTOS section. Routing allocations to an RTOS heap and sizing newlib's syscall heap is neither this project's expertise to claim nor Mbed TLS's page to hold it on — and a page that explains another platform is a page that has to be revisited when that platform changes. What remains is the part that is ours: the adapter allocates nothing itself, so a TLS session costs what your mbedtls_config.h says it costs, and the sizing guidance belongs upstream. Part of #708
Trims lwIP (3,059 words to 718) and FreeRTOS-Plus-FAT (802 to 551) to the same test the TLS pages set, and writes the six that were missing. Every platform now has both an overview and a setup page, so the folder shape is uniform and both chips appear on all ten. The lwIP page kept its marshal — the one thing that is genuinely hard to get right, and the reason an asynchronous hop corrupts stack state rather than failing — and the bare-metal trap where a Sleep that does not drive the stack leaves a connection unable to complete. It loses its class table, its tunables table, its adapter internals, a paragraph on layering TLS, and a note recording that earlier advice in this guide had been wrong, which is history rather than documentation. Corrects a claim on the FreeRTOS-Plus-FAT page. It said the adapter calls ff_fflush after every write. The library declares ff_fflush and never defines it, so the adapter flushes the IO manager's cache instead. The claim came from the page's original stub and I repeated it in the obligations section, where it mattered more. The six new pages are short because there is little to say, which is the cost of the fixed folder shape rather than a gap. Each says what to link, how to wire it, and the one thing that will catch you: Winsock needing initialisation that the adapter deliberately does not do, FreeRTOS needing static allocation, and the atomic counter's Null fallback returning the same value forever, so an unfilled role leaves gap detection proving nothing. Part of #708
Each platform's .dox block carries the canonical docs URL so that someone reading the source tree can find the page. Doxygen auto-links it, so the group page's Detailed Description sent readers to the published site — from a local preview, or from a pull request's build artefact, which is exactly where the difference matters. The hook now rewrites that URL to a relative link, labelled with the platform rather than repeated as a bare address. The URL stays in the source, where it is the only pointer a source reader has. "Pack" leaves the .dox blocks too, so the group pages read the same way as the platform pages. Part of #708
# Conflicts: # docs/integrating-lwip.md # docs/integrating-mbedtls.md
DavidCozens
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/iec62443.md (2)
110-118: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep
sequenceIdsemantics in one authoritative location.Lines 110-118 repeat the raise-time assignment and end-to-end gap semantics already stated in the CR 6.2 mapping and
docs/platforms/stdatomic/setup.md. Keep the detailed explanation in one page and link to it elsewhere.As per path instructions, give each fact one authoritative home and link to it elsewhere.
🤖 Prompt for AI Agents
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/iec62443.md` around lines 110 - 118, Remove the duplicated detailed explanation of sequenceId assignment and end-to-end gap detection from the IEC 62443 document, retaining only a concise reference link to the authoritative CR 6.2 or stdatomic setup documentation. Ensure the remaining text still directs readers to the single source of truth.Source: Path instructions
104-108: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not claim that every release is BDD-verified. CI runs both BDD lanes on
mainpushes and pull requests, butrelease-please.ymlis manual-only and has no dependency on them. Change “Every release” to the CI scope, or add a release gate for both BDD lanes.🤖 Prompt for AI Agents
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/iec62443.md` around lines 104 - 108, Update the documentation statement around the BDD suite to claim verification only for CI runs, not every release; replace the “Every release” scope with wording that accurately reflects the main-push and pull-request CI coverage, without implying release-please.yml is gated by either BDD lane.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 516-520: Update the opening statement in the Documentation section
to say “Three rules,” matching the three documented headings: “Verify before
asserting,” “One claim, one place,” and “A platform page never describes another
platform.”
- Around line 539-546: Update the “What Core does” row in the documentation
ownership table so it names exactly one authoritative source between Core
documentation and Core headers, then direct readers from the other location to
that source using an appropriate link or cross-reference.
In `@Core/Interface/SolidSyslogStreamDefinition.h`:
- Around line 20-22: Move the complete caller-facing semantics for all four
stream methods from SolidSyslogStreamDefinition.h to the corresponding
SolidSyslogStream_* declarations in SolidSyslogStream.h. Keep
SolidSyslogStreamDefinition.h limited to implementer-specific obligations and a
reference linking to the authoritative contracts, without duplicating documented
facts.
- Around line 25-42: Document the contract exceptions in the test transports
SocketStream and BioPairStream, explicitly noting that they are test-only
implementations and do not close after failed Send or negative Read.
Alternatively, update those implementations to close internally on these failure
paths before using them in contract tests; leave the production stream contract
unchanged.
In `@Core/Source/SolidSyslogMacros.h`:
- Around line 5-7: Update the compatibility comment around the diagnostic in
SolidSyslogMacros to scope the static_assert requirement to C++11 and later.
Remove the C99-toolchain fallback discussion, since the project’s declared C++17
target does not require it.
In `@docs/cra.md`:
- Line 88: Update the disclosure-policy link label in the documentation table to
match its security/policy.md destination, using the new path or a neutral
“security policy” label instead of SECURITY.md; keep the existing link target
and table structure unchanged.
- Around line 88-89: Update the visible link label for the security policy in
docs/cra.md lines 88-89 and docs/iec62443.md lines 98-100 to use
security/policy.md or “security policy” instead of SECURITY.md; keep both links
targeting the authoritative security/policy.md document.
- Line 89: Update the release-provenance statement in the table to qualify that
signed source-tree hash and CycloneDX SBOM assets are published only when the
associated workflow, including sbom.yml, completes successfully; preserve
verification against the producing workflow and tag.
In `@docs/iec62443.md`:
- Line 50: Revise the CR 6.1 row in the compliance page to describe only the
injected-role contract and audit-log access implications. Remove
platform-specific references and details, including
SolidSyslogPosixMessageQueueBuffer, TCP socket behavior, connection budgets, and
TLS handshakes; link to the capability matrix or relevant platform pages for
those facts.
- Line 98: Update the disclosure-policy reference in the documentation link list
to remove the stale SECURITY.md label, using either security/policy.md or the
neutral security policy label while preserving the existing security/policy.md
target.
In `@docs/misra-deviations.md`:
- Around line 43-45: Remove every outbound GitHub link from
docs/misra-deviations.md, replacing the S10.06 reference at lines 43-45, S10.01
at 185-187, D.003 at 424-425, D.005 at 513-514, D.006 at 656-657, D.007 at
712-713, D.008 at 768-769, D.009 at 866-867, D.010 at 944-945, D.011 at
1016-1017, D.012 at 1101-1102, D.013 at 1171-1172, and the D.014 issue and
retirement links at 1190-1193 with approved local or canonical references. Run
the repository-wide link check afterward.
- Around line 1184-1186: Update the retired Rule 8.7 rationale in the relevant
entry of misra-deviations.md to remove the incorrect claim that the 44 matching
*Static.c files reference their SolidSyslogErrorSource or provide
cross-translation-unit evidence. Preserve only accurate rationale supported by
the files.
- Around line 283-290: Update the deviation documentation in
docs/misra-deviations.md and the corresponding suppression entries in
misra_suppressions.txt (lines 92-93) so the const char* and char* stream-buffer
casts in SolidSyslogWinsockTcpStream.c are covered by the same applicable
deviation. Keep both files aligned, and do not leave these casts documented only
under callback, uint8_t*, or unsigned char* cases.
In `@docs/platforms/fatfs/index.md`:
- Around line 27-31: Update the FatFs power-loss documentation around
FatFsFile_Write to make the “at most the record in flight is lost” statement
conditional on the diskio.c CTRL_SYNC implementation and underlying hardware.
Preserve the explanation that f_sync follows each complete f_write and retain
the caveat that uncommitted FAT metadata or partially written directory entries
may remain after power loss.
In `@docs/platforms/fatfs/setup.md`:
- Around line 50-52: Update the documentation around FatFsFile_Write to describe
f_sync as reducing the loss window rather than guaranteeing that power loss
cannot discard a record. Retain the diskio.c and medium caveat, and add a link
to docs/platforms/fatfs/index.md as the authoritative durability statement.
In `@docs/platforms/lwipraw/setup.md`:
- Around line 37-45: Update the MyLwipSleep example to reject or normalize
non-positive milliseconds before converting the duration to uint32_t, then use
wrap-safe elapsed-time arithmetic based on MyTimebase_NowMs() rather than
comparing against an absolute deadline. Preserve the timeout and
MyNetif_DrivePolledRx polling behavior while ensuring timer wrap does not cause
early return or excessive delay.
- Around line 78-87: Update MyTcpipMarshal and its MarshalHop/RunHop flow so the
wrapper records callback completion, waits until RunHop has executed before
returning, and captures the err_t result from tcpip_callback_with_block. If
enqueueing fails, handle the error without waiting indefinitely; preserve the
direct callback path when already on the TCP/IP thread.
In `@docs/platforms/mbedtls/setup.md`:
- Around line 64-75: Revise the PSA/RNG guidance so it is
configuration-specific: state that psa_crypto_init() must run before any PSA API
use, and require seeding mbedtls_ctr_drbg first only when
MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG routes PSA randomness through it. Clarify that
MBEDTLS_NO_PLATFORM_ENTROPY alone does not require the external RNG when a
strong source is registered via mbedtls_entropy_add_source, and state the
entropy-chain invariant for each configuration.
In `@docs/platforms/openssl/index.md`:
- Around line 63-68: Update the rotation guidance around SSL_CTX and
SolidSyslogSender_Disconnect to say that the rebuilt context rereads all
configured credential files, rather than claiming that all three files are
always present. Preserve the existing reconnection and no-reload-callback
behavior.
In `@docs/platforms/openssl/setup.md`:
- Around line 14-16: Update the CMake example’s SOLIDSYSLOG_PLATFORMS setting to
include a transport-capable platform alongside OpenSsl, using a concrete pair
such as OpenSsl;Posix, so the setup matches the capability matrix and provides
Transport.
In `@docs/platforms/plusfat/index.md`:
- Around line 24-32: Update the durability section near “Durability is bounded
by the write, not guaranteed by it” to remove the claim that at most one record
can be lost. Document that appended records may remain hidden until
FileBlockDevice_Append closes the file when the block changes or cleanup runs,
unless metadata is explicitly committed before claiming per-record durability.
In `@docs/platforms/plusfat/setup.md`:
- Around line 61-72: Update the durability statement in the Plus-FAT setup
documentation to remove the unconditional claim that flushing prevents record
loss after power failure. Clarify that flushing persists data sectors, but a
hard power loss before close may lose the last unclosed record because file size
and directory metadata are committed on close; retain the existing
graceful-shutdown behavior and discard-policy guidance.
In `@docs/platforms/plustcp/setup.md`:
- Around line 14-16: Update the CMake example’s SOLIDSYSLOG_PLATFORMS value to
include every platform token required by the documented PlusTcp integration,
including the mutex and clock providers referenced in the surrounding text; keep
target_link_libraries unchanged.
In `@docs/platforms/posix/index.md`:
- Around line 42-47: Update the POSIX UDP documentation around “The blocking
surface is bounded but not zero” to accurately reflect that
SolidSyslogPosixDatagram uses a blocking socket without a send timeout; either
revise the claim to apply only to the non-blocking TCP/TLS transport or
implement non-blocking setup and EAGAIN/EWOULDBLOCK handling in
SolidSyslogPosixDatagram.c, PosixDatagram_ConnectIfNeeded(), and SendTo.
In `@docs/platforms/posix/setup.md`:
- Around line 16-18: Update the POSIX platform documentation to state that
SolidSyslog publicly propagates rt whenever SOLIDSYSLOG_POSIX is enabled,
including circular-buffer builds, while glibc 2.34+ provides POSIX message
queues through libc and only older glibc versions require -lrt. Keep this
guidance authoritative in one build document and replace the duplicate guidance
on the other platform page with a link to that document.
In `@docs/platforms/stdatomic/setup.md`:
- Around line 30-39: Update the no-counter fallback description to distinguish
pool exhaustion from a missing Counter. In the
SolidSyslogStdAtomicCounter_Create case, state that exhaustion reports
SOLIDSYSLOG_STDATOMIC_COUNTER_ERROR_POOL_EXHAUSTED, uses
SolidSyslogNullAtomicCounter_Get(), and Increment returns 1U; for a missing
Counter, describe SolidSyslogMetaSd_Create reporting
SOLIDSYSLOG_META_SD_ERROR_NULL_COUNTER, returning NullSd, and emitting no
sequenceId.
In `@docs/platforms/windows/index.md`:
- Line 10: Update the Windows platform documentation around the “What it ships”
heading: either populate it with the relevant adapter list before “Requirements”
or remove the empty heading, preserving the adapter information and document
structure.
In `@docs/platforms/windows/setup.md`:
- Around line 18-20: Update the Winsock guidance in the setup documentation to
state that the application owns Winsock lifecycle management: balance every
successful WSAStartup call with WSACleanup after all sockets are closed. Remove
the prohibition-oriented wording about repeated calls while retaining the
adapter’s responsibility boundary.
In `@docs/README.md`:
- Line 62: Update the vulnerability-reporting link in the README so its label
accurately describes security/policy.md, using “Vulnerability reporting policy”
or the page’s actual title while preserving the existing destination.
In `@docs/rfc-compliance.md`:
- Line 33: Update docs/rfc-compliance.md:33 to state that unique, non-zero
sequence IDs require a configured SolidSyslogAtomicCounter and that the default
without one repeats sequenceId = 1. Update docs/build-integration.md:416-417 to
describe AtomicCounter as enabling unique sequence IDs, not merely sequence ID
generation; verify both claims against the implementation.
- Line 28: Update the RFC compliance row for SD-ID / SD-NAME syntax validation
to remove the outbound GitHub issue URL while retaining `#64` as plain text or
linking it to a local tracking page.
In `@docs/structured-data.md`:
- Around line 19-20: Update the worked-example reference in
docs/structured-data.md to use a Markdown link instead of an inline code span,
targeting the repository source-link route that remains valid under the strict
MkDocs build. Preserve the existing displayed path text.
In `@Platform/FreeRtos/Source/SolidSyslogFreeRtosMutex.c`:
- Line 34: Remove the claim that every Mutex adapter applies the same
initialization-failure defense from the comment in SolidSyslogFreeRtosMutex.c,
while retaining the embedded-primitive statement. Also review
SolidSyslogFreeRtosMutexPrivate.h and make no direct change unless the same
inaccurate claim is present there.
- Line 33: Update the failure-mode comment near the static mutex creation in the
FreeRTOS mutex implementation to describe defensive handling of an unexpected
NULL result from xSemaphoreCreateMutexStatic(&self->Buffer). Remove the
dangling-handle wording and avoid implying that FreeRTOS-Kernel V11.1.0 is
globally pinned.
In `@Platform/Windows/Source/SolidSyslogWinsockTcpStream.c`:
- Around line 246-248: Update the explanatory comment near the Winsock keepalive
socket options to attribute TCP_KEEPIDLE, TCP_KEEPINTVL, and TCP_KEEPCNT to
<ws2tcpip.h>/Ws2ipdef.h, and identify <mstcpip.h> only as defining the legacy
tcp_keepalive API.
In `@scripts/check_platform_docs.py`:
- Around line 142-172: Update naming_faults in scripts/check_platform_docs.py to
also validate published descriptions keyed under platforms/<slug>/ using the
existing platform vocabulary and ownership rules. In hooks/page_descriptions.py
lines 79-82, remove the Plus-TCP and lwIP references; in lines 115-122, remove
the FreeRTOS references from the Mbed TLS and FatFs descriptions, keeping each
page focused on its own platform.
---
Outside diff comments:
In `@docs/iec62443.md`:
- Around line 110-118: Remove the duplicated detailed explanation of sequenceId
assignment and end-to-end gap detection from the IEC 62443 document, retaining
only a concise reference link to the authoritative CR 6.2 or stdatomic setup
documentation. Ensure the remaining text still directs readers to the single
source of truth.
- Around line 104-108: Update the documentation statement around the BDD suite
to claim verification only for CI runs, not every release; replace the “Every
release” scope with wording that accurately reflects the main-push and
pull-request CI coverage, without implying release-please.yml is gated by either
BDD lane.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8508bb9-f40f-492e-828c-93eedfd0a2b6
⛔ Files ignored due to path filters (3)
docs/assets/postit/README.mdis excluded by!docs/assets/**docs/assets/postit/api-audiences.svgis excluded by!**/*.svg,!**/*.svg,!docs/assets/**docs/assets/stylesheets/brand.cssis excluded by!docs/assets/**
📒 Files selected for processing (90)
.github/workflows/ci.ymlBdd/Targets/FreeRtosLwip/README.mdBdd/features/mtls_transport.featureCLAUDE.mdCMakeLists.txtCore/Interface/SolidSyslogStreamDefinition.hCore/Source/SolidSyslogMacros.hPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutex.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutexPrivate.hPlatform/LwipRaw/Interface/SolidSyslogLwipRawDnsResolver.hPlatform/LwipRaw/Interface/SolidSyslogLwipRawMarshal.hPlatform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.hPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.cPlatform/MbedTls/Interface/SolidSyslogMbedTlsAesGcmPolicy.hPlatform/MbedTls/Interface/SolidSyslogMbedTlsStream.hPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStream.cPlatform/PlusTcp/Interface/SolidSyslogPlusTcpTcpStream.hPlatform/PlusTcp/Source/SolidSyslogPlusTcpDatagram.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpTcpStream.cPlatform/Posix/Interface/SolidSyslogPosixTcpStream.hPlatform/Windows/Interface/SolidSyslogWinsockTcpStream.hPlatform/Windows/Source/SolidSyslogWindowsClockInternal.hPlatform/Windows/Source/SolidSyslogWindowsFile.cPlatform/Windows/Source/SolidSyslogWindowsHostnameInternal.hPlatform/Windows/Source/SolidSyslogWindowsProcessIdInternal.hPlatform/Windows/Source/SolidSyslogWindowsSysUpTimeInternal.hPlatform/Windows/Source/SolidSyslogWinsockDatagramInternal.hPlatform/Windows/Source/SolidSyslogWinsockResolverInternal.hPlatform/Windows/Source/SolidSyslogWinsockTcpStream.cPlatform/Windows/Source/SolidSyslogWinsockTcpStreamInternal.hREADME.mddocs/README.mddocs/bdd.mddocs/build-integration.mddocs/builds.mddocs/ci.mddocs/containers.mddocs/cra.mddocs/hardening-path.mddocs/iec62443.mddocs/integrating-lwip.mddocs/integrating-mbedtls.mddocs/integrating-plusfat.mddocs/misra-deviations.mddocs/platforms/fatfs.mddocs/platforms/fatfs/index.mddocs/platforms/fatfs/setup.mddocs/platforms/freertos.mddocs/platforms/freertos/index.mddocs/platforms/freertos/setup.mddocs/platforms/index.mddocs/platforms/lwipraw/index.mddocs/platforms/lwipraw/setup.mddocs/platforms/mbedtls.mddocs/platforms/mbedtls/index.mddocs/platforms/mbedtls/setup.mddocs/platforms/openssl.mddocs/platforms/openssl/index.mddocs/platforms/openssl/setup.mddocs/platforms/plusfat.mddocs/platforms/plusfat/index.mddocs/platforms/plusfat/setup.mddocs/platforms/plustcp.mddocs/platforms/plustcp/index.mddocs/platforms/plustcp/setup.mddocs/platforms/posix.mddocs/platforms/posix/index.mddocs/platforms/posix/setup.mddocs/platforms/stdatomic.mddocs/platforms/stdatomic/index.mddocs/platforms/stdatomic/setup.mddocs/platforms/windows.mddocs/platforms/windows/index.mddocs/platforms/windows/setup.mddocs/porting.mddocs/rfc-compliance.mddocs/security/at-rest-cryptography.mddocs/security/threat-model.mddocs/security/triage-runbook.mddocs/structured-data.mdhooks/page_descriptions.pyhooks/platform_backlinks.pyhooks/root_pages.pyhooks/test_page_descriptions.pyhooks/test_platform_backlinks.pyhooks/test_root_pages.pymisra_suppressions.txtmkdocs.ymlscripts/check_platform_docs.py
💤 Files with no reviewable changes (13)
- docs/platforms/mbedtls.md
- docs/platforms/openssl.md
- docs/platforms/freertos.md
- docs/platforms/stdatomic.md
- docs/integrating-lwip.md
- docs/integrating-plusfat.md
- docs/platforms/plustcp.md
- docs/platforms/posix.md
- Platform/Windows/Source/SolidSyslogWindowsFile.c
- docs/platforms/windows.md
- docs/platforms/plusfat.md
- docs/platforms/fatfs.md
- docs/integrating-mbedtls.md
Closed
5 tasks
…view findings Part of #708
3 tasks
DavidCozens
marked this pull request as ready for review
August 9, 2026 21:52
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
docs/misra-deviations.md (1)
287-293: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSynchronise the D.002 inventory count.
misra_suppressions.txtnow contains 12 Rule 11.2 entries, 57 Rule 11.3 entries, and 8 Rule 11.5 entries under D.002: 77 suppressions in total. The three Windows byte-buffer casts moved to D.013, so the current66pragmatic-site and80total counts still include those sites. Update the record to63pragmatic sites,77total suppressions, and8Rule 11.5 suppressions.Proposed correction
-- ... 66 sites ... +- ... 63 sites ... -80 line-specific suppressions in total — 12 against rule 11.2, 57 against 11.3 and 11 against 11.5. +77 line-specific suppressions in total — 12 against rule 11.2, 57 against 11.3 and 8 against 11.5.As per path instructions, documentation claims must match the authoritative source and configuration records.
🤖 Prompt for AI Agents
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.md` around lines 287 - 293, Update the D.002 inventory in docs/misra-deviations.md to report 63 pragmatic sites, 77 total suppressions, and 8 Rule 11.5 suppressions, while preserving the existing Rule 11.2 and 11.3 counts and explanatory scope.Source: Path instructions
docs/rfc-compliance.md (1)
74-76: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winQualify security guarantees for caller-supplied TLS streams.
The introductory text permits a caller-supplied TLS
Stream, but Lines 74-76 state certificate validation, mutual TLS, and a TLS 1.2 floor as unconditional guarantees. Core cannot enforce those settings on an injected stream. State that shipped adapters provide these guarantees and make caller-supplied stream compliance the integrator’s responsibility.As per path instructions, published documentation must match the actual contract and must not overstate backend-independent behaviour.
🤖 Prompt for AI Agents
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/rfc-compliance.md` around lines 74 - 76, Update the introductory TLS documentation and the entries for server certificate validation, mutual TLS, and TLS 1.2+ cipher suites to qualify these guarantees as provided by shipped adapters. Explicitly state that callers supplying a TLS Stream are responsible for configuring and verifying equivalent compliance, since Core cannot enforce those settings on injected streams.Source: Path instructions
docs/platforms/windows/setup.md (1)
25-29: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle
WSAStartupfailure before creating senders.The example discards the return value at Line 27 and continues with sender setup. Check the result and stop setup when it is non-zero.
As per path instructions, documentation must verify failure behaviour against the implementation.
🤖 Prompt for AI Agents
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/platforms/windows/setup.md` around lines 25 - 29, Update the Windows setup example around WSAStartup to check its non-zero return value before creating senders, and stop setup on failure. Verify the documented failure handling matches the implementation’s WSAStartup behavior.Source: Path instructions
docs/platforms/mbedtls/setup.md (1)
64-83:⚠️ Potential issue | 🟠 MajorMake the PSA initialisation guidance configuration-specific.
psa_crypto_init()must run before PSA API use. It does not universally depend on the applicationmbedtls_ctr_drbgbeing seeded. The external-RNG configuration requiresmbedtls_psa_external_get_random()to provide seeded cryptographic randomness; other configurations use their selected entropy chain.Also,
MBEDTLS_NO_PLATFORM_ENTROPYalone does not prove that no strong source is registered. Gate thePSA_ERROR_INSUFFICIENT_ENTROPYclaim on the absence of a valid source.Mbed TLS documents both the PSA initialisation requirement and the external-RNG responsibility. (mbed-tls.readthedocs.io)
Proposed direction
-**Call `psa_crypto_init()` after the DRBG is seeded, not before.** +**Call `psa_crypto_init()` before any other PSA API.** -**On a target with no platform entropy, give PSA a strong source.** With -`MBEDTLS_NO_PLATFORM_ENTROPY` defined ... +**Ensure the selected PSA entropy chain is seeded.** If no strong source is +registered for the selected configuration, `psa_crypto_init()` can fail with +`PSA_ERROR_INSUFFICIENT_ENTROPY`.As per path instructions, verify security and failure claims against authoritative behaviour.
#!/usr/bin/env bash set -euo pipefail curl -fsSL \ 'https://mbed-tls.readthedocs.io/projects/api/en/v3.6.5/api/group/group__initialization/' | grep -A12 -B3 'psa_crypto_init' curl -fsSL \ 'https://mbed-tls.readthedocs.io/projects/api/en/v3.6.4/api/group/group__psa__external__rng/' | grep -A12 -B3 'MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG'🤖 Prompt for AI Agents
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/platforms/mbedtls/setup.md` around lines 64 - 83, Revise the PSA initialization guidance to state that psa_crypto_init() must run before PSA API use, without implying it universally depends on the application mbedtls_ctr_drbg; make the seeded mbedtls_psa_external_get_random() requirement specific to MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG, and describe other configurations as using their configured entropy chain. Qualify the MBEDTLS_NO_PLATFORM_ENTROPY discussion so PSA_ERROR_INSUFFICIENT_ENTROPY is claimed only when no valid strong entropy source is registered, including hardware-poll alternatives.Source: Path instructions
docs/platforms/freertos/setup.md (1)
37-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the public circular-buffer API in this example.
SolidSyslogCircularBuffer_Createtakes(mutex, ring, ringBytes);SolidSyslogCircularBufferConfigis not part of the contract. Declare caller-owned storage withSOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(...), pass it withsizeof ring, and keep it alive with the mutex for the buffer lifetime.🤖 Prompt for AI Agents
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/platforms/freertos/setup.md` around lines 37 - 44, Update the FreeRTOS setup example to call the public SolidSyslogCircularBuffer_Create API with the mutex, caller-owned ring storage, and its size. Replace the SolidSyslogCircularBufferConfig usage with storage declared using SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(...), pass sizeof ring, and ensure the ring remains alive alongside the mutex for the buffer’s lifetime.Source: Path instructions
docs/platforms/lwipraw/index.md (1)
47-49: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDescribe ARP queueing and loss limits accurately.
Without
ARP_QUEUEING=1, lwIP queues only the most recent packet for an unresolved address; earlier packets can be dropped. When multiple packets can be sent before ARP resolution completes, enableARP_QUEUEING=1and sizeARP_QUEUE_LENfor the required burst.🤖 Prompt for AI Agents
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/platforms/lwipraw/index.md` around lines 47 - 49, Update the lwIP configuration guidance near ARP_QUEUEING to state that without ARP_QUEUEING=1 only the most recent packet is queued and earlier packets may be dropped. Instruct users to enable ARP_QUEUEING=1 when bursts can occur before ARP resolution, and size ARP_QUEUE_LEN for the required burst.Source: Path instructions
docs/platforms/openssl/setup.md (1)
85-88: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winCentralise the generic create-failure contract.
These sections repeat error-handler behaviour,
CRITICALhandling, Null-object fallback, and delivery cessation. Keep the severity and fallback semantics indocs/error-severity.md. Link to that document from each platform page and retain only platform-specific failure details locally.
docs/platforms/openssl/setup.md#L85-L88: replace the generic paragraph with a link and OpenSSL-specific failure detail.
docs/platforms/plusfat/setup.md#L83-L88: replace the generic paragraph with a link and Plus-FAT-specific failure detail.
docs/platforms/plustcp/setup.md#L59-L64: replace the generic paragraph with a link and Plus-TCP-specific failure detail.
docs/platforms/posix/setup.md#L59-L64: replace the generic paragraph with a link and POSIX-specific failure detail.
docs/platforms/stdatomic/setup.md#L59-L64: replace the generic paragraph with a link and StdAtomic-specific failure detail.As per path instructions, each behavioural fact must have one authoritative home and link to it elsewhere.
🤖 Prompt for AI Agents
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/platforms/openssl/setup.md` around lines 85 - 88, Centralize generic create-failure semantics in docs/error-severity.md and replace the repeated paragraphs with links plus only platform-specific failure details. Update docs/platforms/openssl/setup.md lines 85-88, docs/platforms/plusfat/setup.md lines 83-88, docs/platforms/plustcp/setup.md lines 59-64, docs/platforms/posix/setup.md lines 59-64, and docs/platforms/stdatomic/setup.md lines 59-64; each site requires a direct change and must link to the authoritative severity documentation while removing duplicated handler, CRITICAL, Null-object, and delivery-stop explanations.Source: Path instructions
docs/platforms/plustcp/setup.md (1)
27-30: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRetain the resolver for numeric endpoints.
SolidSyslogStreamSender_Createrejects aNULLResolver, and the sender passes every endpoint toFreeRTOS_getaddrinfo. State that numeric endpoints can avoid hostname DNS lookup, but still require a resolver instance.🤖 Prompt for AI Agents
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/platforms/plustcp/setup.md` around lines 27 - 30, Update the documentation around SolidSyslogStreamSender_Create to clarify that numeric collector endpoints avoid hostname DNS lookup but still require a non-NULL resolver instance because every endpoint is passed to FreeRTOS_getaddrinfo; remove the claim that numeric addresses need neither DNS nor the resolver.Source: Path instructions
♻️ Duplicate comments (1)
docs/platforms/lwipraw/setup.md (1)
103-117:⚠️ Potential issue | 🟠 MajorPropagate marshal failures instead of returning silently.
If
sys_sem_new()fails, ortcpip_callback_with_block()returns an error,MyTcpipMarshal()returns without executingcallbackand without reporting failure. Callers can then consume unchanged result state, despite the synchronous contract stated earlier.Route both failures through an error or status path that stops the operation. Do not free the semaphore and continue as if the hop completed.
lwIP documents that the callback post blocks until the request is posted and returns a non-OK
err_twhen it is not called. Ignoring that result is not safe. (nongnu.org)As per path instructions, verify failure and blocking claims against authoritative behaviour.
#!/usr/bin/env bash set -euo pipefail rg -n -C 8 \ 'MyTcpipMarshal|sys_sem_new|tcpip_callback_with_block|sys_arch_sem_wait|sys_sem_free' \ docs Platform🤖 Prompt for AI Agents
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/platforms/lwipraw/setup.md` around lines 103 - 117, Update MyTcpipMarshal to propagate failures from sys_sem_new and tcpip_callback_with_block through the established error/status path and stop the operation instead of returning silently. Only wait on hop.done after a successful callback post, and do not free the semaphore while treating the hop as completed when either operation fails.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/iec62443.md`:
- Line 50: Add a comma after “Either way” in the CR 6.1 audit log accessibility
text, changing it to “Either way, the bounded surface...” while preserving the
rest of the sentence.
In `@docs/misra-deviations.md`:
- Around line 1158-1159: Update the Alignment and Type safety mitigation bullets
in D.013 to explicitly cover casts to both char* and unsigned char*. Keep the
existing rationale and ensure the wording matches the Winsock and mbedTLS
examples.
In `@docs/platforms/fatfs/index.md`:
- Around line 27-28: Update the sentence describing f_sync so a comma separates
“updates the directory entry” from “so the recorded file size includes the
record,” while preserving the rest of the wording.
In `@docs/platforms/fatfs/setup.md`:
- Around line 32-36: Update the platform-selection wording from “says fill the
rest of what your build needs” to “says fill the rest with what your build
needs” in docs/platforms/fatfs/setup.md lines 32-36 and
docs/platforms/freertos/setup.md lines 17-22, preserving the UK English
documentation style.
In `@docs/platforms/openssl/index.md`:
- Around line 65-67: Update the sentence in the SSL_CTX rebuild explanation to
replace “the config names” with “named in the configuration,” while preserving
the surrounding credential and connection behavior description.
In `@docs/platforms/openssl/setup.md`:
- Around line 18-21: Move the shared CMake selection semantics out of the
platform setup pages and keep them authoritative in
docs/build-integration.md#cmake. In docs/platforms/openssl/setup.md lines 18-21
and docs/platforms/plustcp/setup.md lines 19-23, replace the repeated rule with
a link to that section while retaining only platform-specific token examples; in
docs/platforms/stdatomic/setup.md lines 15-20, do the same and correct the “fill
the rest of what” wording.
In `@docs/platforms/plusfat/index.md`:
- Around line 26-37: Make docs/platforms/plusfat/index.md lines 26-37 the single
authoritative home for the Plus-FAT durability contract, covering data-sector
flushing, directory-entry updates, open-file visibility, and the unavailable
ff_fflush path; update docs/platforms/plusfat/setup.md lines 61-74 to remove
those duplicated mechanics and retain only setup-specific shutdown and
discard-policy guidance, linking to the authoritative explanation in index.md.
In `@docs/platforms/plustcp/setup.md`:
- Line 12: Update the capability-matrix sentence near “clock” to name the
subject and noun it currently omits, so it clearly states which capabilities the
matrix shows are supported or fulfilled. Preserve the surrounding documentation
and formatting.
In `@docs/platforms/stdatomic/setup.md`:
- Around line 42-43: Update the documentation wording around the pool-exhaustion
and omitted-configuration cases to name both situations explicitly instead of
referring generally to “if the counter is missing.” Preserve the distinction
that pool exhaustion returns a usable Null counter, while omitted configuration
leaves the counter out of the meta-element configuration.
In `@docs/rfc-compliance.md`:
- Line 31: Update the range notation in the SolidSyslogOriginSd description to
use “0 to N-1” instead of “0..N-1”, preserving the surrounding behavior and
wording.
In `@docs/security/triage-runbook.md`:
- Line 3: Update the triage runbook’s remaining references to SECURITY.md so
they consistently point to policy.md or link to the Security policy, including
the introductory reference and all later statements. Ensure this file contains
no stale SECURITY.md references and preserves a single authoritative
documentation location.
---
Outside diff comments:
In `@docs/misra-deviations.md`:
- Around line 287-293: Update the D.002 inventory in docs/misra-deviations.md to
report 63 pragmatic sites, 77 total suppressions, and 8 Rule 11.5 suppressions,
while preserving the existing Rule 11.2 and 11.3 counts and explanatory scope.
In `@docs/platforms/freertos/setup.md`:
- Around line 37-44: Update the FreeRTOS setup example to call the public
SolidSyslogCircularBuffer_Create API with the mutex, caller-owned ring storage,
and its size. Replace the SolidSyslogCircularBufferConfig usage with storage
declared using SOLIDSYSLOG_CIRCULAR_BUFFER_RING_BYTES(...), pass sizeof ring,
and ensure the ring remains alive alongside the mutex for the buffer’s lifetime.
In `@docs/platforms/lwipraw/index.md`:
- Around line 47-49: Update the lwIP configuration guidance near ARP_QUEUEING to
state that without ARP_QUEUEING=1 only the most recent packet is queued and
earlier packets may be dropped. Instruct users to enable ARP_QUEUEING=1 when
bursts can occur before ARP resolution, and size ARP_QUEUE_LEN for the required
burst.
In `@docs/platforms/mbedtls/setup.md`:
- Around line 64-83: Revise the PSA initialization guidance to state that
psa_crypto_init() must run before PSA API use, without implying it universally
depends on the application mbedtls_ctr_drbg; make the seeded
mbedtls_psa_external_get_random() requirement specific to
MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG, and describe other configurations as using
their configured entropy chain. Qualify the MBEDTLS_NO_PLATFORM_ENTROPY
discussion so PSA_ERROR_INSUFFICIENT_ENTROPY is claimed only when no valid
strong entropy source is registered, including hardware-poll alternatives.
In `@docs/platforms/openssl/setup.md`:
- Around line 85-88: Centralize generic create-failure semantics in
docs/error-severity.md and replace the repeated paragraphs with links plus only
platform-specific failure details. Update docs/platforms/openssl/setup.md lines
85-88, docs/platforms/plusfat/setup.md lines 83-88,
docs/platforms/plustcp/setup.md lines 59-64, docs/platforms/posix/setup.md lines
59-64, and docs/platforms/stdatomic/setup.md lines 59-64; each site requires a
direct change and must link to the authoritative severity documentation while
removing duplicated handler, CRITICAL, Null-object, and delivery-stop
explanations.
In `@docs/platforms/plustcp/setup.md`:
- Around line 27-30: Update the documentation around
SolidSyslogStreamSender_Create to clarify that numeric collector endpoints avoid
hostname DNS lookup but still require a non-NULL resolver instance because every
endpoint is passed to FreeRTOS_getaddrinfo; remove the claim that numeric
addresses need neither DNS nor the resolver.
In `@docs/platforms/windows/setup.md`:
- Around line 25-29: Update the Windows setup example around WSAStartup to check
its non-zero return value before creating senders, and stop setup on failure.
Verify the documented failure handling matches the implementation’s WSAStartup
behavior.
In `@docs/rfc-compliance.md`:
- Around line 74-76: Update the introductory TLS documentation and the entries
for server certificate validation, mutual TLS, and TLS 1.2+ cipher suites to
qualify these guarantees as provided by shipped adapters. Explicitly state that
callers supplying a TLS Stream are responsible for configuring and verifying
equivalent compliance, since Core cannot enforce those settings on injected
streams.
---
Duplicate comments:
In `@docs/platforms/lwipraw/setup.md`:
- Around line 103-117: Update MyTcpipMarshal to propagate failures from
sys_sem_new and tcpip_callback_with_block through the established error/status
path and stop the operation instead of returning silently. Only wait on hop.done
after a successful callback post, and do not free the semaphore while treating
the hop as completed when either operation fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07f376e5-683d-47a4-8b20-2eaf4f20a58e
📒 Files selected for processing (31)
CLAUDE.mdCore/Source/SolidSyslogMacros.hPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutex.cPlatform/LwipRaw/Interface/SolidSyslogLwipRawMarshal.hdocs/README.mddocs/cra.mddocs/iec62443.mddocs/misra-deviations.mddocs/platforms/fatfs/index.mddocs/platforms/fatfs/setup.mddocs/platforms/freertos/setup.mddocs/platforms/lwipraw/index.mddocs/platforms/lwipraw/setup.mddocs/platforms/mbedtls/setup.mddocs/platforms/openssl/index.mddocs/platforms/openssl/setup.mddocs/platforms/plusfat/index.mddocs/platforms/plusfat/setup.mddocs/platforms/plustcp/setup.mddocs/platforms/posix/index.mddocs/platforms/posix/setup.mddocs/platforms/stdatomic/setup.mddocs/platforms/windows/setup.mddocs/porting.mddocs/rfc-compliance.mddocs/security/threat-model.mddocs/security/triage-runbook.mddocs/structured-data.mdhooks/page_descriptions.pymisra_suppressions.txtscripts/check_platform_docs.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/platforms/openssl/setup.md (2)
10-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse complete capability-matrix wording.
Both sentences omit
platforms. Replace the incomplete wording with a sentence that names the subject and the role.
docs/platforms/openssl/setup.md#L10-L12: write “the capability matrix shows which platforms fill that role”.docs/platforms/plustcp/setup.md#L12-L12: write “the capability matrix shows which platforms fill those roles”.🤖 Prompt for AI Agents
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/platforms/openssl/setup.md` around lines 10 - 12, The capability-matrix wording is incomplete in both documentation sites. Update docs/platforms/openssl/setup.md lines 10-12 to say “the capability matrix shows which platforms fill that role,” and docs/platforms/plustcp/setup.md line 12 to say “the capability matrix shows which platforms fill those roles.”
80-81: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDocument complete ownership and teardown guidance in both setup pages.
SolidSyslogStreamSender_CreateborrowsResolver,Stream, andAddress. Addresolverto the OpenSSL teardown guidance. Add reverse-order destruction forsender,stream,address, andresolverto the PlusTcp example. Destroy the TLS stream before its transport.🤖 Prompt for AI Agents
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/platforms/openssl/setup.md` around lines 80 - 81, Update the teardown guidance in docs/platforms/openssl/setup.md:80-81 to include resolver, documenting reverse-order destruction of sender, address, TLS stream, resolver, and then the created transport as appropriate. Update docs/platforms/plustcp/setup.md:33-44 to explicitly destroy sender, stream, address, and resolver in reverse ownership order, ensuring the TLS stream is destroyed before its transport.Source: Path instructions
docs/rfc-compliance.md (1)
74-74: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSeparate trust-chain and endpoint-identity validation.
OpenSSL and Mbed TLS accept chain-valid certificates without endpoint verification when
ServerNameisNULLor empty. State that endpoint verification depends on adapter configuration.🤖 Prompt for AI Agents
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/rfc-compliance.md` at line 74, Update the “Server certificate validation” entry in docs/rfc-compliance.md to distinguish trust-chain validation from endpoint-identity validation, stating that OpenSSL and Mbed TLS may validate the chain without checking the endpoint when ServerName is NULL or empty, and that endpoint verification depends on adapter configuration.Source: Path instructions
docs/platforms/plustcp/setup.md (1)
26-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the collector-address guidance with the implemented sender contract.
SolidSyslogPlusTcpResolveralways callsFreeRTOS_getaddrinfo, including for numeric addresses.Resolver = NULLis not valid becauseSolidSyslogStreamSenderConfig.Resolveris required; creation then returns the NullSender. Mark this example as the DNS path, or add and document a direct numeric-address path.🤖 Prompt for AI Agents
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/platforms/plustcp/setup.md` around lines 26 - 29, Update the collector-address guidance to match SolidSyslogPlusTcpResolver and SolidSyslogStreamSenderConfig: numeric addresses still use FreeRTOS_getaddrinfo and require DNS, while Resolver = NULL must not be presented as valid. Either describe the example exclusively as the DNS-based path or implement and document a supported direct numeric-address path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/iec62443.md`:
- Line 50: Update the transport sentence in the CR 6.1 documentation so the
incomplete phrase reads “what a given transport actually blocks on is for its
platform page to state,” preserving the surrounding wording and UK English.
- Line 50: Update the CR 6.1 documentation around SolidSyslog_Log and
SolidSyslogBuffer to state that queued logging returns before transport I/O only
for current queueing implementations, while a custom Write implementation may
block and full queues drop records. Limit timeout claims to TCP connect and TLS
handshake callbacks, and explicitly exclude connectionless datagram transports,
which expose neither budget.
In `@docs/misra-deviations.md`:
- Around line 1171-1173: Update the “Elimination path” section in
docs/misra-deviations.md to define separate retirement conditions for the Stream
and Datagram adapters covered by D.013. Retain the future Stream API condition,
and add conditions tied to SolidSyslogDatagram::SendTo and
WinsockDatagram_SendTo so each deviation is retired only when its corresponding
API adopts the compliant character-pointer form.
- Around line 1167-1170: Update the D.013 rationale in docs/misra-deviations.md
to distinguish SolidSyslogStream::Send from SolidSyslogStream::Read: describe
Send’s buffer as an existing formatted syslog record, and Read’s buffer as
caller-provided writable byte storage used as the destination. Preserve the
type-safety explanation while stating the appropriate byte-buffer contract for
each direction.
In `@docs/platforms/openssl/setup.md`:
- Around line 18-21: Centralize the shared CMake-selection contract in
docs/build-integration.md#cmake and link to it from every affected setup page.
In docs/platforms/openssl/setup.md lines 18-21, replace the repeated <Network>
substitution and omission semantics with the link; in
docs/platforms/fatfs/setup.md lines 32-36, retain the FatFs-specific role
example while linking to the section; in docs/platforms/stdatomic/setup.md lines
15-19, retain the StdAtomic example and remove repeated list semantics; in
docs/platforms/plustcp/setup.md lines 19-22, retain the <OsPrimitives> example
and remove repeated list semantics; and in docs/platforms/freertos/setup.md
lines 17-21, retain the FreeRTOS role description while removing repeated list
semantics.
---
Outside diff comments:
In `@docs/platforms/openssl/setup.md`:
- Around line 10-12: The capability-matrix wording is incomplete in both
documentation sites. Update docs/platforms/openssl/setup.md lines 10-12 to say
“the capability matrix shows which platforms fill that role,” and
docs/platforms/plustcp/setup.md line 12 to say “the capability matrix shows
which platforms fill those roles.”
- Around line 80-81: Update the teardown guidance in
docs/platforms/openssl/setup.md:80-81 to include resolver, documenting
reverse-order destruction of sender, address, TLS stream, resolver, and then the
created transport as appropriate. Update docs/platforms/plustcp/setup.md:33-44
to explicitly destroy sender, stream, address, and resolver in reverse ownership
order, ensuring the TLS stream is destroyed before its transport.
In `@docs/platforms/plustcp/setup.md`:
- Around line 26-29: Update the collector-address guidance to match
SolidSyslogPlusTcpResolver and SolidSyslogStreamSenderConfig: numeric addresses
still use FreeRTOS_getaddrinfo and require DNS, while Resolver = NULL must not
be presented as valid. Either describe the example exclusively as the DNS-based
path or implement and document a supported direct numeric-address path.
In `@docs/rfc-compliance.md`:
- Line 74: Update the “Server certificate validation” entry in
docs/rfc-compliance.md to distinguish trust-chain validation from
endpoint-identity validation, stating that OpenSSL and Mbed TLS may validate the
chain without checking the endpoint when ServerName is NULL or empty, and that
endpoint verification depends on adapter configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42f96bc7-6512-4c01-88e1-3b056d46845d
📒 Files selected for processing (11)
docs/iec62443.mddocs/misra-deviations.mddocs/platforms/fatfs/index.mddocs/platforms/fatfs/setup.mddocs/platforms/freertos/setup.mddocs/platforms/openssl/index.mddocs/platforms/openssl/setup.mddocs/platforms/plusfat/setup.mddocs/platforms/plustcp/setup.mddocs/platforms/stdatomic/setup.mddocs/rfc-compliance.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/platforms/fatfs/setup.md (1)
70-75: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winGive the repeated error-handling fact one authoritative home.
All three pages repeat the generic instruction to install the error handler and consult
docs/error-severity.md. Keep that guidance indocs/error-severity.md; retain only each platform's specific create-time fallback consequence on its setup page.
docs/platforms/fatfs/setup.md#L70-L75: replace the repeated generic text with a link todocs/error-severity.md; retain the FatFs Null-object consequence.docs/platforms/freertos/setup.md#L59-L64: replace the repeated generic text with a link todocs/error-severity.md; retain the FreeRTOS Null-object consequence.docs/platforms/stdatomic/setup.md#L58-L63: replace the repeated generic text with a link todocs/error-severity.md; retain the StdAtomic Null-object consequence.As per coding guidelines and path instructions, give each documented fact one authoritative home and link to it elsewhere.
🤖 Prompt for AI Agents
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/platforms/fatfs/setup.md` around lines 70 - 75, Update the setup guidance in docs/platforms/fatfs/setup.md lines 70-75, docs/platforms/freertos/setup.md lines 59-64, and docs/platforms/stdatomic/setup.md lines 58-63: remove the repeated generic error-handler and severity instructions, replace them with links to docs/error-severity.md, and retain each platform’s specific create-time Null-object fallback consequence.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/iec62443.md`:
- Line 50: The CR 6.1 audit-log accessibility text must narrow its timeout
statement: identify GetConnectTimeoutMs as applying to TCP connection attempts
and GetHandshakeTimeoutMs as applying to OpenSSL TLS handshakes, without
presenting either as a bound on data transfer. Remove the implication that these
phases apply to datagram transports, and direct transport-specific send blocking
or yielding details to the relevant platform pages.
In `@docs/misra-deviations.md`:
- Around line 1173-1177: Update the “Elimination path” text in the MISRA
deviations documentation to use the full C API names SolidSyslogStream_Send,
SolidSyslogStream_Read, and SolidSyslogDatagram_SendTo, while preserving the
existing explanation of which deviation sites each API change retires.
---
Outside diff comments:
In `@docs/platforms/fatfs/setup.md`:
- Around line 70-75: Update the setup guidance in docs/platforms/fatfs/setup.md
lines 70-75, docs/platforms/freertos/setup.md lines 59-64, and
docs/platforms/stdatomic/setup.md lines 58-63: remove the repeated generic
error-handler and severity instructions, replace them with links to
docs/error-severity.md, and retain each platform’s specific create-time
Null-object fallback consequence.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5157540b-804d-4f0f-b516-ff5dc8f9de50
📒 Files selected for processing (5)
docs/iec62443.mddocs/misra-deviations.mddocs/platforms/fatfs/setup.mddocs/platforms/freertos/setup.mddocs/platforms/stdatomic/setup.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/misra-deviations.md`:
- Line 1154: Update the API documentation for SolidSyslogStream_Send and
SolidSyslogStream_Read, including the elimination path, to preserve their const
contracts: document Send with const unsigned char* and Read with unsigned char*.
Replace the imprecise “character pointer” wording while keeping the source
declarations and ABI rationale unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9247598b-70dd-45ce-bc0f-519a585fbb8e
📒 Files selected for processing (2)
docs/iec62443.mddocs/misra-deviations.md
Contributor
☀️ Quality Summary Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result. |
Contributor
☀️ Quality Summary Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result. |
This was referenced Aug 10, 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
Part of #708.
The documentation had accumulated the failure that documentation accumulates: the
same fact stated in several places, drifting apart. Platform detail sat on generic
pages, so adding an eleventh platform would have meant editing ten of them, and a
correction to one adapter left three stale copies elsewhere. Two of those copies
were found to be wrong while removing them — the porting guide described a
SendToreturn value the enum does not have, and the RFC matrix understated theMbed TLS TLS 1.2 floor as inherited from a preset when the adapter pins it
explicitly, precisely because the preset can negotiate down.
This restructures the documentation so each fact has one home, and adds CI checks
that keep it that way.
Change Description
A folder per platform, keyed on the CMake registry.
SOLIDSYSLOG_PLATFORM_REGISTRYin the top-level
CMakeLists.txtis the single declaration of what a platform is;the token now determines the directory, the docs slug, the nav entry and the
generated manifest. Ten
docs/platforms/<slug>/{index,setup}.mdon one templatereplace a mixture of flat pages and three long
integrating-*.mdguides.Platform detail moved off the generic pages.
iec62443.mdlost its per-substratesections and now describes Core in role terms;
build-integration.mdlost itscapability matrix. Both deferred to the platform pages, which is the only place a
per-adapter claim is maintained.
One platform never names another — a hard boundary, enforced by
scripts/check_platform_docs.pyover each platform's whole tree and its docsfolder, implementation comments included. Naming a sibling couples the two, so the
eleventh platform would mean editing ten pages. Where a capability comes from is the
capability matrix's job. 20 sites fixed.
Generated rather than hand-kept: the What it ships manifest is built from each
platform's
Interface/*.hand their own@filebriefs, and the doorway chipsbetween an API page, its platform and its setup guide are injected rather than
written, so they stay inside MkDocs' link validation. The Doxygen group pages are
dropped — a platform had two pages answering the same question, and nothing consumed
the tags.
Asserted rather than generated where a person must decide: the nav entry, the
meta description and the page itself are hand-written and checked. A missing entry
is a decision not yet made, not a mechanical gap. The same script asserts every
declared role is listed everywhere roles are enumerated, replacing a count that was
stated in prose in six places.
Core has the top-level place the product has. It sat below the generated indexes
inside API reference while Platforms — the convenience layer — had a tab; the nav
said the opposite of the product.
overview.mdmoved into Compliance, where its owntitle ("Compliance in one page") stops contradicting the label above it.
Root documents are published into the site rather than linked away to, and no
page body links out to GitHub.
Corrections made along the way, each verified against the code rather than against
a neighbouring document: the
SendToand Mbed TLS floor claims above; Plus-FATdocumented as calling
ff_fflush, which it declares and never defines (the adaptercalls
FF_FlushCache); path-MTU discovery attributed to Core rather than to theDatagram adapters; an AtomicCounter "fails silently" claim that was false in four
places; RFC 5424 section numbers; the CI job table; and
cra.mdclaiming"reproducible artefacts", a term of art meaning bit-identical binaries, when the
release ships source with a signed source-tree hash and a signed SBOM.
Test Evidence
Documentation, so the evidence is the gates rather than unit tests — but three of
these gates are new and exist because hand review kept losing the same things.
scripts/check_platform_docs.py— new, in thedocs-buildlane. Asserts everyregistered platform has its folder, nav entry, meta description and matrix row;
that no docs folder exists without a registry row; that no platform names another;
and that every declared role is linked from the porting guide, the roles index and
the nav. Each assertion was verified by injecting the fault and watching it fail,
not by the check passing.
chips in both directions, and the root-page publication.
mkdocs build --strict, clean. Zero dead anchors.cppcheck --addon=misraverified identical to base rather than merely passing,since comment edits move suppression line numbers.
Production-tree changes are comment-only — the platform-boundary fixes — so no
build was run for them.
Areas Affected
docs/throughout;mkdocs.ymlnav;hooks/(platform_backlinks.py,root_pages.pyand their tests);scripts/check_platform_docs.py; and commentsonly under
Core/Interface/,Platform/*/Interface/andPlatform/*/Source/.Twelve pages are deleted and nineteen created — the three
integrating-*.mdguidesand the flat platform pages become the ten folders. Published URLs move. That is
deliberate and settled: link stability begins at 0.1.0, and this lands before it. I
grepped both example repositories, the website content and the docs-site repo for
inbound links to the old paths and found none.
Review outcome
Four review rounds, 58 findings. 53 fixed here, 3 tracked as issues, 2
declined with the reason recorded in the thread. Every thread carries its
disposition; the two left open are the two whose work belongs elsewhere.
Four findings were wrong claims rather than wording, and all four are the same
failure — a statement about behaviour that had not been read out of the code:
record". FatFs'
f_syncdoes update the directory entry and issueCTRL_SYNC, so it was merely too absolute; Plus-FAT'sFF_FlushCachedoesnot commit the file size, and the store holds the file open across appends,
so for that platform it was false — and contradicted by its own next
paragraph.
posix/index.mdsaid "sockets are non-blocking once open". Only the TCP oneis; the UDP socket is created blocking and never made otherwise.
Counteroptional. The header documents itas required and
Createreturns the Null SD without it, so a missing counterdrops the whole element rather than one parameter.
SolidSyslogLwipRawMarshal.h— saidtcpip_callback_with_block(.., 1)satisfies the synchronous-marshal contract. It blocks until the mailbox
accepts the message, not until the callback runs, and the documented example
passed a stack local to a callback that executes later. Our own BDD target
refuses to use it and says why in a comment.
The review also found a hole in
scripts/check_platform_docs.pyas added bythis PR: it did not scan
hooks/page_descriptions.py, whose entries arepublished as each page's meta description, so three descriptions named a sibling
platform while the check reported the boundary clean. Both are fixed.
Raised, not fixed here
including the Mutex. A production defect found while writing the platform
page.
WinsockTcpStream.cincludes<mstcpip.h>, which cannot supplythe keepalive constants it uses; they arrive transitively from a test-seam
header. Needs a Windows SDK to fix and prove.
close-on-failure clauses they are used to test against.
Still deferred, so not omissions
platform by platform; StdAtomic, FatFs and FreeRTOS are done, the other seven
follow as their own PRs.
docs/core/index.mdis thin for a page that now has a top-level tab. Itis being rewritten, not left.
brand.csshas no table rules at all today. It pairs with folding each*Errors.hinto its class's row, which changes the same column.NAMING.mdhas not been audited.