feat: S14.07 move header-field callbacks off the formatter#557
Conversation
Introduce SolidSyslogHeaderField — an opaque, stack-transient US-ASCII writer for the RFC 5424 HOSTNAME / APP-NAME / PROCID header fields. It wraps the message-buffer formatter and exposes only a PRINTUSASCII append (printable US-ASCII '!'..'~', everything else — space included — substituted) bounded to the field width, plus a Uint32 for PROCID. The PRINTUSASCII charset is a different rule from SD escaping, hence a dedicated writer rather than reusing SolidSyslogSdValue. Change the three header-field callbacks from SolidSyslogStringFunction to the new SolidSyslogHeaderFieldFunction (void(SolidSyslogHeaderField*, void* context)) with paired context fields on SolidSyslogConfig and the MessageFormatter context. MessageFormatter now builds one HeaderField per field (capped at the field width) and passes it; the scratch-field formatter + re-filter is gone. The Posix/Windows hostname & processId helpers and every BDD header-field callback move to the new shape. SolidSyslogStringFunction.h is retired — it was the last public consumer of SolidSyslogFormatter, clearing the way for S14.08. PosixMessageQueueBuffer, which reused the PROCID helper to stamp its queue name with the pid, now formats getpid() directly rather than depend on the header-field helper. Output is byte-for-byte identical; existing tests stand. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR migrates HOSTNAME/APP-NAME/PROCID header callbacks from formatter/string callbacks to a new SolidSyslogHeaderField writer with per-call ChangesHeader-field callback migration
Sequence Diagram(s)(omitted — changes are primarily API migration and internal wiring; no additional diagram provided) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1506 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/Source/SolidSyslogHeaderField.c`:
- Around line 28-34: SolidSyslogHeaderField_Uint32 currently subtracts
HeaderField_Consumed from field->Remaining without protecting against the
formatter emitting more bytes than the remaining budget; change
SolidSyslogHeaderField_Uint32 to first record size_t before =
SolidSyslogFormatter_Length(field->Formatter) and size_t beforeRemaining =
field->Remaining, skip calling SolidSyslogFormatter_Uint32 entirely if
beforeRemaining == 0, otherwise call the formatter, compute size_t consumed =
HeaderField_Consumed(field, before) and then if (consumed >= beforeRemaining)
set field->Remaining = 0 else subtract consumed from field->Remaining; this
prevents size_t underflow and enforces the header-field width while still
accounting for actual bytes produced.
🪄 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: f6e398ec-f2f1-44cb-999e-ba40944eb2ac
📒 Files selected for processing (43)
Bdd/Targets/Common/BddTargetAppName.cBdd/Targets/Common/BddTargetAppName.hBdd/Targets/Common/BddTargetFreeRtosPipeline.cBdd/Targets/Common/BddTargetFreeRtosPipeline.hBdd/Targets/FreeRtos/main.cBdd/Targets/FreeRtosLwip/main.cCore/Interface/SolidSyslogConfig.hCore/Interface/SolidSyslogHeaderField.hCore/Interface/SolidSyslogHeaderFieldFunction.hCore/Interface/SolidSyslogStringFunction.hCore/Source/CMakeLists.txtCore/Source/SolidSyslog.cCore/Source/SolidSyslogHeaderField.cCore/Source/SolidSyslogHeaderFieldPrivate.hCore/Source/SolidSyslogMessageFormatter.cCore/Source/SolidSyslogMessageFormatter.hCore/Source/SolidSyslogPrivate.hCore/Source/SolidSyslogStatic.cDEVLOG.mdPlatform/Posix/Interface/SolidSyslogPosixHostname.hPlatform/Posix/Interface/SolidSyslogPosixProcessId.hPlatform/Posix/Source/SolidSyslogPosixHostname.cPlatform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.cPlatform/Posix/Source/SolidSyslogPosixProcessId.cPlatform/Windows/Interface/SolidSyslogWindowsHostname.hPlatform/Windows/Interface/SolidSyslogWindowsProcessId.hPlatform/Windows/Source/SolidSyslogWindowsHostname.cPlatform/Windows/Source/SolidSyslogWindowsProcessId.cTests/Bdd/Targets/BddTargetAppNameTest.cppTests/Bdd/Targets/BddTargetServiceThreadTest.cppTests/CMakeLists.txtTests/SolidSyslogHeaderFieldTest.cppTests/SolidSyslogMessageFormatterTest.cppTests/SolidSyslogPoolTest.cppTests/SolidSyslogPosixMessageQueueBufferTest.cppTests/SolidSyslogTest.cppTests/SolidSyslogWindowsHostnameTest.cppTests/SolidSyslogWindowsProcessIdTest.cppTests/StringFake.cTests/StringFake.hTests/StringFakeTest.cppdocs/rfc-compliance.mdmisra_suppressions.txt
💤 Files with no reviewable changes (1)
- Core/Interface/SolidSyslogStringFunction.h
A header-field callback is arbitrary user code that may make any number of mixed _PrintUsAscii / _Uint32 appends on one field, so the field-width budget must hold across calls. _Uint32 subtracted the consumed bytes from Remaining unconditionally, underflowing size_t when a number overran the remainder. Skip the write once the field is full and clamp Remaining to zero instead of underflowing. Two tests cover the mixed-append cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1508 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Closes #547. Parent epic: #64.
What changes
Introduces
SolidSyslogHeaderField— an opaque, stack-transient US-ASCII writer forthe RFC 5424 HOSTNAME / APP-NAME / PROCID header fields. It wraps the message-buffer
formatter and exposes only:
_PrintUsAscii(source, maxLength)— printable-US-ASCII (!..~) filter, everythingelse (space included) substituted, NUL-stopped, bounded by
maxLengthand the fieldwidth.
_Uint32(value)— for PROCID.These fields use the PRINTUSASCII charset (a different rule from SD escaping), so this is a
dedicated writer rather than a reuse of
SolidSyslogSdValue.SolidSyslogStringFunctiontoSolidSyslogHeaderFieldFunction(void(struct SolidSyslogHeaderField*, void* context)),in its own header, with paired context fields on
SolidSyslogConfigand theMessageFormatter context.
MessageFormatterbuilds oneSolidSyslogHeaderFieldper field (cap = field-storagesize − 1, preserving the old scratch-formatter's null-slot width) and passes it — the
scratch-field + re-filter path is gone.
to the new shape.
SolidSyslogStringFunction.his retired — it was the last public consumer ofSolidSyslogFormatter, clearing the way for S14.08 to make the formatter fully private.Output is byte-for-byte identical — existing tests stand (including the 255 / 48 / 128
field-truncation cases).
SolidSyslogHeaderFielditself is TDD'd by a new 10-test unit.Surprises handled
PosixMessageQueueBuffercoincidentally reused the PROCID helper to stamp its queue namewith the pid; decoupled to a direct
getpid()+Formatter_Uint32rather than route aqueue name through the header-field helper (which would force a Platform source to include
a Core/Source-private header).
SolidSyslogConfig{}test literals silently misaligned once three contextfields were interleaved (
storelanding in avoid*context slot) — fixed.Acceptance criteria
_PrintUsAsciiStringrule)NULLcallback → NILVALUE-as todayvoid* contextSolidSyslogStringFunction.hremoved and no references remainOut of scope
SolidSyslogFormatter.hout of the public interface (S14.08), with the wholesaleCLAUDE.md header-table revision.
Checks
Debug green (1469 tests); BddTargetTests 66/66. clang-format applied; cppcheck-MISRA exit 0
(14 suppression anchors moved: SolidSyslog.c 11.8 ×8, MessageFormatter.c 11.8 ×5 + 5.7
20→21;
misra_renumber.pyclean). The Windows hostname/processId helper tests are updatedbut only build in the Windows CI lane.
🤖 Generated with Claude Code
Summary by CodeRabbit