feat: S28.04 SolidSyslogLwipRawDatagram#464
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Split the per-test Open/Close boilerplate into a TEST_BASE + TEST_GROUP_BASE pair: SolidSyslogLwipRawDatagram (Created-only) and SolidSyslogLwipRawDatagramOpen (Open lifted into setup). Test bodies now focus on the single behaviour under observation. Add LwipUdpFake_OutstandingPcbCount and check it in the shared teardown: every udp_pcb handed out by udp_new must return via udp_remove by the end of the test. Catches PCB leaks across all 13 lifecycle tests at zero per-test cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…axPayload SendTo allocates a PBUF_REF pbuf pointing at the caller's buffer, calls udp_sendto, and frees the pbuf — on success and on every failure path. Guards: not-open returns FAILED; pbuf_alloc-NULL returns FAILED without sendto; udp_sendto error returns FAILED. MaxPayload returns the IPv6-safe 1232-byte default unconditionally (no netif MTU lookup). Test fixture grew an LwipPbufFake with the same shape as LwipUdpFake (spies, failure injection, OutstandingPbufCount). The shared teardown asserts both pcb and pbuf outstanding counts are zero, catching leaks across every lifecycle test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ytes helper Add a 1232-byte sendBuffer member and a sendBytes(length = 1) helper on LwipRawDatagramTestBase. Most SendTo tests reduce to a single helper call; tests asserting on length pass it explicitly. Cuts ~7 lines of boilerplate per test without losing any check. SendToPbufPayloadPointsAtCallerBuffer keeps its own distinct local buffer so the pointer-identity assertion stays meaningful — the test must demonstrate the production forwarded the exact pointer the test handed in, not a fixture address that always round-trips. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…endBuffer Drop the local callerBuffer in favour of writing the known content into the fixture's sendBuffer and going through the shared sendBytes helper. Same pointer-identity proof, consistent with every other SendTo test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-of-story bundle: CLAUDE.md public-headers rows for the new
LwipRawDatagram + LwipRawDatagramErrors headers; CMake STATUS message
updated to "Address+Resolver+Datagram only; TcpStream/BDD arrive in
S28.05-S28.06"; DEVLOG entry covering the pbuf strategy / ARP
delegation / leak-invariant test pattern and the next-up pointer.
IWYU pass over the four touched files (production + two fakes + the
test cpp) — three header adds, zero removes. clang-format reflow on the
test file.
cppcheck-misra surfaced six findings in Platform/LwipRaw/ that the CI
analyze-cppcheck step does not yet scan. CI is extended in this PR to
add Platform/LwipRaw/{Interface,Source/} to all three cppcheck
invocations; suppressions land for the four S28.03-era findings
(LwipRawAddressPrivate.h: 11.2 / 11.3; LwipRawAddressStatic.c: 11.3)
that have been invisible since S28.03 merged, plus the two S28.04
findings (LwipRawDatagram.c:43 SelfFromBase 11.3; LwipRawDatagram.c:94
pbuf->payload const-strip 11.8). D.006 in docs/misra-deviations.md
gains a sub-section for the lwIP pbuf->payload site as the second
"forced by an external interface" entry alongside the Winsock select()
case.
S28-04-HANDOFF.md removed.
Closes #463.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR implements S28.04 of the E28 epic: a new lwIP Raw UDP datagram transport ( ChangeslwIP Raw Datagram Implementation
Sequence DiagramsequenceDiagram
participant App
participant Datagram as SolidSyslogLwipRawDatagram
participant pbuf as pbuf_alloc/free
participant UDP as udp_new/sendto/remove
App->>Datagram: Create()
Datagram->>UDP: udp_new()
UDP-->>Datagram: PCB
Datagram-->>App: datagram handle
App->>Datagram: Open()
Datagram->>UDP: udp_new() [idempotent]
UDP-->>Datagram: existing PCB
App->>Datagram: SendTo(buffer, len, ip, port)
Datagram->>pbuf: pbuf_alloc(TRANSPORT, len, REF)
pbuf-->>Datagram: pbuf (or NULL)
alt success
Datagram->>UDP: udp_sendto(pcb, pbuf, ip, port)
UDP-->>Datagram: ERR_OK or error
Datagram->>pbuf: pbuf_free(pbuf)
pbuf-->>Datagram: freed
Datagram-->>App: SENT or FAILED
else pbuf allocation fails
Datagram-->>App: FAILED
end
App->>Datagram: Close()
Datagram->>UDP: udp_remove(pcb)
UDP-->>Datagram: ok
App->>Datagram: Destroy()
Datagram-->>App: ok
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 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.
🧹 Nitpick comments (1)
CMakeLists.txt (1)
170-177: ⚡ Quick winAlign the nearby milestone comment with the new LWIP status text.
The status strings now say Datagram is already available, but the explanatory block around Line 153 still says “Datagram/TcpStream/BDD arrive in S28.04-S28.06.” Updating that comment in the same edit will avoid roadmap confusion.
🤖 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 `@CMakeLists.txt` around lines 170 - 177, Update the explanatory milestone comment that references the SOLIDSYSLOG_FREERTOS_NET status so it matches the new message(STATUS) text for LWIP and BOTH (which now indicate Datagram is available); find the block that previously said “Datagram/TcpStream/BDD arrive in S28.04-S28.06” and change it to reflect the new timeline wording used in the message(STATUS) strings (e.g., indicate Datagram is already available and TcpStream/BDD arrive in S28.05-S28.06), ensuring the comment and the message(STATUS) outputs for SOLIDSYSLOG_FREERTOS_NET (values "LWIP" and "BOTH") are consistent.
🤖 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.
Nitpick comments:
In `@CMakeLists.txt`:
- Around line 170-177: Update the explanatory milestone comment that references
the SOLIDSYSLOG_FREERTOS_NET status so it matches the new message(STATUS) text
for LWIP and BOTH (which now indicate Datagram is available); find the block
that previously said “Datagram/TcpStream/BDD arrive in S28.04-S28.06” and change
it to reflect the new timeline wording used in the message(STATUS) strings
(e.g., indicate Datagram is already available and TcpStream/BDD arrive in
S28.05-S28.06), ensuring the comment and the message(STATUS) outputs for
SOLIDSYSLOG_FREERTOS_NET (values "LWIP" and "BOTH") are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93d5a997-e78f-4d70-a33a-d5ec6d6d5b86
📒 Files selected for processing (20)
.github/workflows/ci.ymlCLAUDE.mdCMakeLists.txtCore/Interface/SolidSyslogTunablesDefaults.hDEVLOG.mdPlatform/LwipRaw/Interface/SolidSyslogLwipRawDatagram.hPlatform/LwipRaw/Interface/SolidSyslogLwipRawDatagramErrors.hPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagram.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.cPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.hPlatform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.cTests/CMakeLists.txtTests/Lwip/CMakeLists.txtTests/Lwip/SolidSyslogLwipRawDatagramTest.cppTests/Support/LwipFakes/Interface/LwipPbufFake.hTests/Support/LwipFakes/Interface/LwipUdpFake.hTests/Support/LwipFakes/Source/LwipPbufFake.cTests/Support/LwipFakes/Source/LwipUdpFake.cdocs/misra-deviations.mdmisra_suppressions.txt
7 tasks
DavidCozens
added a commit
that referenced
this pull request
May 28, 2026
* chore: S28.05 LwipRawTcpStream pool plumbing + tunables + tests Lays the three-TU pool scaffolding for the upcoming SolidSyslogLwipRawTcpStream adapter — Interface header, Errors header, Private header, .c (Initialise/Cleanup only — vtable methods land in commits 2/3 alongside Open/Send/Read), Messages.c (ErrorSource), Static.c (pool + Create/Destroy with the ConfigLock-wrapped slot walks). Adds three new tunables (SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE default 2, SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS default 10, SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE default 8). Bad-config (NULL config or NULL Sleep) resolves silently to the shared SolidSyslogNullStream — matches the sibling-backend contract; only POOL_EXHAUSTED and UNKNOWN_DESTROY are reported. 13 tests: CreateReturnsNonNullStream, CreatedStreamIsNotTheNullStreamSingleton, DestroyReleasesSlotToPool, CreateWithNullConfigReturnsFallback, CreateWithNullSleepReturnsFallback, plus the 9-test pool block mirroring SolidSyslogPlusTcpTcpStream's shape. Coverage: 100% line on TcpStream.c + TcpStreamStatic.c; Messages.c exercised only via integrator-installed error handlers (matches S28.04 Datagram). Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: S28.05 LwipRawTcpStream Open/Close lifecycle + connect callback bridge Wires Open / Close against lwIP Raw's tcp_new / tcp_arg / tcp_recv / tcp_sent / tcp_err / tcp_connect / tcp_close / tcp_abort. Bridges the lwIP callback Raw TCP API to SolidSyslogStream's synchronous Open(addr) contract via a bounded spin loop driven by the integrator-injected SolidSyslogSleepFunction — each iteration sleeps SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS so lwIP's timer / RX paths keep getting cycles to advance the SYN/SYN-ACK exchange. Deadline is the existing per-instance GetConnectTimeoutMs getter (S12.17 pattern), with a Null Object falling back to SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS. Encapsulates lwIP's tcp_close-after-tcp_err gotcha: the wrapper's ErrCallback nulls self->Pcb when lwIP releases the pcb upstream, and Close checks Pcb != NULL before calling tcp_close — integrators never see the rule. Sets SOF_KEEPALIVE on every pcb (idle / intvl / cnt come from the integrator's lwipopts.h — LWIP_TCP_KEEPALIVE=1 is the recommended-on setting, documented in S28.05 commit 4's docs/integrating-lwip.md). Send / Read / RX queue land in the next commit; for now Recv / Sent callback slots hold no-op stubs (lwIP requires them set, but the synchronous API surface doesn't exercise their behaviour yet). LwipTcpFake.{h,c} added — captures registered callbacks so tests can drive connected_cb / err_cb directly; default tcp_connect behaviour synchronously fires connected_cb with ERR_OK so happy-path Open tests don't loop. OutstandingPcbCount leak invariant pinned in shared teardown. 26 new tests across two TEST_GROUP_BASE groups (created-only + Open-already), covering: tcp_new fail, keepalive, callback registration, immediate connect-error path, errored-callback path, timeout-with-abort path, sleep-between-polls behaviour, runtime-tunable deadline, idempotency, close-after-tcp_err, destroy-after-tcp_err. Coverage: 100% line on TcpStream.c + TcpStreamStatic.c (35 tests in the exe; full debug suite 17/17 green). Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: S28.05 LwipRawTcpStream Send/Read + RX queue + tcp_err handling Wires Send / Read against lwIP Raw's tcp_write / tcp_output / tcp_recved + a bounded pbuf ring drained by Stream_Read. Send takes the TCP_WRITE_FLAG_COPY path — lwIP copies the caller's buffer into its own pbufs before tcp_write returns, so the synchronous Stream_Send(buf, len) lifetime contract is honoured. ERR_MEM from tcp_output after a successful tcp_write is queued-not- failed (matches POSIX's "kernel accepted into the send buffer" semantics); anything else closes internally. RX queue is a fixed ring sized by SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE (default 8). The tcp_recv callback enqueues incoming pbufs, returning ERR_MEM when full so lwIP retains the pbuf and replays the callback later (flow control). Read drains bytes from the head pbuf, calls tcp_recved(pcb, n) to ACK back to lwIP's receive window, and pbuf_free's the head when fully drained. Peer FIN (tcp_recv with p == NULL) sets Errored but the wrapper drains any already-queued bytes before signalling EOF — next Read with an empty queue returns -1 and ClosePcb's the connection internally per the Stream contract. tcp_err's Pcb-already-nulled state is handled by IsOpen() — Read returns -1 without a double-tcp_close. ClosePcb now drains the queue unconditionally before tcp_close (under the existing Pcb != NULL guard) — pbufs we accepted via tcp_recv are ours to free regardless of pcb state. 20 new tests across the Connected group: Send happy/fail/COPY-flag/ ERR_MEM-queued/ERR_CONN-closes, post-tcp_err Send-false. Read queue-empty/drain-head/recved-ACK/partial-then-finish/multi-pbuf- advance/peer-FIN-after-drain/post-tcp_err. RX backpressure when queue full. Destroy-drains-queue + Close-drains-queue leak invariants. Coverage: 100% line + function on TcpStream.c + TcpStreamStatic.c (55 tests in the exe; full debug suite 17/17 green). Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: S28.05 LwipRawTcpStream tests — sendBytes/readBytes helpers, auto-balanced pushIncomingPbuf, CHECK_FORWARDED_PCB Three test-side cleanups, no production code change: 1. sendBytes() / readBytes() on the shared TEST_BASE fixture wrap the abstract Stream Send/Read against the existing sendBuffer/readBuffer fields. Matches the Datagram precedent's sendBytes helper. Removes ~18 repetitions of SolidSyslogStream_{Send,Read}(stream, buffer, N). 2. pushIncomingPbuf now returns the err_t the wrapper's tcp_recv callback returned and only bumps LwipPbufFake_NoteIncomingPbuf when that return was ERR_OK. The semantic "if the wrapper accepted the pbuf the test counts it; otherwise lwIP retains it and the count stays put" is now encoded in one place, not duplicated in each call site. RecvCallbackBackpressuresWhenQueueFull drops its inline fabrication + manual drain, both now subsumed by the helper's auto-balancing (queue drain runs in shared teardown via Destroy). 3. CHECK_FORWARDED_PCB(getter) macro replaces the recurring POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) intent ("the lwIP call received our pcb"). Used across Open/Send/Read/ Close pcb-forwarding asserts. Coverage unchanged: 100% line + function on TcpStream.c + Static.c. All 55 tests still green; full debug suite 17/17 green. Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: S28.05 SolidSyslogLwipRawTcpStream + integrating-lwip.md Closes #465. Housekeeping bundle closing the S28.05 PR: - docs/integrating-lwip.md new file. Covers NO_SYS=1 vs NO_SYS=0, the tcpip_callback() threading rule, lwipopts.h expectations (LWIP_RAW / LWIP_UDP / LWIP_TCP / ARP_QUEUEING / LWIP_TCP_KEEPALIVE / TCP_MSS / PBUF_POOL_SIZE / MEMP_NUM_TCP_PCB), a bare-metal NO_SYS=1 wiring example, per-adapter design notes (pbuf-strategy split UDP REF / TCP COPY, synchronous-Open spin, RX queue, tcp_close-after-tcp_err encapsulation), and the SolidSyslog tunables relevant to lwIP integrators. - CLAUDE.md gains two rows for SolidSyslogLwipRawTcpStream.h and SolidSyslogLwipRawTcpStreamErrors.h, slotted next to the other TCP-stream rows. - Top-level CMakeLists.txt STATUS strings for SOLIDSYSLOG_FREERTOS_NET =LWIP and =BOTH update to "Address+Resolver+Datagram+TcpStream only; BDD arrives in S28.06". The comment block above (the deferred PR #464 nit per project_s28_04_pr464_deferred_cmake_comment) is brought into sync. - DEVLOG entry capturing the five locked-in design calls (TCP_WRITE_FLAG_COPY, spin-with-injected-sleep, tcp_close-after-tcp_err encapsulation, bounded RX queue, SOF_KEEPALIVE), the commit shape, the LwipTcpFake's default-fires-cb behaviour that simplifies happy-path tests, and the new tunables/error source. - Pre-PR three-step: - IWYU on touched files (clang-19, freertos-host preset). Two findings applied: LwipTcpFake.h drops transitive lwip/arch.h + lwip/err.h; TcpStream.c drops SolidSyslogStreamDefinition.h + lwip/ip_addr.h and adds the public SolidSyslogLwipRawTcpStream.h header. Test file adjusts include set. - clang-format -i on every touched .c/.h/.cpp. - cppcheck-misra surfaced eight findings on the new TcpStream code. Six were resolved by code refactor (READ_FAILED moved into Read, RxQueueCount > 0U, pointer-arith → array indexing in DrainHeadBytes, == true on the IndexIsValid predicate, plus a new LwipRawTcpStream_SelfFromArg helper that concentrates the three lwIP-callback void*-arg casts into a single named site). Two new suppressions added under D.002 — 11.3 on SelfFromBase and 11.5 on SelfFromArg — matching the sibling-backend precedent. - docs/misra-deviations.md D.002 gets a sub-section (c) explicitly documenting the third-party-callback void*-arg pattern that the suppression file has been treating as D.002 since S08.07's MbedTls landed. Closes a pre-existing audit-trail gap and gives my new SelfFromArg entry the documented backing it needs. - scripts/misra_renumber.py extended to include the Platform/LwipRaw tree (-IPlatform/LwipRaw/Interface + Platform/LwipRaw/Source/). Oversight from S28.03; the script was previously blind to LwipRaw findings, which made the AMBIGUOUS-noise output during this PR's pre-push checks impossible to interpret. Pre-existing AMBIGUOUS entries on other rules are unchanged — they're orthogonal stale- suppression noise. - S28-05-HANDOFF.md removed (planning artefact carried from the session start; its job is done now that #465 is closing). Coverage: 100% line + function on Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c and SolidSyslogLwipRawTcpStreamStatic.c (Messages.c stays at 0%, matching the S28.04 Datagram established pattern — exercised only via integrator-installed error handlers). Full debug suite 17/17 green; the 55 TcpStream tests are inside SolidSyslogLwipRawTcpStreamTest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: S28.05 PR #466 review — IWYU fwd-decl, tidy, CodeRabbit Three review-feedback fixes bundled into one commit: - IWYU: restore the `struct SolidSyslogAddress;` forward declaration in `Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c`. The earlier IWYU pass dropped it while adding `struct SolidSyslogStream;` — I misread "should add" as "replace" instead of additive. CI's IWYU lane caught it; local re-run on the freertos-host preset now clean. Both forward decls are needed (Open takes Address, the vtable methods take Stream). - analyze-tidy-freertos-plustcp (5 errors on Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp): - `[[nodiscard]]` on `sendBytes` and `readBytes` per modernize-use-nodiscard. Two test sites that didn't capture the return get explicit `(void)` casts. - `static` on `pushIncomingPbuf` / `pushPeerFin` / `pushTcpErr` per readability-convert-member-functions-to-static — none access `this`, all delegate to the LwipTcpFake/LwipPbufFake free functions. - `NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)` on the one `const_cast<void*>(data)` inside `pushIncomingPbuf` — pbuf's payload field is `void*`; callers pass string literals, so the cast is structural, not a const-strip. - CodeRabbit two inline comments, both valid: - NOLINTBEGIN/NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) wrapper around the three test macros (`CHECK_IS_FALLBACK`, `CHECK_REPORTED`, `CHECK_FORWARDED_PCB`). Matches the established project convention in DEVLOG ("wrap with NOLINTBEGIN and use do { ... } while (0) for safe single-statement use"). I forgot the wrapper. - Extract the composite condition `(tcpConnectError == ERR_OK) && connectCallbackFires && (connected != NULL)` in `tcp_connect` (LwipTcpFake.c) into a `ShouldFireConnectCallback` static-inline predicate. Matches the project's "intent-naming static-inline predicates" pattern. Tests: 55/55 TcpStream tests green; full debug suite 17/17 green; clang-tidy now clean on the freertos-plustcp preset locally. Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: S28.05 PR #466 review — clang-format + MISRA line drift Two CI follow-ups on top of 5d579c2: - analyze-format: clang-format in the cpputest CI container reformatted `CHECK_FORWARDED_PCB` from a one-line `#define` onto two lines with a line-continuation backslash. My local clang-format-19 thought the one-liner was fine; the cpputest image's version disagrees. Apply the two-line form by hand — matches the visual style of the other CHECK_* macros above. - analyze-cppcheck: the IWYU fwd-decl I added in 5d579c2 shifted the SelfFromBase + SelfFromArg lines down by one. Bump the matching `misra_suppressions.txt` entries (11.3 :110→:111, 11.5 :119→:120) to match. Tests still 55/55 green; cppcheck-misra clean on `Platform/LwipRaw/Source/` locally. Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: S28.05 PR #466 — clang-format off/on around CHECK_FORWARDED_PCB Workaround for a divergent clang-format binary between containers: my freertos-host devcontainer (cpputest-freertos image) ships clang-format-19 and is happy with both the one-line and two-line forms of the macro; CI's analyze-format runs in the cpputest image and ships a different clang-format that wants a different alignment (first try: failed at col 95 on the one-liner; second try: failed at col 36 on the backslash position of the two-line form). Bracket the macro with `// clang-format off` / `// clang-format on` markers so neither version reformats it. The macro is a single expression — formatting it manually is fine. Follow-up to investigate: align the clang-format binaries across cpputest / cpputest-clang / cpputest-freertos images so dev and CI agree, or run analyze-format inside cpputest-freertos so the lwIP-touching files don't see a tooling mismatch. Part of #465. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Story S28.04 — UDP datagram adapter for the lwIP Raw API. Fourth
story in E28 (parent #439); follow-on to S28.03's Address +
Resolver pair. Closes #463.
Change Description
Implements
SolidSyslogLwipRawDatagrammirroring theSolidSyslogPlusTcpDatagramshape — three-TU pool split + Messages.cfor the error source. New tunable
SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE(default
1U).Design decisions (DEVLOG 2026-05-27 has the full record):
pbufvia PBUF_REF + per-sendpbuf_alloc/pbuf_free. Zero-copysend —
pbuf->payloadpoints at the caller's buffer. Discussedmid-commit-3 whether to embed a
struct pbufin our datagram andskip
pbuf_allocentirely (MISRA-flavoured); concluded PBUF_REF isthe right call because lwIP's
udp_sendtointernally allocates aheader pbuf regardless, so the asymmetry doesn't pay for the
manual-init complexity.
ARP_QUEUEING=1(lwIP default) keeps the first datagram alive. Wrapper stays
netif-agnostic.
ERR_OK→SENT; any othererr_t→FAILED.No
OVERSIZE(lwIP has no clean signal;ServicehonoursMaxPayload()upstream).MaxPayload()= 1232.SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOADunconditionally, same as PlusTcp. No netif MTU lookup.
Test-shape work worth highlighting (applicable to every future
pool-allocated adapter):
TEST_BASE+ twoTEST_GROUP_BASEderivations(
SolidSyslogLwipRawDatagramCreated-only;…Openwith Open liftedinto setup) so most lifecycle tests drop the boilerplate
Opencall.LwipUdpFake_OutstandingPcbCountand
LwipPbufFake_OutstandingPbufCountare asserted zero after everytest. Earned its keep mid-commit-3 — the pbuf invariant caught the
missing
pbuf_freebefore the explicit "frees pbuf" test landed.sendBytes(size_t length = 1U)helper + sharedsendBuffermember —most SendTo tests reduce to a single helper call.
CI gap addressed. The
analyze-cppchecklane did not previouslyscan
Platform/LwipRaw/Source/— an oversight from S28.03's landing.This PR adds
-IPlatform/LwipRaw/Interface+Platform/LwipRaw/Source/to all three cppcheck invocations and bundles the suppressions for the
four S28.03-era findings (
LwipRawAddressPrivate.h11.2/11.3 ×2;LwipRawAddressStatic.c11.3 ×2) alongside the two S28.04 findings(
LwipRawDatagram.c:43SelfFromBase 11.3;LwipRawDatagram.c:94pbuf->payloadconst-strip 11.8).docs/misra-deviations.mdD.006gains a sub-section for the lwIP
pbuf->payloadsite as the second"forced by an external interface" entry alongside the Winsock
select()case.Test Evidence
TDD cadence — 27 new red→green→refactor cycles across the three
commits that drove behaviour. Distinct groups in the final test file:
SolidSyslogLwipRawDatagram— 8 Created-only tests (Create, Destroyrelease, Open success, Close-without-open no-op, Open-calls-udp_new,
MaxPayload, SendTo-fails-before-open, Open-returns-false-on-udp_new-fail)
SolidSyslogLwipRawDatagramOpen— 19 already-opened tests (Closesemantics, idempotence, SendTo happy path, SendTo argument
forwarding, SendTo failure modes, Destroy lifecycle)
SolidSyslogLwipRawDatagramPool— 9 pool tests (mirror PlusTcp)Total 36 tests / 113 checks in
SolidSyslogLwipRawDatagramTest,all green locally.
Two new host-side fakes under
Tests/Support/LwipFakes/Source/:LwipUdpFake(udp_new/udp_remove/udp_sendto+ spies +failure injection +
OutstandingPcbCount) andLwipPbufFake(
pbuf_alloc/pbuf_free+ spies + alloc-fail injection +OutstandingPbufCount). Sources inlined into the test exe via aLWIP_FAKE_SOURCESvariable inTests/Lwip/CMakeLists.txt, matchingthe FreeRtosFakes / FatFsFakes precedent.
Local pre-PR check evidence
ctest --preset debug -j$(nproc)→ 16/16 PASSED (full suite)removes)
clang-formatclean (one minor reflow in the test file)CI sees it) — exit 0 after the suppressions land
SolidSyslogPlusTcpDatagramTestor other lifecycletests
Areas Affected
new pool tunable (additive; no existing production class touched)
registration
Source/directory created; two newfake files
tunable block (
SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE)doc updates per project convention
analyze-cppcheckextended to scanPlatform/LwipRaw/(three invocations updated symmetrically)suppressions; D.006 extended to cover the lwIP
pbuf->payloadconst-strip
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation