refactor!: S23.22 name platform callbacks for what they supply - #739
Merged
Conversation
The platform token becomes the class and the function names what it fills, matching the SolidSyslogConfig field it is assigned to. A bare _Get passes the naming regex while carrying no meaning, and Sleep had no function part at all. No compatibility alias: an old name fails to compile rather than working quietly. The rule is written into docs/NAMING.md so it stops being implicit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Corrects the host-identity claim, and states the three things the page was silent on: writes are not flushed, the keepalive window is 85 seconds, and tuning it needs Windows 10 1709. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughThe change renames platform callback APIs to use consolidated naming. Target integrations, tests, and documentation now use the new names. Callback behaviour and signatures remain unchanged. ChangesPlatform callback naming
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/platforms/posix/setup.md`:
- Around line 48-49: Update the callback names in the documentation example to
SolidSyslogPosix_GetHostname and SolidSyslogPosix_GetProcessId, alongside the
existing SolidSyslogPosix_GetTimestamp name. Preserve the surrounding guidance
and ensure all listed callbacks follow the required
SolidSyslog<RegistryToken>_<WhatItSupplies> naming form.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b279669-a6dc-4e35-9240-1c4b7b817500
📒 Files selected for processing (42)
Bdd/Targets/Common/BddTargetFreeRtosPipeline.cBdd/Targets/Common/BddTargetTlsSender_OpenSsl_PosixTcp.cBdd/Targets/Common/BddTargetTlsSender_OpenSsl_WinsockTcp.cBdd/Targets/Linux/main.cBdd/Targets/Windows/BddTargetWindows.cPlatform/FreeRtos/Interface/SolidSyslogFreeRtosSysUpTime.hPlatform/FreeRtos/Source/SolidSyslogFreeRtosSysUpTime.cPlatform/Posix/Interface/SolidSyslogPosixClock.hPlatform/Posix/Interface/SolidSyslogPosixHostname.hPlatform/Posix/Interface/SolidSyslogPosixProcessId.hPlatform/Posix/Interface/SolidSyslogPosixSleep.hPlatform/Posix/Interface/SolidSyslogPosixSysUpTime.hPlatform/Posix/Source/SolidSyslogPosixClock.cPlatform/Posix/Source/SolidSyslogPosixHostname.cPlatform/Posix/Source/SolidSyslogPosixProcessId.cPlatform/Posix/Source/SolidSyslogPosixSleep.cPlatform/Posix/Source/SolidSyslogPosixSysUpTime.cPlatform/Windows/Interface/SolidSyslogWindowsClock.hPlatform/Windows/Interface/SolidSyslogWindowsFile.hPlatform/Windows/Interface/SolidSyslogWindowsHostname.hPlatform/Windows/Interface/SolidSyslogWindowsProcessId.hPlatform/Windows/Interface/SolidSyslogWindowsSleep.hPlatform/Windows/Interface/SolidSyslogWindowsSysUpTime.hPlatform/Windows/Source/SolidSyslogWindowsClock.cPlatform/Windows/Source/SolidSyslogWindowsHostname.cPlatform/Windows/Source/SolidSyslogWindowsProcessId.cPlatform/Windows/Source/SolidSyslogWindowsSleep.cPlatform/Windows/Source/SolidSyslogWindowsSysUpTime.cTests/FreeRtos/SolidSyslogFreeRtosSysUpTimeTest.cppTests/SolidSyslogPosixClockTest.cppTests/SolidSyslogPosixSleepTest.cppTests/SolidSyslogPosixSysUpTimeTest.cppTests/SolidSyslogWindowsClockTest.cppTests/SolidSyslogWindowsHostnameTest.cppTests/SolidSyslogWindowsProcessIdTest.cppTests/SolidSyslogWindowsSleepTest.cppTests/SolidSyslogWindowsSysUpTimeTest.cppdocs/NAMING.mddocs/hardening-path.mddocs/platforms/posix/setup.mddocs/platforms/windows/index.mddocs/platforms/windows/setup.md
The Posix page carried the same missing-suffix defect the Windows page did, and the FreeRTOS page named the component where it means the function. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/platforms/freertos/setup.md`:
- Line 53: Update the documentation for SolidSyslogFreeRtos_GetSysUpTime to
state that it returns uptime in hundredths of a second derived from the FreeRTOS
tick count, replacing the claim that it reports kernel ticks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 28bf721b-2a21-434c-90b1-20379ce36e5c
📒 Files selected for processing (2)
docs/platforms/freertos/setup.mddocs/platforms/posix/setup.md
Contributor
☀️ Quality Summary Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result. |
DavidCozens
added a commit
that referenced
this pull request
Aug 10, 2026
* docs: S23.22 hold the Posix page to what the adapters do Adds the three things the page was silent on — writes are not fsynced, the dead-peer window is 30 to 85 seconds, and the message queue is capped by fs/mqueue rather than by the caller — and corrects the host-identity wording. The link guidance said the build links rt for you. Nothing in the CMake tree does; glibc 2.34 moved the message-queue functions into libc, so the promise has never been tested by a container new enough to run CI. Also carries the FreeRTOS uptime unit correction deferred from #739's review. Part of #708. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: say what the build actually does about rt Core/Source/CMakeLists.txt links rt PUBLIC under SOLIDSYSLOG_POSIX, so a CMake consumer inherits it. Only a direct consumer of the archive adds -lrt, and only on glibc before 2.34. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Two things, both wanted before 0.1.0.
The Windows triage under S23.22 found the setup page naming two callbacks that
do not exist. Fixing the page raised the better question:
_Getas a wholefunction name passes the naming regex while saying nothing, because the noun has
already been spent on the class. Renaming is free now and costs a major version
later, so it happens here rather than after the beta.
The Windows page itself is then held to what the adapters actually do.
Part of #708.
Change Description
The rename. The platform's registry token becomes the class, and the
function names what it supplies — the same words as the
SolidSyslogConfigfield it is assigned to:
SolidSyslog<Plat>Clock_GetTimestampSolidSyslog<Plat>_GetTimestampSolidSyslog<Plat>Hostname_GetSolidSyslog<Plat>_GetHostnameSolidSyslog<Plat>ProcessId_GetSolidSyslog<Plat>_GetProcessIdSolidSyslog<Plat>SysUpTime_GetSolidSyslog<Plat>_GetSysUpTimeSolidSyslog<Plat>SleepSolidSyslog<Plat>_SleepEleven public symbols across
Posix,WindowsandFreeRtos, which ships theuptime alone. Decisions worth stating:
quietly, which is the behaviour a pre-1.0 rename should have.
SolidSyslogPosixHostname.hstill holdsSolidSyslogPosix_GetHostname, so the platform-token rule holds over everyfilename and the file is still findable by guessing. This is the one place the
file name and the class name in it differ, and
docs/NAMING.mdnow says so —the previous convention lived nowhere, which is how it drifted.
The Windows page. Two corrections and three additions, each read out of the
adapter rather than inferred:
SolidSyslogWindowsHostname/SolidSyslogWindowsProcessId,neither of which is a symbol;
physical DNS host name;
WindowsFile_Writeis_writeand nothing else, so a record the storebelieves is stored is the operating system's rather than the disk's;
equivalent, so the write-in-flight case falls to the system's retransmission
behaviour;
whether
setsockopttook.Checked and deliberately left alone: the Datagram adapter meets its contract with
no exception (
OVERSIZEfromWSAEMSGSIZE,MaxPayloadfromIP_MTU), so#736does not reach it; the connected UDP socket does not pin a stale peer,because an endpoint version change closes it; and all-or-nothing
Sendis theStream contract's claim, not this platform's.
Test Evidence
No behaviour changes, so no tests were added. The existing suites are what prove
the rename:
debug+junit(gcc image)SolidSyslogFreeRtosSysUpTimeTest(freertos-host image)clang-format --dry-run --Werror, whole treescripts/check_platform_docs.pyscripts/misra_renumber.pymkdocs build --strictThe MSVC sources compile nowhere locally. They were renamed by the same
sweep and reviewed by hand;
build-windows-msvcandbdd-windows-otelare thefirst real compile of them.
Areas Affected
Platform/Posix/,Platform/Windows/,Platform/FreeRtos/public headers andtheir sources; the tests and BDD targets that wire them;
docs/NAMING.mdand thePosix and Windows platform pages.
Breaking for integrators, which is why it is milestoned into the beta. The
two example repos regenerate from the 0.1.0 tag and pick up the new names in
that pass; they are deliberately not patched here.
Summary by CodeRabbit
Changes
Documentation
Tests