feat: S12.26 error text decoupling — codes-only library#514
Merged
Conversation
Remove every error message string from the library: delete the 43
*Messages.c tables and slim struct SolidSyslogErrorSource to { Name }.
Error text now lives in one consumer-side handler (BddTargetErrorText),
keyed on the portable category axis; the library carries codes only.
- Relocate each error-source symbol to its allocation-agnostic vtable TU
(Xxx.c), not the static-allocation Static.c.
- Unwind the crypto-policy _Report wrapper to direct SolidSyslog_Error
calls, uniform with the other 39 classes.
- Retire MISRA deviation D.014 (rule 8.7): the unwind makes the crypto
sources cross-TU referenced; cppcheck-misra clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughShrinks error-source struct to name-only, removes ~41 per-module message TUs, relocates source-name constants into implementation files, adds consumer-side category→text helper, replaces per-source AsString usage in BDD/test handlers, updates inline report helpers and CMake/test wiring. ChangesError Text Decoupling — Codes-Only Architecture
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1428 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@DEVLOG.md`:
- Around line 3-73: The new DEVLOG section titled "## 2026-06-03 — S12.26 Error
text decoupling — codes-only library" must be moved from its current insertion
point at the top of DEVLOG.md to the end of the file; cut the entire S12.26
block (from that heading through the final "Open questions" paragraph) and
append it after the last existing entry so the file remains append-only and
follows the project convention described in SKILL.md.
In `@Tests/Bdd/Targets/BddTargetErrorTextTest.cpp`:
- Around line 11-69: The tests repeat the same STRCMP_EQUAL pattern; define a
CHECK_BDD_TARGET_ERROR_CATEGORY(expected, category) macro that calls
STRCMP_EQUAL(expected, BddTargetErrorText_Category(category)) and wrap its
definition in the required NOLINT block with do { ... } while (0); then replace
each STRCMP_EQUAL line in tests (e.g., in BadConfigCategoryMapsToText,
BadArgumentCategoryMapsToText, PoolExhaustedCategoryMapsToText, etc.) with
CHECK_BDD_TARGET_ERROR_CATEGORY("text", SOLIDSYSLOG_CAT_...); keep the
UnrecognisedCategory test using the (uint16_t) 0xFFFFU cast when invoking the
macro.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce3fe5dc-3203-443c-9946-6716e767e2f8
📒 Files selected for processing (120)
Bdd/Targets/CMakeLists.txtBdd/Targets/Common/BddTargetErrorText.cBdd/Targets/Common/BddTargetErrorText.hBdd/Targets/Common/BddTargetStderrErrorHandler.cBdd/Targets/FreeRtos/CMakeLists.txtBdd/Targets/FreeRtos/main.cBdd/Targets/FreeRtosLwip/CMakeLists.txtBdd/Targets/FreeRtosLwip/main.cCore/Interface/SolidSyslogError.hCore/Source/CMakeLists.txtCore/Source/SolidSyslog.cCore/Source/SolidSyslogBlockStore.cCore/Source/SolidSyslogBlockStoreMessages.cCore/Source/SolidSyslogCircularBuffer.cCore/Source/SolidSyslogCircularBufferMessages.cCore/Source/SolidSyslogFileBlockDevice.cCore/Source/SolidSyslogFileBlockDeviceMessages.cCore/Source/SolidSyslogMessages.cCore/Source/SolidSyslogMetaSd.cCore/Source/SolidSyslogMetaSdMessages.cCore/Source/SolidSyslogOriginSd.cCore/Source/SolidSyslogOriginSdMessages.cCore/Source/SolidSyslogPassthroughBuffer.cCore/Source/SolidSyslogPassthroughBufferMessages.cCore/Source/SolidSyslogStreamSender.cCore/Source/SolidSyslogStreamSenderMessages.cCore/Source/SolidSyslogSwitchingSender.cCore/Source/SolidSyslogSwitchingSenderMessages.cCore/Source/SolidSyslogTimeQualitySd.cCore/Source/SolidSyslogTimeQualitySdMessages.cCore/Source/SolidSyslogUdpSender.cCore/Source/SolidSyslogUdpSenderMessages.cDEVLOG.mdPlatform/Atomics/CMakeLists.txtPlatform/Atomics/Source/SolidSyslogStdAtomicCounter.cPlatform/Atomics/Source/SolidSyslogStdAtomicCounterMessages.cPlatform/FatFs/Source/SolidSyslogFatFsFile.cPlatform/FatFs/Source/SolidSyslogFatFsFileMessages.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutex.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutexMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawAddress.cPlatform/LwipRaw/Source/SolidSyslogLwipRawAddressMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagram.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDnsResolver.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawResolver.cPlatform/LwipRaw/Source/SolidSyslogLwipRawResolverMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.cPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyMessages.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyStatic.cPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyMessages.cPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyStatic.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStream.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStreamMessages.cPlatform/OpenSsl/CMakeLists.txtPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicy.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyMessages.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyPrivate.hPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyStatic.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyMessages.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyPrivate.hPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyStatic.cPlatform/OpenSsl/Source/SolidSyslogTlsStream.cPlatform/OpenSsl/Source/SolidSyslogTlsStreamMessages.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpAddress.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpAddressMessages.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpDatagram.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpDatagramMessages.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpResolver.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpResolverMessages.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpTcpStream.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpTcpStreamMessages.cPlatform/Posix/CMakeLists.txtPlatform/Posix/Source/SolidSyslogGetAddrInfoResolver.cPlatform/Posix/Source/SolidSyslogGetAddrInfoResolverMessages.cPlatform/Posix/Source/SolidSyslogPosixAddress.cPlatform/Posix/Source/SolidSyslogPosixAddressMessages.cPlatform/Posix/Source/SolidSyslogPosixDatagram.cPlatform/Posix/Source/SolidSyslogPosixDatagramMessages.cPlatform/Posix/Source/SolidSyslogPosixFile.cPlatform/Posix/Source/SolidSyslogPosixFileMessages.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBufferMessages.cPlatform/Posix/Source/SolidSyslogPosixMutex.cPlatform/Posix/Source/SolidSyslogPosixMutexMessages.cPlatform/Posix/Source/SolidSyslogPosixTcpStream.cPlatform/Posix/Source/SolidSyslogPosixTcpStreamMessages.cPlatform/Windows/CMakeLists.txtPlatform/Windows/Source/SolidSyslogWindowsAtomicCounter.cPlatform/Windows/Source/SolidSyslogWindowsAtomicCounterMessages.cPlatform/Windows/Source/SolidSyslogWindowsFile.cPlatform/Windows/Source/SolidSyslogWindowsFileMessages.cPlatform/Windows/Source/SolidSyslogWindowsMutex.cPlatform/Windows/Source/SolidSyslogWindowsMutexMessages.cPlatform/Windows/Source/SolidSyslogWinsockAddress.cPlatform/Windows/Source/SolidSyslogWinsockAddressMessages.cPlatform/Windows/Source/SolidSyslogWinsockDatagram.cPlatform/Windows/Source/SolidSyslogWinsockDatagramMessages.cPlatform/Windows/Source/SolidSyslogWinsockResolver.cPlatform/Windows/Source/SolidSyslogWinsockResolverMessages.cPlatform/Windows/Source/SolidSyslogWinsockTcpStream.cPlatform/Windows/Source/SolidSyslogWinsockTcpStreamMessages.cTests/Bdd/Targets/BddTargetErrorTextTest.cppTests/Bdd/Targets/CMakeLists.txtTests/FatFs/CMakeLists.txtTests/FreeRtos/CMakeLists.txtTests/Lwip/CMakeLists.txtTests/MbedTls/CMakeLists.txtTests/MbedTlsIntegration/CMakeLists.txtTests/SolidSyslogErrorTest.cppTests/SolidSyslogUdpSenderTest.cppdocs/misra-deviations.mdmisra_suppressions.txt
💤 Files with no reviewable changes (60)
- Platform/Atomics/CMakeLists.txt
- Tests/MbedTlsIntegration/CMakeLists.txt
- Platform/Posix/Source/SolidSyslogPosixMutexMessages.c
- Core/Source/SolidSyslogFileBlockDeviceMessages.c
- Platform/Posix/Source/SolidSyslogPosixTcpStreamMessages.c
- Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressMessages.c
- Core/Source/SolidSyslogBlockStoreMessages.c
- Core/Source/SolidSyslogStreamSenderMessages.c
- Platform/Posix/Source/SolidSyslogPosixDatagramMessages.c
- Core/Interface/SolidSyslogError.h
- Platform/Windows/Source/SolidSyslogWindowsAtomicCounterMessages.c
- Core/Source/SolidSyslogUdpSenderMessages.c
- Platform/Windows/CMakeLists.txt
- Core/Source/SolidSyslogSwitchingSenderMessages.c
- Core/Source/SolidSyslogTimeQualitySdMessages.c
- Platform/PlusTcp/Source/SolidSyslogPlusTcpResolverMessages.c
- Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyMessages.c
- Platform/FreeRtos/Source/SolidSyslogFreeRtosMutexMessages.c
- Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c
- Platform/PlusTcp/Source/SolidSyslogPlusTcpDatagramMessages.c
- Platform/Windows/Source/SolidSyslogWinsockAddressMessages.c
- Core/Source/SolidSyslogMetaSdMessages.c
- Platform/FatFs/Source/SolidSyslogFatFsFileMessages.c
- Core/Source/SolidSyslogPassthroughBufferMessages.c
- Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyMessages.c
- Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverMessages.c
- Platform/PlusTcp/Source/SolidSyslogPlusTcpTcpStreamMessages.c
- Platform/Windows/Source/SolidSyslogWinsockResolverMessages.c
- Core/Source/SolidSyslogCircularBufferMessages.c
- Core/Source/SolidSyslogOriginSdMessages.c
- Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c
- Platform/LwipRaw/Source/SolidSyslogLwipRawResolverMessages.c
- Platform/OpenSsl/Source/SolidSyslogTlsStreamMessages.c
- Platform/Windows/Source/SolidSyslogWindowsMutexMessages.c
- Platform/Windows/Source/SolidSyslogWinsockDatagramMessages.c
- Platform/OpenSsl/CMakeLists.txt
- Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyMessages.c
- Platform/Posix/Source/SolidSyslogGetAddrInfoResolverMessages.c
- Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyPrivate.h
- Platform/LwipRaw/Source/SolidSyslogLwipRawAddressMessages.c
- Platform/Windows/Source/SolidSyslogWinsockTcpStreamMessages.c
- Platform/Posix/Source/SolidSyslogPosixAddressMessages.c
- Platform/Windows/Source/SolidSyslogWindowsFileMessages.c
- Tests/Lwip/CMakeLists.txt
- Core/Source/SolidSyslogMessages.c
- Platform/MbedTls/Source/SolidSyslogMbedTlsStreamMessages.c
- Tests/MbedTls/CMakeLists.txt
- Tests/FatFs/CMakeLists.txt
- Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyPrivate.h
- Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyPrivate.h
- Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferMessages.c
- Platform/Atomics/Source/SolidSyslogStdAtomicCounterMessages.c
- Platform/Posix/CMakeLists.txt
- misra_suppressions.txt
- Platform/Posix/Source/SolidSyslogPosixFileMessages.c
- Core/Source/CMakeLists.txt
- Tests/FreeRtos/CMakeLists.txt
- Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyMessages.c
- Tests/SolidSyslogUdpSenderTest.cpp
- Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyPrivate.h
Comment on lines
+3
to
+73
| ## 2026-06-03 — S12.26 Error text decoupling — codes-only library | ||
|
|
||
| Completed the error-reporting redesign begun in S12.25: the library is now | ||
| **codes-only**. The 43 `*Messages.c` translation units — every error message | ||
| string table — are deleted. `struct SolidSyslogErrorSource` slims to | ||
| `{ const char* Name; }`; `AsString` is gone. | ||
|
|
||
| ### Decisions | ||
|
|
||
| - **Slim struct over the link-seam override.** The original intent (recalled by | ||
| David) was that `AsString` stay on the struct as an *overridable* per-class | ||
| symbol, so an integrator could substitute their own and the library's strings | ||
| would never link. The implementation had drifted — the resolver was `static` | ||
| and co-located with the source symbol, so referencing a source always linked | ||
| its table. We could have restored the seam, but it was judged overkill: its | ||
| only payoff is free per-detail dispatch in the bundled handler, and it forces a | ||
| `TextNull.c` workaround to reach true zero-text (no weak symbols allowed). The | ||
| slim struct gives zero-text **by construction** — strings link only if some | ||
| code calls a text function, and none does. | ||
| - **Consumer-lens reframing.** A library *user* has one hook: install one | ||
| `SolidSyslogErrorHandler` and react on the four event axes | ||
| (Severity / Source / Category / Detail). Those reaction axes were all delivered | ||
| by S12.25; S12.26 only moves *text*, which only the log path wants — an HMI | ||
| keys on the portable `Category`, and a codes-only build links nothing. So text | ||
| decoupling is invisible to the user and actively helps the flash-constrained / | ||
| i18n integrator. | ||
| - **One consumer-side text owner.** `BddTargetErrorText_Category(uint16_t)` (12 | ||
| categories, TDD'd) is the single place mapping the portable category axis to | ||
| prose. The three BDD/FreeRtos handlers print `Source->Name` + category text + | ||
| numeric `Detail`; no per-source dispatch table needed. Lives in | ||
| `Bdd/Targets/Common/` for now — promotable to a top-level `Example/` for the | ||
| future SL1/SL2/SL3 worked examples without touching the library. | ||
| - **Source symbol relocates to the vtable TU `Xxx.c`, never `Static.c`.** The | ||
| error identity is allocation-agnostic; `Static.c` is the static-allocation TU | ||
| (a future `Dynamic.c` would emit the same source). Each `Xxx.c` gains | ||
| `SolidSyslogError.h` + its `*Errors.h` (MISRA 8.4). | ||
| - **`_Report` wrapper unwound.** The 4 crypto-policy classes were the only ones | ||
| routing emission through an `Xxx_Report(sev, cat, code)` indirection; inlined | ||
| all 24 call sites to the direct `SolidSyslog_Error(...)` the other 39 use, so | ||
| they're uniform. A *uniform* wrapper, if ever wanted, is a clean separate | ||
| refactor. | ||
| - **D.014 (MISRA 8.7) retired.** It existed because `_Report` confined each | ||
| crypto source's use to one TU; the unwind makes them cross-TU referenced like | ||
| every other source. cppcheck-misra reports no 8.7; the 4 suppressions and the | ||
| deviation are gone (tombstoned). | ||
| - **Validation:** gcc + clang `1391` tests; freertos-host all platform suites | ||
| (FreeRtos/Lwip/MbedTls/FatFs); both FreeRTOS cross BDD ELFs built; clang-tidy, | ||
| clang-format, cppcheck-misra all clean. Windows/MSVC left to CI. | ||
|
|
||
| ### Deferred | ||
|
|
||
| - **Delivery-health events.** Tracing the consumer use cases surfaced a real gap: | ||
| "server down" raises *no* event today — the Sender/Stream category bases are | ||
| reserved-but-unallocated and `PosixTcpStream` is silent on connect/send | ||
| failure, so a user cannot react to "can't reach the server". Worth more than | ||
| any text decision; proposed as the next E12 story (edge-triggered | ||
| stalled/resumed categories + documented handler execution contract). Out of | ||
| scope here. | ||
| - **Rich per-detail prose** in the text handler — shipped category-only + | ||
| numeric detail; per-class prose is addable later as deletable example code. | ||
| - **Codes-only CI gate** — dropped. With the strings physically removed (not | ||
| hidden behind a link choice), codes-only is true by construction; a regression | ||
| would be a visible code addition in review, not a silent link inclusion. | ||
|
|
||
| ### Open questions | ||
|
|
||
| - CI to confirm Windows/MSVC and the exact cppcheck / freertos-analysis lane | ||
| scoping. `misra_renumber.py` bails `AMBIGUOUS` (new 2.4 *analysis* findings on | ||
| now-unused enum tags confuse its count-matching), but the actual cppcheck-misra | ||
| gate is green, so nothing is gate-failing to renumber. | ||
|
|
Contributor
There was a problem hiding this comment.
Append DEVLOG entries instead of prepending them.
This new S12.26 section is inserted above prior entries; project convention is append-only DEVLOG history. Please move this entire entry to the end of the file (after the latest existing entry) rather than placing it at the top.
Based on learnings: “Do not edit historical DEVLOG entries; follow SKILL.md ‘Never rewrite history. Always append.’”
🤖 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 `@DEVLOG.md` around lines 3 - 73, The new DEVLOG section titled "## 2026-06-03
— S12.26 Error text decoupling — codes-only library" must be moved from its
current insertion point at the top of DEVLOG.md to the end of the file; cut the
entire S12.26 block (from that heading through the final "Open questions"
paragraph) and append it after the last existing entry so the file remains
append-only and follows the project convention described in SKILL.md.
Owner
Author
There was a problem hiding this comment.
Declining this one. The project convention is newest-at-top (the head of DEVLOG.md descends by date), confirmed by the maintainer — this entry is correctly placed under the # Dev Log header. The "always append" wording in SKILL.md is what has produced the two-regime ordering in the file (a cluster of recent entries got bottom-appended); that wording is being reconciled to "prepend newest-first" separately so this stops recurring.
Contributor
There was a problem hiding this comment.
@DavidCozens, understood — thanks for the correction. I was over-applying the prior SKILL.md “append” learning here. I’ll treat DEVLOG.md as newest-at-top going forward and won’t flag entries placed directly under # Dev Log for that reason.
I’ve also recorded this so future reviews don’t repeat the same comment.
🐇 noted, ears adjusted.
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 514
File: DEVLOG.md:3-85
Timestamp: 2026-06-03T13:14:18.671Z
Learning: In `DavidCozens/solid-syslog`, `DEVLOG.md` uses a maintainer-confirmed newest-at-top convention: new entries should be placed directly under the `# Dev Log` header in descending date order. The older SKILL.md wording “always append” is being reconciled separately and should not be used to flag newest-at-top DEVLOG entries as incorrect.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 502
File: DEVLOG.md:0-0
Timestamp: 2026-06-01T10:52:21.000Z
Learning: For PR `#502` / S12.21 in `solid-syslog`, Structured Data and MSG-body tests intentionally remain in `Tests/SolidSyslogTest.cpp` as integration coverage of the full `SolidSyslog_Log` pipeline rather than being migrated to `Tests/SolidSyslogMessageFormatterTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 513
File: Bdd/Targets/Common/BddTargetStderrErrorHandler.c:19-22
Timestamp: 2026-06-02T22:57:23.006Z
Learning: In `Bdd/Targets/Common/BddTargetStderrErrorHandler.c` (Tier-3 BDD diagnostic code), the cast `source->AsString((uint8_t) event->Detail)` is intentionally left without a prior range guard in S12.25. The rationale: in S12.25 `event->Detail` is always a per-class `enum SolidSyslog<Class>Errors` value (values well under 256), and `AsString` itself takes `uint8_t` and returns `"unknown"` for out-of-range inputs. The explicit range check `(event->Detail >= 0 && event->Detail <= UINT8_MAX)` before the cast, together with the `AsString` signature change to accept wider native codes (errno, X509_V_ERR_*), is deferred to sibling story S12.26. Do not flag this cast as a truncation risk until S12.26 lands.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 327
File: Core/Source/SolidSyslog.c:220-224
Timestamp: 2026-05-11T10:23:17.520Z
Learning: In DavidCozens/solid-syslog, `InstallStructuredData` in `Core/Source/SolidSyslog.c` intentionally assigns `instance.sd = configured` and `instance.sdCount = count` without a NULL guard. The correct follow-on fix for the `sd == NULL && sdCount > 0` inconsistency is to report a new `SOLIDSYSLOG_ERROR_MSG_CREATE_INCONSISTENT_SD` error rather than silently skipping the assignment. This is tracked under E12 alongside S12.06 (NULL guards for SD implementations). Do not flag the missing NULL guard in `InstallStructuredData` as a defect in the nil-object defaults PR (S12.04); it requires its own TDD red and error-message constant.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-05-13T22:05:44.151Z
Learning: In this codebase (solid-syslog), CLAUDE.md explicitly forbids adding error handling, fallbacks, or validation for scenarios that "can't happen". Internal vtable wrappers (e.g. in `Core/Source/SolidSyslogStore.c`) dereference function pointers unconditionally — a NULL vtable method is a wiring bug that should surface as an immediate crash, not be silently defaulted. Validation is only added at system boundaries (user input, external APIs).
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 352
File: Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp:175-182
Timestamp: 2026-05-13T22:05:29.329Z
Learning: In the `solid-syslog` project, the CLAUDE.md guideline explicitly discourages adding defensive error handling or validation for scenarios that cannot happen given the existing constraints. For example, test fixture helpers like `WriteMessage` in `Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp` intentionally omit payload-size guards because `payloadSize` is always derived from `SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100` (>= 412) or hardcoded values (e.g., 64), making the < 4 case unreachable. Do not suggest adding such guards in this codebase.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: CMakeLists.txt:94-100
Timestamp: 2026-04-21T05:30:34.467Z
Learning: In `Example/Threaded/main.c` of `DavidCozens/solid-syslog`, three `#ifdef SOLIDSYSLOG_HAVE_OPENSSL` blocks for TLS wiring are a known temporary stopgap. Issue `#171` (S3.13 under E3) will replace them with per-backend CMake-selected factory files (`ExampleTlsSender_OpenSsl.c` / `_WolfSsl.c` / `_Unavailable.c`) to keep application code free of build-flag knowledge and provide a natural extension point for wolfSSL. Do not flag these `#ifdef` blocks as needing immediate refactoring in future reviews of `main.c`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, the inconsistent null-check guard pattern in `Tests/SolidSyslogTlsStreamTest.cpp` (lines 161–187) is intentionally deferred to issue `#31` (E12: Error Handling), which covers a project-wide null-guard consistency sweep. Do not flag this pattern inconsistency in future reviews of `SolidSyslogTlsStreamTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 263
File: Platform/Posix/Source/SolidSyslogPosixMutex.c:24-27
Timestamp: 2026-05-04T14:12:11.201Z
Learning: In the solid-syslog project (DavidCozens/solid-syslog), error handling for _Create functions (e.g., SolidSyslogPosixMutex_Create returning NULL on pthread_mutex_init failure) is intentionally deferred to an error-handling epic (E12). The project has no error-reporting infrastructure yet, and _Create functions returning NULL would be inconsistent without callers uniformly handling it. This is a deliberate design decision, not an oversight.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-28T22:11:55.250Z
Learning: In `DavidCozens/solid-syslog`, Epic E07 (Standard Structured Data) tracks missing parameters on standard SDs (e.g., §6.3.4 origin `ip`/`enterpriseId` → issue `#75` S07.05; §6.3.5 meta `sysUpTime`/`language` → issue `#193` S07.06). Epic E14 (`#64`) is scoped exclusively to caller-built custom SD validation (e.g., SD-NAME syntax enforcement). Do not suggest filing standard SD parameter gaps under E14 in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: misra_suppressions.txt:27-27
Timestamp: 2026-05-18T05:43:07.289Z
Learning: In the solid-syslog codebase (`misra_suppressions.txt` and `docs/misra-deviations.md`), D.002 (Rules 11.2/11.3/11.5) covers two distinct cast patterns: (1) the vtable downcast (`struct SolidSyslogBuffer*` → `struct SolidSyslogCircularBuffer*` via `CircularBuffer_SelfFromBase`, and analogous `SelfFromBase` helpers in every other class) which is retained for all classes including those migrated under E11; and (2) the caller-storage `void*` cast which E11-migrated classes (currently only `SolidSyslogCircularBuffer`) no longer use. Do not flag the `SelfFromBase` vtable downcast as uncovered by D.002 — it is explicitly documented in `docs/misra-deviations.md` (updated in commit 9bd1d39 of PR `#394`). A potential future refactor to split D.002 into two separate blocks for the whole library was noted as out of scope for S11.01.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 342
File: DEVLOG.md:129-155
Timestamp: 2026-05-11T19:55:25.604Z
Learning: Do not edit historical DEVLOG entries; follow SKILL.md “Never rewrite history. Always append.” For future DEVLOG entries, avoid bare *N tokens; use backticks (e.g., `*48`) or explicit expressions instead.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 326
File: DEVLOG.md:0-0
Timestamp: 2026-05-11T07:09:35.151Z
Learning: In DEVLOG.md entry headings, always cite the squash-merge PR number in parentheses at the end of the title (e.g., "… (`#326`)"), matching prior entries.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Read `SKILL.md` at session start for cross-context conventions including DEVLOG cadence (append an entry after every meaningful session) and TDD pairing contract
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-25T22:33:52.849Z
Learning: In `DavidCozens/solid-syslog`, `CLAUDE.md` explicitly prohibits adding fields or abstractions for hypothetical future requirements ("Don't design for hypothetical future requirements. Three similar lines is better than a premature abstraction"). Do not suggest adding speculative tracking fields (e.g., `lastExpectedSeen`, `lastDesiredSeen`) to test fakes or other modules in the absence of a concrete failing test that requires them.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 103
File: Source/SolidSyslogFileStore.c:418-423
Timestamp: 2026-04-11T22:49:49.792Z
Learning: In `Source/SolidSyslogFileStore.c`, `DiscardOldestFile()` (around lines 418–423) ignores the return value of `SolidSyslogFile_Delete()` and unconditionally advances `instance.oldestSequence` regardless of whether the delete succeeded. This is intentional: guarding the delete result and only bumping `oldestSequence` on success is deferred to the error-handling epic, tracked in `project_robustness_backlog.md` under "FileStore error handling (PR `#103`)". Do not flag this as a bug in future reviews of this function.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 205
File: Core/Source/SolidSyslogFormatter.c:248-274
Timestamp: 2026-04-24T19:21:31.563Z
Learning: In `Core/Source/SolidSyslogFormatter.c` (`DavidCozens/solid-syslog`), `SolidSyslogFormatter_EscapedString` has a known deferred defect: when the output buffer has exactly 1 byte of remaining capacity and the current source byte needs escaping (`"`, `\`, `]`), `ESCAPE_PREFIX` is written but the escaped character is silently clamped, leaving a dangling `\` at the buffer tail. `AsFormattedBuffer`'s `TrimTruncatedMultiByteTail` does not cover this case because `\` is valid ASCII. The practical impact is zero for all in-tree callers because they size via `SOLIDSYSLOG_ESCAPED_MAX_SIZE(rawMax)` = `2 * rawMax`, which always provides sufficient headroom. The agreed fix (Option 1) is a targeted output-capacity check before the escape+codepoint pair write (add `OutputHasCapacityFor(formatter, outputBytes)` guard; break on failure). Do not re-flag this as a new issue in future reviews until the targeted fix is merged.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag the missing NULL checks in StreamSender_Create as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag missing NULL checks in `StreamSender_Create` as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 411
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosStaticResolverStatic.c:28-35
Timestamp: 2026-05-20T12:24:57.101Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogFreeRtosStaticResolver_Create(const uint8_t ipv4Octets[4])` must NOT have a NULL guard on `ipv4Octets`. This is integrator setup wiring: the integrator owns the buffer (e.g., `static const uint8_t dest[] = {…}`) and NULL is not in the contract. The pre-migration `_Create` also had no NULL check. Omitting the guard is consistent with project policy (CLAUDE.md: only validate at true system boundaries) and with equivalent adapter classes in the same migration story (e.g., PosixMessageQueueBuffer, FreeRtosMutex, FreeRtosDatagram).
Resolve the cppcheck-misra fallout from the codes-only change: - Add a static inline Xxx_Report(severity, uint16_t category, enum SolidSyslog<Class>Errors code) to each class's Private.h; route all 134 emit sites through it. The typed param keeps each per-class error enum tag referenced (deleting the message tables had orphaned them -> MISRA 2.4), static inline avoids 8.7, and the uncast enum->int32_t body (value-preserving, no 10.3) drops the (int32_t) cast from every call site. Supersedes the crypto-only _Report unwind. - Renumber the suppressions shifted by the relocation + emit-site collapse (misra_renumber.py --apply + 8 manual Address-Private.h 11.3 fixes). - Extract CHECK_CATEGORY_TEXT macro in BddTargetErrorTextTest (CodeRabbit). cppcheck-misra full Core+Platform scope exit 0; tidy + format clean; freertos-host all suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1428 passed) 🚧 Error MessagesCreated by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
clang -Wsign-conversion treats the unscoped error enums' underlying type as unsigned, so the enum->int32_t conversion must stay explicit. Move the cast from the (removed) call sites into the one wrapper body per class — call sites stay cast-free, 2.4 still fixed by the typed param. clang/gcc/cppcheck green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1428 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h`:
- Around line 39-46: The header is missing an extern declaration for the error
source referenced by LwipRawAddress_Report; add an extern declaration for
LwipRawAddressErrorSource (matching its type used by SolidSyslog_Error) before
the static inline function LwipRawAddress_Report so the symbol resolves when the
header is included from multiple translation units; ensure the declaration uses
the exact identifier LwipRawAddressErrorSource and the same type expected by
SolidSyslog_Error.
In `@Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h`:
- Around line 38-45: The header defines PosixMessageQueueBuffer_Report which
references PosixMessageQueueBufferErrorSource but no extern declaration exists;
add an extern declaration for PosixMessageQueueBufferErrorSource in the header
(above PosixMessageQueueBuffer_Report) so translation units including this
header can link to the definition (e.g., the definition in
SolidSyslogPosixMessageQueueBufferStatic.c); ensure the symbol name is exactly
PosixMessageQueueBufferErrorSource and its type matches the definition used by
SolidSyslog_Error.
In `@Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h`:
- Around line 22-29: The header's inline helper PosixMutex_Report references
PosixMutexErrorSource but the header lacks an extern declaration; add an extern
declaration for PosixMutexErrorSource (matching its definition in
SolidSyslogPosixMutexStatic.c) above the PosixMutex_Report function so the
symbol is visible to translation units that include this header.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea398f1f-e30c-4243-a9ba-19753b0b3259
📒 Files selected for processing (99)
Core/Source/SolidSyslog.cCore/Source/SolidSyslogBlockStorePrivate.hCore/Source/SolidSyslogBlockStoreStatic.cCore/Source/SolidSyslogCircularBufferPrivate.hCore/Source/SolidSyslogCircularBufferStatic.cCore/Source/SolidSyslogFileBlockDevicePrivate.hCore/Source/SolidSyslogFileBlockDeviceStatic.cCore/Source/SolidSyslogMetaSdPrivate.hCore/Source/SolidSyslogMetaSdStatic.cCore/Source/SolidSyslogOriginSdPrivate.hCore/Source/SolidSyslogOriginSdStatic.cCore/Source/SolidSyslogPassthroughBufferPrivate.hCore/Source/SolidSyslogPassthroughBufferStatic.cCore/Source/SolidSyslogPrivate.hCore/Source/SolidSyslogStatic.cCore/Source/SolidSyslogStreamSenderPrivate.hCore/Source/SolidSyslogStreamSenderStatic.cCore/Source/SolidSyslogSwitchingSenderPrivate.hCore/Source/SolidSyslogSwitchingSenderStatic.cCore/Source/SolidSyslogTimeQualitySdPrivate.hCore/Source/SolidSyslogTimeQualitySdStatic.cCore/Source/SolidSyslogUdpSender.cCore/Source/SolidSyslogUdpSenderPrivate.hCore/Source/SolidSyslogUdpSenderStatic.cDEVLOG.mdPlatform/Atomics/Source/SolidSyslogStdAtomicCounterPrivate.hPlatform/Atomics/Source/SolidSyslogStdAtomicCounterStatic.cPlatform/FatFs/Source/SolidSyslogFatFsFilePrivate.hPlatform/FatFs/Source/SolidSyslogFatFsFileStatic.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutexPrivate.hPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutexStatic.cPlatform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDnsResolver.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverStatic.cPlatform/LwipRaw/Source/SolidSyslogLwipRawResolverPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawResolverStatic.cPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyStatic.cPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyStatic.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStream.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsStreamStatic.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicy.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyPrivate.hPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyStatic.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyPrivate.hPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyStatic.cPlatform/OpenSsl/Source/SolidSyslogTlsStream.cPlatform/OpenSsl/Source/SolidSyslogTlsStreamPrivate.hPlatform/OpenSsl/Source/SolidSyslogTlsStreamStatic.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.hPlatform/PlusTcp/Source/SolidSyslogPlusTcpAddressStatic.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpDatagramPrivate.hPlatform/PlusTcp/Source/SolidSyslogPlusTcpDatagramStatic.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpResolverPrivate.hPlatform/PlusTcp/Source/SolidSyslogPlusTcpResolverStatic.cPlatform/PlusTcp/Source/SolidSyslogPlusTcpTcpStreamPrivate.hPlatform/PlusTcp/Source/SolidSyslogPlusTcpTcpStreamStatic.cPlatform/Posix/Source/SolidSyslogGetAddrInfoResolverPrivate.hPlatform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.cPlatform/Posix/Source/SolidSyslogPosixAddressPrivate.hPlatform/Posix/Source/SolidSyslogPosixAddressStatic.cPlatform/Posix/Source/SolidSyslogPosixDatagramPrivate.hPlatform/Posix/Source/SolidSyslogPosixDatagramStatic.cPlatform/Posix/Source/SolidSyslogPosixFilePrivate.hPlatform/Posix/Source/SolidSyslogPosixFileStatic.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.hPlatform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.cPlatform/Posix/Source/SolidSyslogPosixMutexPrivate.hPlatform/Posix/Source/SolidSyslogPosixMutexStatic.cPlatform/Posix/Source/SolidSyslogPosixTcpStreamPrivate.hPlatform/Posix/Source/SolidSyslogPosixTcpStreamStatic.cPlatform/Windows/Source/SolidSyslogWindowsAtomicCounterPrivate.hPlatform/Windows/Source/SolidSyslogWindowsAtomicCounterStatic.cPlatform/Windows/Source/SolidSyslogWindowsFilePrivate.hPlatform/Windows/Source/SolidSyslogWindowsFileStatic.cPlatform/Windows/Source/SolidSyslogWindowsMutexPrivate.hPlatform/Windows/Source/SolidSyslogWindowsMutexStatic.cPlatform/Windows/Source/SolidSyslogWinsockAddressPrivate.hPlatform/Windows/Source/SolidSyslogWinsockAddressStatic.cPlatform/Windows/Source/SolidSyslogWinsockDatagramPrivate.hPlatform/Windows/Source/SolidSyslogWinsockDatagramStatic.cPlatform/Windows/Source/SolidSyslogWinsockResolverPrivate.hPlatform/Windows/Source/SolidSyslogWinsockResolverStatic.cPlatform/Windows/Source/SolidSyslogWinsockTcpStreamPrivate.hPlatform/Windows/Source/SolidSyslogWinsockTcpStreamStatic.cTests/Bdd/Targets/BddTargetErrorTextTest.cppmisra_suppressions.txt
✅ Files skipped from review due to trivial changes (8)
- Platform/Windows/Source/SolidSyslogWindowsFilePrivate.h
- Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyStatic.c
- Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyStatic.c
- Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyStatic.c
- Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.c
- Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyStatic.c
- Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.c
- DEVLOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Core/Source/SolidSyslogUdpSender.c
- Tests/Bdd/Targets/BddTargetErrorTextTest.cpp
Comment on lines
+38
to
+45
| static inline void PosixMessageQueueBuffer_Report( | ||
| enum SolidSyslogSeverity severity, | ||
| uint16_t category, | ||
| enum SolidSyslogPosixMessageQueueBufferErrors code | ||
| ) | ||
| { | ||
| SolidSyslog_Error(severity, &PosixMessageQueueBufferErrorSource, category, (int32_t) code); | ||
| } |
Contributor
There was a problem hiding this comment.
Add missing extern declaration for the error source.
The inline helper references PosixMessageQueueBufferErrorSource (line 44), but the header lacks an extern declaration. Translation units that include this header but do not define the source (e.g., SolidSyslogPosixMessageQueueBufferStatic.c) will fail to compile.
🔧 Add extern declaration
Insert before the helper function:
+extern const struct SolidSyslogErrorSource PosixMessageQueueBufferErrorSource;
+
static inline void PosixMessageQueueBuffer_Report(
enum SolidSyslogSeverity severity,
uint16_t category,
enum SolidSyslogPosixMessageQueueBufferErrors code
)🤖 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 `@Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h` around
lines 38 - 45, The header defines PosixMessageQueueBuffer_Report which
references PosixMessageQueueBufferErrorSource but no extern declaration exists;
add an extern declaration for PosixMessageQueueBufferErrorSource in the header
(above PosixMessageQueueBuffer_Report) so translation units including this
header can link to the definition (e.g., the definition in
SolidSyslogPosixMessageQueueBufferStatic.c); ensure the symbol name is exactly
PosixMessageQueueBufferErrorSource and its type matches the definition used by
SolidSyslog_Error.
Owner
Author
There was a problem hiding this comment.
False positive — declining. The header includes SolidSyslogPosixMessageQueueBufferErrors.h (line 13), which declares extern const struct SolidSyslogErrorSource PosixMessageQueueBufferErrorSource; (Errors.h line 20). In scope, and the build compiles clean across clang/gcc/freertos-host.
Contributor
There was a problem hiding this comment.
@DavidCozens, you're right — the extern declaration comes in via the SolidSyslogPosixMessageQueueBufferErrors.h include, so the symbol is fully in scope. My mistake, thanks for the clear explanation.
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 514
File: Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferPrivate.h:38-45
Timestamp: 2026-06-03T13:25:00.916Z
Learning: In DavidCozens/solid-syslog, the *Private.h inline report helpers (e.g. `PosixMessageQueueBuffer_Report`, `LwipRawDatagram_Report`, etc.) reference an `ErrorSource` symbol that is declared `extern` inside the corresponding *Errors.h header (e.g. `SolidSyslogPosixMessageQueueBufferErrors.h` line 20: `extern const struct SolidSyslogErrorSource PosixMessageQueueBufferErrorSource;`). Because each *Private.h already `#include`s its *Errors.h, the extern declaration is always in scope and no additional forward declaration is needed. Do not flag missing extern declarations for ErrorSource symbols in *Private.h report helpers.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 416
File: DEVLOG.md:155-155
Timestamp: 2026-05-20T17:56:11.873Z
Learning: There are two distinct headers: Core/Interface/SolidSyslogPrival.h (public facility/severity enums) and Core/Source/SolidSyslogPrivate.h (TU-internal struct SolidSyslog). SolidSyslogStatic.c includes both; the DEVLOG IWYU note referred specifically to adding the Prival include for the enum used at SolidSyslog_Error call sites.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 458
File: Tests/Lwip/SolidSyslogLwipRawResolverTest.cpp:236-244
Timestamp: 2026-05-26T09:51:07.851Z
Learning: In the solid-syslog codebase (per CLAUDE.md Design Patterns), the CHECK_REPORTED(severity, source, code) macro is the established pattern for asserting ErrorHandlerFake payloads in CppUTest tests. It wraps CALLED_FAKE(ErrorHandlerFake_Handle, ONCE), LONGS_EQUAL(severity, ErrorHandlerFake_LastSeverity()), POINTERS_EQUAL(source, ErrorHandlerFake_LastSource()), and UNSIGNED_LONGS_EQUAL(code, ErrorHandlerFake_LastCode()) in a do { } while(0) block, with NOLINTBEGIN/NOLINTEND suppressions for cppcoreguidelines-macro-usage and cppcoreguidelines-avoid-do-while. It is applied to all ErrorHandlerFake assertion sites (e.g., ExhaustedCreate, DestroyOfUnknownHandle, DestroyOfStaleHandle) across LwipRaw and other test files.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: Core/Source/SolidSyslogCircularBufferStatic.c:116-127
Timestamp: 2026-05-18T05:42:39.906Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogCircularBuffer_Destroy` (Core/Source/SolidSyslogCircularBufferStatic.c) intentionally emits `SOLIDSYSLOG_SEVERITY_WARNING` / `SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY` for ALL handles not currently issued by `_Create`, including pool-exhaustion Fallback handles. This is the designed E11 reference contract: one uniform rule covers Fallback handles, double-Destroy, and stranger handles — no Fallback short-circuit. Do not flag this as a bug.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 400
File: Core/Source/SolidSyslogTimeQualitySdStatic.c:0-0
Timestamp: 2026-05-18T17:07:41.514Z
Learning: In the DavidCozens/solid-syslog codebase, when a `_Create` function receives a required function-pointer argument as NULL (e.g., `getTimeQuality` in `SolidSyslogTimeQualitySd_Create`), the correct severity is `SOLIDSYSLOG_SEVERITY_ERROR` (not WARNING), because a NULL required callback is a programmer contract violation equivalent to UdpSender bad-config errors (missing Resolver/Datagram/Endpoint). The function should log the error and return `SolidSyslogNullSd_Get()` (or the appropriate null fallback), without allocating from the pool. This convention is documented in `feedback_bad_setup_contract.md`.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Applies to Core/Source/**/*.c : Pool exhaustion falls back to a shared null sibling (e.g., `SolidSyslogNullSender`, `SolidSyslogNullBuffer`); caller code keeps running; integrator sees `SolidSyslog_Error` if handler installed
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-05-13T22:05:44.151Z
Learning: In this codebase (solid-syslog), CLAUDE.md explicitly forbids adding error handling, fallbacks, or validation for scenarios that "can't happen". Internal vtable wrappers (e.g. in `Core/Source/SolidSyslogStore.c`) dereference function pointers unconditionally — a NULL vtable method is a wiring bug that should surface as an immediate crash, not be silently defaulted. Validation is only added at system boundaries (user input, external APIs).
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: Interface/SolidSyslogStream.h:0-0
Timestamp: 2026-04-21T05:30:24.982Z
Learning: In `Interface/SolidSyslogStream.h` of `DavidCozens/solid-syslog`, `ssize_t` (POSIX-only) is intentionally NOT used in the public Stream interface. Instead, a project-owned `typedef intptr_t SolidSyslogSsize` is declared in `Interface/SolidSyslogStream.h` and used as the return type of `SolidSyslogStream_Read`. Internal Platform/Posix implementations use POSIX `ssize_t` with a cast at the API boundary. This keeps the public API free of platform-specific types and is consistent with the project's naming conventions. Do not suggest replacing `SolidSyslogSsize` with `ssize_t` or adding MSVC shims in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 402
File: Core/Source/SolidSyslogStreamSenderStatic.c:42-51
Timestamp: 2026-05-19T06:57:28.948Z
Learning: In DavidCozens/solid-syslog, the WARNING emitted by `SolidSyslogStreamSender_Destroy` (and `SolidSyslogFileBlockDevice_Destroy`) when called with a handle not issued by the pool (including the null-object fallback returned on pool exhaustion) is intentional and must NOT be short-circuited. The Create ERROR (pool exhaustion) and Destroy WARNING together form a correlated two-sided diagnostic for pool misconfiguration. The null-object vtable ensures crash safety; the warning ensures the misconfiguration is visible from both sides of the lifecycle. Do not suggest silencing either signal.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Applies to Core/{Interface,Source}/**/*.{c,h} : Public C functions use pattern `SolidSyslogClass_Function`; public types use `SolidSyslogClass`; public macros use `SOLIDSYSLOG_SCREAMING_SNAKE`
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 263
File: Platform/Posix/Source/SolidSyslogPosixMutex.c:24-27
Timestamp: 2026-05-04T14:12:11.201Z
Learning: In the solid-syslog project (DavidCozens/solid-syslog), error handling for _Create functions (e.g., SolidSyslogPosixMutex_Create returning NULL on pthread_mutex_init failure) is intentionally deferred to an error-handling epic (E12). The project has no error-reporting infrastructure yet, and _Create functions returning NULL would be inconsistent without callers uniformly handling it. This is a deliberate design decision, not an oversight.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-17T19:45:13.677Z
Learning: In `DavidCozens/solid-syslog`, the extern function-pointer seam variables in `Platform/Windows/Source/SolidSyslogWinsockDatagramInternal.h` and `Platform/Windows/Source/SolidSyslogWinsockResolverInternal.h` (`Winsock_socket`, `Winsock_sendto`, `Winsock_closesocket`, `Winsock_getaddrinfo`, `Winsock_freeaddrinfo`) intentionally use underscore-based naming (mirroring the real Winsock API names) to preserve visual clarity at `UT_PTR_SET` call sites. Do not flag these names as non-camelCase in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the `_CRT_SECURE_NO_WARNINGS` compile definition is a known temporary stopgap for MSVC CRT deprecation warnings in test fakes. Issue `#126` (S13.3) will introduce a `SafeString` abstraction (Windows impl wraps `_s` variants; POSIX falls through to standard functions); once test fakes migrate, this suppression is removed. Production code is already SDL-clean. Do not flag this definition as an unjustified suppression in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 113
File: Interface/SolidSyslogFormatter.h:24-27
Timestamp: 2026-04-13T22:11:31.767Z
Learning: In the `DavidCozens/solid-syslog` repository, `static inline` functions are permitted in `Interface/` public headers. The "no implementation in `Interface/`" guideline applies only to implementation files (`.c`); `static inline` helpers (e.g. zero-cost casts like `SolidSyslogFormatter_FromStorage`) in interface headers are acceptable and intentional. Do not flag `static inline` functions in `Interface/*.h` as violating the separation-of-concerns rule.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the MSVC suppression `/wd4297` (exception thrown from `extern "C"` function) is intentional and permanent test-infrastructure design: C fakes require C linkage, and `TestAssert_Fail` throws to propagate failures through CppUTest. Issue `#125` will add an inline comment documenting this justification. Do not flag `/wd4297` as an unjustified suppression in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 205
File: Core/Source/SolidSyslogFormatter.c:248-274
Timestamp: 2026-04-24T19:21:31.563Z
Learning: In `Core/Source/SolidSyslogFormatter.c` (`DavidCozens/solid-syslog`), `SolidSyslogFormatter_EscapedString` has a known deferred defect: when the output buffer has exactly 1 byte of remaining capacity and the current source byte needs escaping (`"`, `\`, `]`), `ESCAPE_PREFIX` is written but the escaped character is silently clamped, leaving a dangling `\` at the buffer tail. `AsFormattedBuffer`'s `TrimTruncatedMultiByteTail` does not cover this case because `\` is valid ASCII. The practical impact is zero for all in-tree callers because they size via `SOLIDSYSLOG_ESCAPED_MAX_SIZE(rawMax)` = `2 * rawMax`, which always provides sufficient headroom. The agreed fix (Option 1) is a targeted output-capacity check before the escape+codepoint pair write (add `OutputHasCapacityFor(formatter, outputBytes)` guard; break on failure). Do not re-flag this as a new issue in future reviews until the targeted fix is merged.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 103
File: Source/SolidSyslogFileStore.c:124-128
Timestamp: 2026-04-11T22:49:29.830Z
Learning: In `Source/SolidSyslogFileStore.c`, the return values of `OpenWriteFile()` and `OpenReadFile()` are intentionally unchecked in `SolidSyslogFileStore_Create` (around lines 124–128) and `RotateToNextFile` (around lines 394–400). Guarding against open failures and aborting the create/rotation flow is deferred to the error-handling epic, tracked in `project_robustness_backlog.md` under "FileStore error handling (PR `#103`)". Do not flag missing open-failure guards in these functions in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 137
File: Interface/SolidSyslogResolverDefinition.h:12-12
Timestamp: 2026-04-15T21:12:35.030Z
Learning: In `Interface/SolidSyslogResolverDefinition.h` (and the corresponding `Source/SolidSyslogResolver.c`), the `Resolve` function pointer returns `void` intentionally. This is a direct extraction of the pre-existing behaviour in `SolidSyslogUdpSender` and `SolidSyslogTcpSender`, neither of which checked resolution success before PR `#137`. Adding a boolean/error return to `Resolve` and propagating failures through sender create/connect paths is deferred to the error-handling epic (Epic `#31`). Do not flag the `void` return on `SolidSyslogResolver.Resolve` as a contract gap in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, the inconsistent null-check guard pattern in `Tests/SolidSyslogTlsStreamTest.cpp` (lines 161–187) is intentionally deferred to issue `#31` (E12: Error Handling), which covers a project-wide null-guard consistency sweep. Do not flag this pattern inconsistency in future reviews of `SolidSyslogTlsStreamTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Tests/StreamFakeTest.cpp:0-0
Timestamp: 2026-05-22T12:15:29.505Z
Learning: In the solid-syslog codebase (DavidCozens/solid-syslog), the project deliberately avoids conditional compilation (`#ifdef`) in both production and test code. When a test needs a `struct SolidSyslogAddress*` purely as a pass-through token (i.e., the code under test never dereferences or inspects its platform-specific sockaddr contents), use the cross-platform `Tests/AddressFake.{h,c}` helper (`AddressFake_Get()` returns an opaque non-null singleton). Only tests that actually inspect the platform sockaddr internals (datagram / TCP-stream / resolver tests) should use the per-platform `SolidSyslog{Posix,Winsock,FreeRtos}Address_Create` + corresponding `Private.h` accessor APIs.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 352
File: Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp:175-182
Timestamp: 2026-05-13T22:05:29.329Z
Learning: In the `solid-syslog` project, the CLAUDE.md guideline explicitly discourages adding defensive error handling or validation for scenarios that cannot happen given the existing constraints. For example, test fixture helpers like `WriteMessage` in `Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp` intentionally omit payload-size guards because `payloadSize` is always derived from `SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100` (>= 412) or hardcoded values (e.g., 64), making the < 4 case unreachable. Do not suggest adding such guards in this codebase.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: Tests/CMakeLists.txt:0-0
Timestamp: 2026-04-21T05:30:25.599Z
Learning: In `DavidCozens/solid-syslog`, `Tests/Support/OpenSslFake.c` is compiled into the `PosixFakes` static library via `POSIX_FAKES_SOURCES` in `Tests/Support/CMakeLists.txt` and is linked into the test binary via `target_link_libraries(${PROJECT_NAME}Tests PRIVATE PosixFakes)`. It does NOT need to appear directly in `TEST_SOURCES`. As of commit 790da74 it is also conditionally included only when `SOLIDSYSLOG_OPENSSL=ON`. Do not flag `OpenSslFake.c` as missing from `TEST_SOURCES` in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 137
File: Source/SolidSyslogUdpSender.c:0-0
Timestamp: 2026-04-15T21:13:00.238Z
Learning: In `Source/SolidSyslogUdpSender.c`, NULL checks for `config` and `config->resolver` in `SolidSyslogUdpSender_Create` are intentionally deferred to the error-handling epic (Epic `#31`). The function currently calls `SolidSyslogResolver_Resolve()` dispatch (not the vtable directly) and includes `SolidSyslogResolver.h`. Do not flag missing NULL guards on config/resolver in this function in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 502
File: DEVLOG.md:0-0
Timestamp: 2026-06-01T10:52:21.000Z
Learning: For PR `#502` / S12.21 in `solid-syslog`, Structured Data and MSG-body tests intentionally remain in `Tests/SolidSyslogTest.cpp` as integration coverage of the full `SolidSyslog_Log` pipeline rather than being migrated to `Tests/SolidSyslogMessageFormatterTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 113
File: Interface/SolidSyslogFormatter.h:11-11
Timestamp: 2026-04-13T22:11:36.013Z
Learning: In `Interface/SolidSyslogFormatter.h`, `SolidSyslogFormatterStorage` is intentionally typedef'd as `size_t` (not `unsigned char`) to guarantee alignment matching `struct SolidSyslogFormatter`'s leading `size_t` fields followed by a flexible `char[]` array. The cast from `SolidSyslogFormatterStorage*` to `struct SolidSyslogFormatter*` is the standard C opaque-handle/caller-owned-storage pattern; the `_Static_assert` in `Source/SolidSyslogFormatter.c` guards layout compatibility. Do not flag this typedef or the associated cast as a strict-aliasing violation or suggest switching to `unsigned char` in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 400
File: Core/Source/SolidSyslogOriginSdStatic.c:11-11
Timestamp: 2026-05-18T17:07:19.499Z
Learning: In this repo (DavidCozens/solid-syslog), an IWYU CI gate enforces strict direct-header inclusion: for every symbol (types, enums, functions, etc.) that is directly referenced in a translation unit’s `.c` or `.h` file, include the specific header that declares that symbol. Do not rely on transitive includes from other headers.
Example: if a `.c`/`.h` file uses `SolidSyslogSeverity` enums, it must directly `#include` `SolidSyslogPrival.h` even if another included header (e.g., `SolidSyslogError.h`) pulls it in transitively.
When reviewing, do not flag or recommend removing includes solely because they are “transitive” in this codebase—removing them may still compile locally but can fail the IWYU CI check.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the MSVC suppression `/wd4200` (flexible array member) is a known stopgap. Issue `#125` (S13.2) will replace `char buffer[]` with the `char buffer[1]` pre-C99 idiom and remove this suppression. Do not flag `/wd4200` in future reviews of this file.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 411
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosStaticResolverStatic.c:28-35
Timestamp: 2026-05-20T12:24:57.101Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogFreeRtosStaticResolver_Create(const uint8_t ipv4Octets[4])` must NOT have a NULL guard on `ipv4Octets`. This is integrator setup wiring: the integrator owns the buffer (e.g., `static const uint8_t dest[] = {…}`) and NULL is not in the contract. The pre-migration `_Create` also had no NULL check. Omitting the guard is consistent with project policy (CLAUDE.md: only validate at true system boundaries) and with equivalent adapter classes in the same migration story (e.g., PosixMessageQueueBuffer, FreeRtosMutex, FreeRtosDatagram).
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-22T18:16:45.920Z
Learning: In `DavidCozens/solid-syslog`, all headers under `Example/` use the **unpunctuated** include guard form (e.g. `EXAMPLEAPPNAME_H`, `EXAMPLEMTLSCONFIG_H`, `EXAMPLESWITCHCONFIG_H`, `EXAMPLETLSSENDER_H`). Do not suggest renaming these guards to the underscore-separated form (e.g. `EXAMPLE_TLS_SENDER_H`) in future reviews of files under `Example/`. `Core/Interface/` has a mix of both spellings and neither is the single project-wide convention; any normalisation across directories should be a dedicated follow-up pass, not a per-file change.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: misra_suppressions.txt:27-27
Timestamp: 2026-05-18T05:43:07.289Z
Learning: In the solid-syslog codebase (`misra_suppressions.txt` and `docs/misra-deviations.md`), D.002 (Rules 11.2/11.3/11.5) covers two distinct cast patterns: (1) the vtable downcast (`struct SolidSyslogBuffer*` → `struct SolidSyslogCircularBuffer*` via `CircularBuffer_SelfFromBase`, and analogous `SelfFromBase` helpers in every other class) which is retained for all classes including those migrated under E11; and (2) the caller-storage `void*` cast which E11-migrated classes (currently only `SolidSyslogCircularBuffer`) no longer use. Do not flag the `SelfFromBase` vtable downcast as uncovered by D.002 — it is explicitly documented in `docs/misra-deviations.md` (updated in commit 9bd1d39 of PR `#394`). A potential future refactor to split D.002 into two separate blocks for the whole library was noted as out of scope for S11.01.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 513
File: Bdd/Targets/Common/BddTargetStderrErrorHandler.c:19-22
Timestamp: 2026-06-02T22:57:23.006Z
Learning: In `Bdd/Targets/Common/BddTargetStderrErrorHandler.c` (Tier-3 BDD diagnostic code), the cast `source->AsString((uint8_t) event->Detail)` is intentionally left without a prior range guard in S12.25. The rationale: in S12.25 `event->Detail` is always a per-class `enum SolidSyslog<Class>Errors` value (values well under 256), and `AsString` itself takes `uint8_t` and returns `"unknown"` for out-of-range inputs. The explicit range check `(event->Detail >= 0 && event->Detail <= UINT8_MAX)` before the cast, together with the `AsString` signature change to accept wider native codes (errno, X509_V_ERR_*), is deferred to sibling story S12.26. Do not flag this cast as a truncation risk until S12.26 lands.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 327
File: Core/Source/SolidSyslog.c:220-224
Timestamp: 2026-05-11T10:23:17.520Z
Learning: In DavidCozens/solid-syslog, `InstallStructuredData` in `Core/Source/SolidSyslog.c` intentionally assigns `instance.sd = configured` and `instance.sdCount = count` without a NULL guard. The correct follow-on fix for the `sd == NULL && sdCount > 0` inconsistency is to report a new `SOLIDSYSLOG_ERROR_MSG_CREATE_INCONSISTENT_SD` error rather than silently skipping the assignment. This is tracked under E12 alongside S12.06 (NULL guards for SD implementations). Do not flag the missing NULL guard in `InstallStructuredData` as a defect in the nil-object defaults PR (S12.04); it requires its own TDD red and error-message constant.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: CMakeLists.txt:94-100
Timestamp: 2026-04-21T05:30:34.467Z
Learning: In `Example/Threaded/main.c` of `DavidCozens/solid-syslog`, three `#ifdef SOLIDSYSLOG_HAVE_OPENSSL` blocks for TLS wiring are a known temporary stopgap. Issue `#171` (S3.13 under E3) will replace them with per-backend CMake-selected factory files (`ExampleTlsSender_OpenSsl.c` / `_WolfSsl.c` / `_Unavailable.c`) to keep application code free of build-flag knowledge and provide a natural extension point for wolfSSL. Do not flag these `#ifdef` blocks as needing immediate refactoring in future reviews of `main.c`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag the missing NULL checks in StreamSender_Create as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag missing NULL checks in `StreamSender_Create` as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 283
File: Platform/OpenSsl/Interface/SolidSyslogTlsStream.h:7-7
Timestamp: 2026-05-07T08:22:53.110Z
Learning: In solid-syslog, the CLAUDE.md callback rule that requires a `void* context` parameter (and the paired config field) should be applied only to callbacks that need to carry real per-instance state. Exempt callbacks that are stateless thin wrappers over OS primitives—i.e., the callback is effectively a pure function of its input parameters with no instance state to preserve—so adding `void* context` would be pointless (the context would always be NULL). For example, platform implementations like `SolidSyslogSleepFunction` that wrap `nanosleep`/`Sleep` and are defined purely in terms of `int milliseconds` should not add `void* context` or the paired config field. For callbacks that do depend on per-instance data, include `void* context` and the paired config field as required by CLAUDE.md.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/Common/ExampleInteractive.h:14-17
Timestamp: 2026-05-09T15:56:13.853Z
Learning: During code review, ensure solid-syslog Tier 1/2 library callbacks in Core/ and Platform/ follow the CLAUDE.md “Callback Conventions”: the callback API must take a `void*` context parameter and the implementation must use the paired configuration field associated with that callback context. Do not apply this requirement to Tier 3 example handlers (e.g., Example/Common/, Example/FreeRtos/, such as `ExampleInteractiveSwitchHandler`/`ExampleInteractiveSetHandler`), which are allowed to use file-static globals per the documented example-tier pattern. If a multi-instance use-case arises, migrate Switch and Set together in a single cohesive change rather than piecemeal updates.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 419
File: Platform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.h:17-18
Timestamp: 2026-05-21T12:53:11.114Z
Learning: When reviewing code in adapter-specific TU-private headers matching *Private.h, accept the established two-tier C naming convention:
- Use `SolidSyslogClass_Function` (full `SolidSyslog...` prefix) only for the public integrator-facing API declared in the public Interface header.
- In `*Private.h`, allow `Class_Function` (bare/short prefix) names for private cross-translation-unit helper functions shared between the two `.c` files of a single adapter.
These `Class_Function` helpers are declared in the TU-private `*Private.h` and intentionally have external linkage (so they must not be `static`) because they are referenced across translation units; therefore reviewers should NOT flag `Class_Function`-style names in `*Private.h` headers as naming-convention violations. Follow the precedent shown in `Platform/OpenSsl/Source/SolidSyslogTlsStreamPrivate.h`.
Comment on lines
+22
to
+29
| static inline void PosixMutex_Report( | ||
| enum SolidSyslogSeverity severity, | ||
| uint16_t category, | ||
| enum SolidSyslogPosixMutexErrors code | ||
| ) | ||
| { | ||
| SolidSyslog_Error(severity, &PosixMutexErrorSource, category, (int32_t) code); | ||
| } |
Contributor
There was a problem hiding this comment.
Add missing extern declaration for the error source.
The inline helper references PosixMutexErrorSource (line 28), but the header does not declare it. When this header is included from SolidSyslogPosixMutexStatic.c, the compiler will not be able to resolve the symbol.
🔧 Add extern declaration
Insert before the helper function:
+extern const struct SolidSyslogErrorSource PosixMutexErrorSource;
+
static inline void PosixMutex_Report(
enum SolidSyslogSeverity severity,
uint16_t category,
enum SolidSyslogPosixMutexErrors code
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static inline void PosixMutex_Report( | |
| enum SolidSyslogSeverity severity, | |
| uint16_t category, | |
| enum SolidSyslogPosixMutexErrors code | |
| ) | |
| { | |
| SolidSyslog_Error(severity, &PosixMutexErrorSource, category, (int32_t) code); | |
| } | |
| extern const struct SolidSyslogErrorSource PosixMutexErrorSource; | |
| static inline void PosixMutex_Report( | |
| enum SolidSyslogSeverity severity, | |
| uint16_t category, | |
| enum SolidSyslogPosixMutexErrors code | |
| ) | |
| { | |
| SolidSyslog_Error(severity, &PosixMutexErrorSource, category, (int32_t) code); | |
| } |
🤖 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 `@Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h` around lines 22 - 29,
The header's inline helper PosixMutex_Report references PosixMutexErrorSource
but the header lacks an extern declaration; add an extern declaration for
PosixMutexErrorSource (matching its definition in SolidSyslogPosixMutexStatic.c)
above the PosixMutex_Report function so the symbol is visible to translation
units that include this header.
Owner
Author
There was a problem hiding this comment.
False positive — declining. The header includes SolidSyslogPosixMutexErrors.h (line 10), which declares extern const struct SolidSyslogErrorSource PosixMutexErrorSource; (Errors.h line 17). In scope, and the build compiles clean across clang/gcc/freertos-host.
Contributor
There was a problem hiding this comment.
@DavidCozens, you're right — apologies for the noise. SolidSyslogPosixMutexErrors.h already carries the extern const struct SolidSyslogErrorSource PosixMutexErrorSource; declaration, so including it in the *Private.h brings everything into scope. My concern was unfounded.
I'll record this pattern to avoid flagging it again in future reviews.
(ᵕ—ᴗ—)
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 514
File: Platform/Posix/Source/SolidSyslogPosixMutexPrivate.h:22-29
Timestamp: 2026-06-03T13:25:06.948Z
Learning: In DavidCozens/solid-syslog, each per-class `*Errors.h` header (e.g. `SolidSyslogPosixMutexErrors.h`) declares both the error-code enum and the matching `extern const struct SolidSyslogErrorSource *ErrorSource;` for that class (e.g. `extern const struct SolidSyslogErrorSource PosixMutexErrorSource;`). The `*Private.h` headers include `*Errors.h`, so inline `*_Report` helpers that reference `*ErrorSource` already have the extern declaration in scope. Do not flag missing extern declarations for `*ErrorSource` in `*Private.h` files that include the corresponding `*Errors.h`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 416
File: DEVLOG.md:155-155
Timestamp: 2026-05-20T17:56:11.873Z
Learning: There are two distinct headers: Core/Interface/SolidSyslogPrival.h (public facility/severity enums) and Core/Source/SolidSyslogPrivate.h (TU-internal struct SolidSyslog). SolidSyslogStatic.c includes both; the DEVLOG IWYU note referred specifically to adding the Prival include for the enum used at SolidSyslog_Error call sites.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 263
File: Platform/Posix/Source/SolidSyslogPosixMutex.c:24-27
Timestamp: 2026-05-04T14:12:11.201Z
Learning: In the solid-syslog project (DavidCozens/solid-syslog), error handling for _Create functions (e.g., SolidSyslogPosixMutex_Create returning NULL on pthread_mutex_init failure) is intentionally deferred to an error-handling epic (E12). The project has no error-reporting infrastructure yet, and _Create functions returning NULL would be inconsistent without callers uniformly handling it. This is a deliberate design decision, not an oversight.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 458
File: Tests/Lwip/SolidSyslogLwipRawResolverTest.cpp:236-244
Timestamp: 2026-05-26T09:51:07.851Z
Learning: In the solid-syslog codebase (per CLAUDE.md Design Patterns), the CHECK_REPORTED(severity, source, code) macro is the established pattern for asserting ErrorHandlerFake payloads in CppUTest tests. It wraps CALLED_FAKE(ErrorHandlerFake_Handle, ONCE), LONGS_EQUAL(severity, ErrorHandlerFake_LastSeverity()), POINTERS_EQUAL(source, ErrorHandlerFake_LastSource()), and UNSIGNED_LONGS_EQUAL(code, ErrorHandlerFake_LastCode()) in a do { } while(0) block, with NOLINTBEGIN/NOLINTEND suppressions for cppcoreguidelines-macro-usage and cppcoreguidelines-avoid-do-while. It is applied to all ErrorHandlerFake assertion sites (e.g., ExhaustedCreate, DestroyOfUnknownHandle, DestroyOfStaleHandle) across LwipRaw and other test files.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Applies to Core/{Interface,Source}/**/*.{c,h} : Public C functions use pattern `SolidSyslogClass_Function`; public types use `SolidSyslogClass`; public macros use `SOLIDSYSLOG_SCREAMING_SNAKE`
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 486
File: Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverPrivate.h:14-21
Timestamp: 2026-05-30T14:03:59.470Z
Learning: In DavidCozens/solid-syslog (`Platform/LwipRaw/`), C11 `<stdatomic.h>` must NOT be used — the layer is OS-agnostic by construction and must compile without it. The designated cross-thread primitive is `SolidSyslogLwipRaw_Marshal` (marshals a callback onto the lwIP/tcpip thread).
Cross-thread synchronization pattern for the lwIP raw DNS resolver (`SolidSyslogLwipRawDnsResolver`):
- `volatile bool Done` is acceptable as a completion-poll flag (consistent with `LwipRawTcpStream`'s `Connected`/`Errored` fields which are poll-only and have no multi-byte companion data read cross-thread).
- Multi-byte companion data (`ResolvedIp`, `ResolvedOk`) MUST NOT be read directly on the caller thread after observing `Done`; instead, once the spin observes `Done`, a second marshal hop (`DoPublishResult`) reads and publishes the authoritative result on the lwIP thread, providing the required happens-before guarantee.
- The synchronous `ERR_OK` path is safe as-is because all reads are ordered by the `dns_gethostbyname` marshal hop's return (no separate spin).
- Under NO_SYS=1 the dns_found_callback fires from inside the spin's own Sleep (same thread), so the race is only present under NO_SYS=0.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: Interface/SolidSyslogStream.h:0-0
Timestamp: 2026-04-21T05:30:24.982Z
Learning: In `Interface/SolidSyslogStream.h` of `DavidCozens/solid-syslog`, `ssize_t` (POSIX-only) is intentionally NOT used in the public Stream interface. Instead, a project-owned `typedef intptr_t SolidSyslogSsize` is declared in `Interface/SolidSyslogStream.h` and used as the return type of `SolidSyslogStream_Read`. Internal Platform/Posix implementations use POSIX `ssize_t` with a cast at the API boundary. This keeps the public API free of platform-specific types and is consistent with the project's naming conventions. Do not suggest replacing `SolidSyslogSsize` with `ssize_t` or adding MSVC shims in future reviews.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Applies to Core/Source/**/*.c : Every pool's `_Create`/`_Destroy` wraps slot probe in `SolidSyslog_LockConfig`/`_UnlockConfig` injection pair for synchronisation
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 263
File: Platform/Posix/Source/SolidSyslogPosixMutex.c:24-27
Timestamp: 2026-05-04T14:12:11.201Z
Learning: In the solid-syslog project (DavidCozens/solid-syslog), clang-tidy's bugprone-unused-return-value default allowlist does NOT include pthread_mutex_init (only pthread_mutex_trylock is in the default list). Ignoring pthread_mutex_init return value does not trigger this check. InitializeCriticalSection on Windows Vista+ returns void, so there is no return value concern for the Windows mutex implementation either.
Learnt from: CR
Repo: DavidCozens/solid-syslog PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-31T10:27:34.414Z
Learning: Applies to Core/Source/**/*.c : Use the `SolidSyslogPoolAllocator` helper (TU-internal in Core/Source/) for three-operation contract: `AcquireFirstFree`, `FreeIfInUse`, `IndexIsValid`
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: Core/Source/SolidSyslogCircularBufferStatic.c:116-127
Timestamp: 2026-05-18T05:42:39.906Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogCircularBuffer_Destroy` (Core/Source/SolidSyslogCircularBufferStatic.c) intentionally emits `SOLIDSYSLOG_SEVERITY_WARNING` / `SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY` for ALL handles not currently issued by `_Create`, including pool-exhaustion Fallback handles. This is the designed E11 reference contract: one uniform rule covers Fallback handles, double-Destroy, and stranger handles — no Fallback short-circuit. Do not flag this as a bug.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-17T19:45:13.677Z
Learning: In `DavidCozens/solid-syslog`, the extern function-pointer seam variables in `Platform/Windows/Source/SolidSyslogWinsockDatagramInternal.h` and `Platform/Windows/Source/SolidSyslogWinsockResolverInternal.h` (`Winsock_socket`, `Winsock_sendto`, `Winsock_closesocket`, `Winsock_getaddrinfo`, `Winsock_freeaddrinfo`) intentionally use underscore-based naming (mirroring the real Winsock API names) to preserve visual clarity at `UT_PTR_SET` call sites. Do not flag these names as non-camelCase in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the `_CRT_SECURE_NO_WARNINGS` compile definition is a known temporary stopgap for MSVC CRT deprecation warnings in test fakes. Issue `#126` (S13.3) will introduce a `SafeString` abstraction (Windows impl wraps `_s` variants; POSIX falls through to standard functions); once test fakes migrate, this suppression is removed. Production code is already SDL-clean. Do not flag this definition as an unjustified suppression in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: Tests/CMakeLists.txt:0-0
Timestamp: 2026-04-21T05:30:25.599Z
Learning: In `DavidCozens/solid-syslog`, `Tests/Support/OpenSslFake.c` is compiled into the `PosixFakes` static library via `POSIX_FAKES_SOURCES` in `Tests/Support/CMakeLists.txt` and is linked into the test binary via `target_link_libraries(${PROJECT_NAME}Tests PRIVATE PosixFakes)`. It does NOT need to appear directly in `TEST_SOURCES`. As of commit 790da74 it is also conditionally included only when `SOLIDSYSLOG_OPENSSL=ON`. Do not flag `OpenSslFake.c` as missing from `TEST_SOURCES` in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 421
File: Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.c:209-254
Timestamp: 2026-05-21T23:03:36.609Z
Learning: In the DavidCozens/solid-syslog codebase, when registering a custom entropy source with mbedTLS via `mbedtls_entropy_add_source`, the `MBEDTLS_ENTROPY_SOURCE_WEAK` vs `MBEDTLS_ENTROPY_SOURCE_STRONG` flag is a *structural* designation, not a quality claim. `mbedtls_entropy_func` only exits its gather loop successfully when `strong_size >= MBEDTLS_ENTROPY_BLOCK_SIZE`; a WEAK-only source means `strong_size` never advances, the loop exhausts `ENTROPY_MAX_LOOP=256` iterations, and the function returns `MBEDTLS_ERR_ENTROPY_SOURCE_FAILED`. This causes `mbedtls_ctr_drbg_seed` to return `MBEDTLS_ERR_CTR_DRBG_ENTROPY_SOURCE_FAILED` (-0x0034). If the return code is discarded (e.g. `(void)`-cast), the half-touched AES-CTR internal state still produces deterministic output that can pass TLS handshakes — a latent predictability bug. Fix: register the source as `MBEDTLS_ENTROPY_SOURCE_STRONG` and use an audit-trail printf to document the real quality level. Relevant file: `Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.c`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the MSVC suppression `/wd4297` (exception thrown from `extern "C"` function) is intentional and permanent test-infrastructure design: C fakes require C linkage, and `TestAssert_Fail` throws to propagate failures through CppUTest. Issue `#125` will add an inline comment documenting this justification. Do not flag `/wd4297` as an unjustified suppression in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 113
File: Interface/SolidSyslogFormatter.h:24-27
Timestamp: 2026-04-13T22:11:31.767Z
Learning: In the `DavidCozens/solid-syslog` repository, `static inline` functions are permitted in `Interface/` public headers. The "no implementation in `Interface/`" guideline applies only to implementation files (`.c`); `static inline` helpers (e.g. zero-cost casts like `SolidSyslogFormatter_FromStorage`) in interface headers are acceptable and intentional. Do not flag `static inline` functions in `Interface/*.h` as violating the separation-of-concerns rule.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 103
File: Source/SolidSyslogFileStore.c:124-128
Timestamp: 2026-04-11T22:49:29.830Z
Learning: In `Source/SolidSyslogFileStore.c`, the return values of `OpenWriteFile()` and `OpenReadFile()` are intentionally unchecked in `SolidSyslogFileStore_Create` (around lines 124–128) and `RotateToNextFile` (around lines 394–400). Guarding against open failures and aborting the create/rotation flow is deferred to the error-handling epic, tracked in `project_robustness_backlog.md` under "FileStore error handling (PR `#103`)". Do not flag missing open-failure guards in these functions in future reviews.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 411
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosStaticResolver.c:12-14
Timestamp: 2026-05-20T12:23:56.479Z
Learning: In DavidCozens/solid-syslog, the `analyze-iwyu` CI job runs in the `cpputest` container where `FREERTOS_KERNEL_PATH` is not set. This means FreeRTOS adapter source files under `Platform/FreeRtos/` are excluded from the IWYU subdir build entirely. Do not flag missing direct includes in these files as IWYU CI failures — the check does not cover them. Includes added for completeness are still fine as project-pattern consistency, but they are not required to pass CI.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, the inconsistent null-check guard pattern in `Tests/SolidSyslogTlsStreamTest.cpp` (lines 161–187) is intentionally deferred to issue `#31` (E12: Error Handling), which covers a project-wide null-guard consistency sweep. Do not flag this pattern inconsistency in future reviews of `SolidSyslogTlsStreamTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Tests/StreamFakeTest.cpp:0-0
Timestamp: 2026-05-22T12:15:29.505Z
Learning: In the solid-syslog codebase (DavidCozens/solid-syslog), the project deliberately avoids conditional compilation (`#ifdef`) in both production and test code. When a test needs a `struct SolidSyslogAddress*` purely as a pass-through token (i.e., the code under test never dereferences or inspects its platform-specific sockaddr contents), use the cross-platform `Tests/AddressFake.{h,c}` helper (`AddressFake_Get()` returns an opaque non-null singleton). Only tests that actually inspect the platform sockaddr internals (datagram / TCP-stream / resolver tests) should use the per-platform `SolidSyslog{Posix,Winsock,FreeRtos}Address_Create` + corresponding `Private.h` accessor APIs.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-05-13T22:05:44.151Z
Learning: In this codebase (solid-syslog), CLAUDE.md explicitly forbids adding error handling, fallbacks, or validation for scenarios that "can't happen". Internal vtable wrappers (e.g. in `Core/Source/SolidSyslogStore.c`) dereference function pointers unconditionally — a NULL vtable method is a wiring bug that should surface as an immediate crash, not be silently defaulted. Validation is only added at system boundaries (user input, external APIs).
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 440
File: misra_suppressions.txt:47-47
Timestamp: 2026-05-24T12:15:41.496Z
Learning: In the solid-syslog project, MISRA cppcheck suppressions for Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c must be validated by running cppcheck with the full CI include set (e.g. -ICore/Interface -IPlatform/Atomics/Interface -IPlatform/Posix/Interface -IPlatform/Windows/Interface -IPlatform/OpenSsl/Interface -IPlatform/MbedTls/Interface -IPlatform/FreeRtos/Interface -IPlatform/FatFs/Interface). Running cppcheck against a single file without these flags causes anonymous-enum types to go unresolved and silently suppresses findings (e.g. misra-c2012-5.7 at line 18), leading to false "stale suppression" conclusions.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 352
File: Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp:175-182
Timestamp: 2026-05-13T22:05:29.329Z
Learning: In the `solid-syslog` project, the CLAUDE.md guideline explicitly discourages adding defensive error handling or validation for scenarios that cannot happen given the existing constraints. For example, test fixture helpers like `WriteMessage` in `Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp` intentionally omit payload-size guards because `payloadSize` is always derived from `SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100` (>= 412) or hardcoded values (e.g., 64), making the < 4 case unreachable. Do not suggest adding such guards in this codebase.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 400
File: Core/Source/SolidSyslogOriginSdStatic.c:11-11
Timestamp: 2026-05-18T17:07:19.499Z
Learning: In this repo (DavidCozens/solid-syslog), an IWYU CI gate enforces strict direct-header inclusion: for every symbol (types, enums, functions, etc.) that is directly referenced in a translation unit’s `.c` or `.h` file, include the specific header that declares that symbol. Do not rely on transitive includes from other headers.
Example: if a `.c`/`.h` file uses `SolidSyslogSeverity` enums, it must directly `#include` `SolidSyslogPrival.h` even if another included header (e.g., `SolidSyslogError.h`) pulls it in transitively.
When reviewing, do not flag or recommend removing includes solely because they are “transitive” in this codebase—removing them may still compile locally but can fail the IWYU CI check.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 502
File: DEVLOG.md:0-0
Timestamp: 2026-06-01T10:52:21.000Z
Learning: For PR `#502` / S12.21 in `solid-syslog`, Structured Data and MSG-body tests intentionally remain in `Tests/SolidSyslogTest.cpp` as integration coverage of the full `SolidSyslog_Log` pipeline rather than being migrated to `Tests/SolidSyslogMessageFormatterTest.cpp`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: CMakeLists.txt:94-100
Timestamp: 2026-04-21T05:30:34.467Z
Learning: In `Example/Threaded/main.c` of `DavidCozens/solid-syslog`, three `#ifdef SOLIDSYSLOG_HAVE_OPENSSL` blocks for TLS wiring are a known temporary stopgap. Issue `#171` (S3.13 under E3) will replace them with per-backend CMake-selected factory files (`ExampleTlsSender_OpenSsl.c` / `_WolfSsl.c` / `_Unavailable.c`) to keep application code free of build-flag knowledge and provide a natural extension point for wolfSSL. Do not flag these `#ifdef` blocks as needing immediate refactoring in future reviews of `main.c`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-05-20T17:56:17.049Z
Learning: In the solid-syslog project (C codebase), composite `if` predicates bridged with `&&` are intentionally left inline when all constituent functions already have intent-revealing names (e.g. `SolidSyslogStore_Write`, `SolidSyslogStore_IsTransient`). Do not suggest extracting them into `static inline bool IsXxx` helpers — extracting would rename without clarifying. See `Core/Source/SolidSyslog.c`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: misra_suppressions.txt:27-27
Timestamp: 2026-05-18T05:43:07.289Z
Learning: In the solid-syslog codebase (`misra_suppressions.txt` and `docs/misra-deviations.md`), D.002 (Rules 11.2/11.3/11.5) covers two distinct cast patterns: (1) the vtable downcast (`struct SolidSyslogBuffer*` → `struct SolidSyslogCircularBuffer*` via `CircularBuffer_SelfFromBase`, and analogous `SelfFromBase` helpers in every other class) which is retained for all classes including those migrated under E11; and (2) the caller-storage `void*` cast which E11-migrated classes (currently only `SolidSyslogCircularBuffer`) no longer use. Do not flag the `SelfFromBase` vtable downcast as uncovered by D.002 — it is explicitly documented in `docs/misra-deviations.md` (updated in commit 9bd1d39 of PR `#394`). A potential future refactor to split D.002 into two separate blocks for the whole library was noted as out of scope for S11.01.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 513
File: Bdd/Targets/Common/BddTargetStderrErrorHandler.c:19-22
Timestamp: 2026-06-02T22:57:23.006Z
Learning: In `Bdd/Targets/Common/BddTargetStderrErrorHandler.c` (Tier-3 BDD diagnostic code), the cast `source->AsString((uint8_t) event->Detail)` is intentionally left without a prior range guard in S12.25. The rationale: in S12.25 `event->Detail` is always a per-class `enum SolidSyslog<Class>Errors` value (values well under 256), and `AsString` itself takes `uint8_t` and returns `"unknown"` for out-of-range inputs. The explicit range check `(event->Detail >= 0 && event->Detail <= UINT8_MAX)` before the cast, together with the `AsString` signature change to accept wider native codes (errno, X509_V_ERR_*), is deferred to sibling story S12.26. Do not flag this cast as a truncation risk until S12.26 lands.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 327
File: Core/Source/SolidSyslog.c:220-224
Timestamp: 2026-05-11T10:23:17.520Z
Learning: In DavidCozens/solid-syslog, `InstallStructuredData` in `Core/Source/SolidSyslog.c` intentionally assigns `instance.sd = configured` and `instance.sdCount = count` without a NULL guard. The correct follow-on fix for the `sd == NULL && sdCount > 0` inconsistency is to report a new `SOLIDSYSLOG_ERROR_MSG_CREATE_INCONSISTENT_SD` error rather than silently skipping the assignment. This is tracked under E12 alongside S12.06 (NULL guards for SD implementations). Do not flag the missing NULL guard in `InstallStructuredData` as a defect in the nil-object defaults PR (S12.04); it requires its own TDD red and error-message constant.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 205
File: Core/Source/SolidSyslogFormatter.c:248-274
Timestamp: 2026-04-24T19:21:31.563Z
Learning: In `Core/Source/SolidSyslogFormatter.c` (`DavidCozens/solid-syslog`), `SolidSyslogFormatter_EscapedString` has a known deferred defect: when the output buffer has exactly 1 byte of remaining capacity and the current source byte needs escaping (`"`, `\`, `]`), `ESCAPE_PREFIX` is written but the escaped character is silently clamped, leaving a dangling `\` at the buffer tail. `AsFormattedBuffer`'s `TrimTruncatedMultiByteTail` does not cover this case because `\` is valid ASCII. The practical impact is zero for all in-tree callers because they size via `SOLIDSYSLOG_ESCAPED_MAX_SIZE(rawMax)` = `2 * rawMax`, which always provides sufficient headroom. The agreed fix (Option 1) is a targeted output-capacity check before the escape+codepoint pair write (add `OutputHasCapacityFor(formatter, outputBytes)` guard; break on failure). Do not re-flag this as a new issue in future reviews until the targeted fix is merged.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag the missing NULL checks in StreamSender_Create as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 124
File: CMakeLists.txt:12-14
Timestamp: 2026-04-15T06:15:28.657Z
Learning: In `CMakeLists.txt` of `DavidCozens/solid-syslog`, the MSVC suppression `/wd4200` (flexible array member) is a known stopgap. Issue `#125` (S13.2) will replace `char buffer[]` with the `char buffer[1]` pre-C99 idiom and remove this suppression. Do not flag `/wd4200` in future reviews of this file.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag missing NULL checks in `StreamSender_Create` as an issue in future PRs until `#424` is addressed.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 283
File: Platform/OpenSsl/Interface/SolidSyslogTlsStream.h:7-7
Timestamp: 2026-05-07T08:22:53.110Z
Learning: In solid-syslog, the CLAUDE.md callback rule that requires a `void* context` parameter (and the paired config field) should be applied only to callbacks that need to carry real per-instance state. Exempt callbacks that are stateless thin wrappers over OS primitives—i.e., the callback is effectively a pure function of its input parameters with no instance state to preserve—so adding `void* context` would be pointless (the context would always be NULL). For example, platform implementations like `SolidSyslogSleepFunction` that wrap `nanosleep`/`Sleep` and are defined purely in terms of `int milliseconds` should not add `void* context` or the paired config field. For callbacks that do depend on per-instance data, include `void* context` and the paired config field as required by CLAUDE.md.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/Common/ExampleInteractive.h:14-17
Timestamp: 2026-05-09T15:56:13.853Z
Learning: During code review, ensure solid-syslog Tier 1/2 library callbacks in Core/ and Platform/ follow the CLAUDE.md “Callback Conventions”: the callback API must take a `void*` context parameter and the implementation must use the paired configuration field associated with that callback context. Do not apply this requirement to Tier 3 example handlers (e.g., Example/Common/, Example/FreeRtos/, such as `ExampleInteractiveSwitchHandler`/`ExampleInteractiveSetHandler`), which are allowed to use file-static globals per the documented example-tier pattern. If a multi-instance use-case arises, migrate Switch and Set together in a single cohesive change rather than piecemeal updates.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 419
File: Platform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.h:17-18
Timestamp: 2026-05-21T12:53:11.114Z
Learning: When reviewing code in adapter-specific TU-private headers matching *Private.h, accept the established two-tier C naming convention:
- Use `SolidSyslogClass_Function` (full `SolidSyslog...` prefix) only for the public integrator-facing API declared in the public Interface header.
- In `*Private.h`, allow `Class_Function` (bare/short prefix) names for private cross-translation-unit helper functions shared between the two `.c` files of a single adapter.
These `Class_Function` helpers are declared in the TU-private `*Private.h` and intentionally have external linkage (so they must not be `static`) because they are referenced across translation units; therefore reviewers should NOT flag `Class_Function`-style names in `*Private.h` headers as naming-convention violations. Follow the precedent shown in `Platform/OpenSsl/Source/SolidSyslogTlsStreamPrivate.h`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Closes #507. Sibling to S12.25 — completes the error-reporting redesign by
decoupling descriptive text from the library. With the portable category
axis (S12.25) in place, the library now carries codes only: no error
message strings link into
Core/orPlatform/. Codes-only / flash-constrainedand i18n integrators get a binary with zero library prose by construction;
porting/dev integrators install a small consumer-side handler that turns codes
into text.
Change Description
struct SolidSyslogErrorSourcedropsAsString→{ const char* Name; }. We considered keepingAsStringas an overridablelink-seam (the original intent, which had drifted to a
static, always-linkedresolver) but judged it overkill: its only payoff is free per-detail dispatch
in the bundled handler, at the cost of a
TextNull.cworkaround to reachzero-text. The slim struct makes codes-only true by construction.
*Messages.cdeleted. Each error-source symbol is relocated to itsclass's allocation-agnostic vtable TU
Xxx.c(notStatic.c, which is thestatic-allocation TU a future
Dynamic.cwould mirror), gainingSolidSyslogError.h+ its*Errors.h(MISRA 8.4). All CMake references purged.Bdd/Targets/Common/BddTargetErrorText.c—
BddTargetErrorText_Category(uint16_t)maps the 12 portable categories toprose. The three BDD/FreeRtos handlers now print
Source->Name+ category textDetail; no per-source dispatch. Promotable to a top-levelExample/later without touching the library.
_Reportwrapper unwound. The 4 crypto-policy classes were the only onesrouting emission through
Xxx_Report(sev, cat, code); inlined all 24 call sitesto the direct
SolidSyslog_Error(...)the other 39 classes use, making themuniform.
source's use to one TU; the unwind makes them cross-TU referenced like every
other source. Suppressions removed; deviation tombstoned.
Test Evidence
BddTargetErrorText_CategoryTDD'd: RED (missing header) → GREEN (onecategory) → full 12-category table; 12 new assertions in
BddTargetErrorTextTest.cpp.SolidSyslogUdpSenderErrorSourcegroup that asserted now-removed message text;fixed the 3
{"test", nullptr}literals inSolidSyslogErrorTest.cpp.suites (FreeRtos/Lwip/MbedTls/FatFs); both FreeRTOS cross BDD ELFs built;
clang-tidy, clang-format, cppcheck-misra all clean. Windows/MSVC left to CI.
Areas Affected
Core/Interface/SolidSyslogError.h(slim struct); 43 vtable TUs acrossCore/Platform/*(relocated symbols); 4 crypto-policy.c/Private.h(_Reportunwind); BDD target handlers + CMake;
docs/misra-deviations.md,misra_suppressions.txt(D.014); DEVLOG.SolidSyslogErrorSourcestruct.🤖 Generated with Claude Code
Summary by CodeRabbit