Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ CheckOptions:
# alternative. New Tier 2 helper structs join this list; Tier 1
# public tags continue to be subject to StructPrefix above.
# See docs/NAMING.md Tier 2 "file-scope struct tags".
- { key: readability-identifier-naming.StructIgnoredRegexp, value: '^(SolidSyslog|EscapedContext|BlockPresence|OpenHandle)$' }
- { key: readability-identifier-naming.StructIgnoredRegexp, value: '^(SolidSyslog|EscapedContext|BlockPresence|OpenHandle|LwipRawDatagramCall|LwipRawTcpStreamCall)$' }

# Tier 1 — public enum tags: SolidSyslogClass
- { key: readability-identifier-naming.EnumCase, value: CamelCase }
Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*`
| `SolidSyslogPlusTcpTcpStreamErrors.h` | Any code installing an error handler that wants to react to PlusTcpTcpStream-specific events (pointer-identity match on `PlusTcpTcpStreamErrorSource`, switch on `enum SolidSyslogPlusTcpTcpStreamErrors`) | `enum SolidSyslogPlusTcpTcpStreamErrors` (`PLUSTCPTCPSTREAM_ERROR_*` codes + `PLUSTCPTCPSTREAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource PlusTcpTcpStreamErrorSource`. Integrators ignore if not handling errors per source. |
| `SolidSyslogLwipRawTcpStream.h` | System setup code on **any target using lwIP Raw API** wiring a TCP sender (or a byte transport for `SolidSyslogTlsStream` / `SolidSyslogMbedTlsStream`) | `SolidSyslogLwipRawTcpStreamConfig` (`GetConnectTimeoutMs` + context + required `Sleep`), `SolidSyslogLwipRawTcpStream_Create(config)`, `_Destroy(base)` (wraps `tcp_new` / `tcp_connect` / `tcp_write` (`TCP_WRITE_FLAG_COPY`) / `tcp_output` / `tcp_recv` / `tcp_recved` / `tcp_close` / `tcp_abort`; integrator-supplied Sleep drives the bounded synchronous-Open spin loop; sets `SOF_KEEPALIVE` on every pcb; bounded RX pbuf queue via `SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE`; encapsulates lwIP's `tcp_close`-after-`tcp_err` use-after-free rule). Instance struct lives in a library-internal static pool (E11); default pool size 2 to support the TLS-over-plain-TCP pair. Pool-exhaustion and bad-config (`NULL` config or `NULL` Sleep) fallback is the shared `SolidSyslogNullStream`. See [`docs/integrating-lwip.md`](docs/integrating-lwip.md) for the full integrator guide. |
| `SolidSyslogLwipRawTcpStreamErrors.h` | Any code installing an error handler that wants to react to LwipRawTcpStream-specific events (pointer-identity match on `LwipRawTcpStreamErrorSource`, switch on `enum SolidSyslogLwipRawTcpStreamErrors`) | `enum SolidSyslogLwipRawTcpStreamErrors` (`LWIPRAWTCPSTREAM_ERROR_*` codes + `LWIPRAWTCPSTREAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource LwipRawTcpStreamErrorSource`. Integrators ignore if not handling errors per source. |
| `SolidSyslogLwipRawMarshal.h` | System setup code on **any target using lwIP Raw API** that needs to pin lwIP calls to the core-owning thread (`NO_SYS=0` integrators) | `SolidSyslogLwipRawCallback`, `SolidSyslogLwipRawMarshalFunction`, `SolidSyslogLwipRaw_SetMarshal(marshal)`. Routes the lwIP-touching Datagram/TcpStream adapter operations through one marshal hop per public operation (the Resolver's numeric `ipaddr_aton` parse touches no lwIP core state and is intentionally not marshalled). Default is a direct-call null marshal (correct for `NO_SYS=1`); `NO_SYS=0` integrators install a `tcpip_callback_with_block` shim or a `LOCK_TCPIP_CORE`/`UNLOCK_TCPIP_CORE` pair. The marshal MUST call its callback synchronously. Setting `marshal = NULL` restores the default. Single global slot — intended for setup-time configuration, not synchronised with concurrent installs. See [`docs/integrating-lwip.md`](docs/integrating-lwip.md). |
| `SolidSyslogWinsockTcpStreamErrors.h` | Any code installing an error handler that wants to react to WinsockTcpStream-specific events (pointer-identity match on `WinsockTcpStreamErrorSource`, switch on `enum SolidSyslogWinsockTcpStreamErrors`) | `enum SolidSyslogWinsockTcpStreamErrors` (`WINSOCKTCPSTREAM_ERROR_*` codes + `WINSOCKTCPSTREAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource WinsockTcpStreamErrorSource`. Integrators ignore if not handling errors per source. |
| `SolidSyslogUdpSender.h` | System setup code using UDP transport | `SolidSyslogUdpSenderConfig` (resolver, datagram, address, endpoint, endpointVersion — the integrator supplies one platform Address handle per sender from `SolidSyslog{Posix,Winsock,PlusTcp}Address_Create`), `SolidSyslogUdpSender_Create(config)`, `_Destroy(sender)`. Instance struct lives in a library-internal static pool (E11); `_Destroy` takes the handle returned by `_Create`. Pool-exhaustion and bad-config (including NULL Address) fallback is the shared `SolidSyslogNullSender`. |
| `SolidSyslogUdpSenderErrors.h` | Any code installing an error handler that wants to react to UdpSender-specific events (pointer-identity match on `UdpSenderErrorSource`, switch on `enum SolidSyslogUdpSenderErrors`) | `enum SolidSyslogUdpSenderErrors` (`UDPSENDER_ERROR_*` codes + `UDPSENDER_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource UdpSenderErrorSource`. Integrators ignore if not handling errors per source. |
Expand Down
56 changes: 56 additions & 0 deletions DEVLOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,61 @@
# Dev Log

## 2026-05-28 — S28.06 LwipRaw marshal injection

Sixth story under E28. Added `SolidSyslogLwipRaw_SetMarshal` — a
process-global seam that routes every lwIP Raw API call the adapters make
through one hop per public operation. Default is a direct-call null object
(correct for `NO_SYS=1`); `NO_SYS=0` integrators install a
`tcpip_callback_with_block` shim or a `LOCK_TCPIP_CORE` pair to pin the
calls to the lwIP-owning thread. Same single-global-slot shape as
`SolidSyslog_SetErrorHandler`. Motivation: the S28.05 guide's "marshal at
the SolidSyslog API boundary" advice was wrong — it dragged file I/O,
crypto, the buffer mutex, and the formatter work onto the tcpip thread. The
right boundary is the individual lwIP call.

The public header carries only the integrator setter; the internal
dispatch `SolidSyslogLwipRaw_Marshal` lives in a private header (David's
call when we paired on the API shape).

### Slicing deviated from the issue, deliberately

The issue's slice 1 ("arm the rail, existing tests go red, then green via a
tracking marshal — before any routing") can't actually go green: the rail
flag only flips inside the tracking marshal, which only runs once production
*routes* through the marshal. And `LwipPbufFake` is shared by the Datagram
and TcpStream test exes, so guarding it couples both wrappers into one
commit. Reordered to: (1) marshal TU with its own red/green/refactor; (2)
route both wrappers + arm the rail together, rail as the genuine red; (3)
Resolver no-op note; (4) docs. Same end state, every commit green. Agreed
the reorder with David up front.

### The rail

`LwipFakeMarshalGuard` arms every faked lwIP function (`udp_*`, `tcp_*`,
`pbuf_*`) to record a breach if called while no marshal is active. Couldn't
fail at the call site directly — the fakes are C and CppUTest aborts by
throwing/longjmp, which is UB through C frames. So the guard *records* the
first breach (with the fake's `__FILE__`/`__LINE__`) and a teardown
`CheckNoBreach()` fails in safe C++ context, still reporting the exact lwIP
call site. Any future refactor that drops a call site out of its marshal
auto-fails the relevant test.

### TcpStream Open keeps the spin on the caller's thread

Open marshals setup+`tcp_connect` in one hop, then the bounded connect spin
(`Sleep` between polls) runs on the *caller's* thread — sleeping the tcpip
thread mid-connect would starve RX/timers — then a `tcp_abort`-on-failure is
a second hop. Send/Read/Close take one hop each; the pure would-block Read
and the no-pcb Close take none. The lwIP callbacks (`connected`/`recv`/
`err`/`sent`) already run on the tcpip thread and touch no lwIP API, so
they're not marshalled.

### Resolver

`ipaddr_aton` is a pure parser touching no lwIP state — left a forward-
looking comment marking where the DnsResolver's `dns_gethostbyname` hop will
belong instead.

## 2026-05-28 — S24.13 IWYU lanes switched to advisory

Pivot story under E24. Started out scoped as "replace IWYU with
Expand Down
36 changes: 36 additions & 0 deletions Platform/LwipRaw/Interface/SolidSyslogLwipRawMarshal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifndef SOLIDSYSLOGLWIPRAWMARSHAL_H
#define SOLIDSYSLOGLWIPRAWMARSHAL_H

#include "ExternC.h"

EXTERN_C_BEGIN

/* Every lwIP Raw API call the SolidSyslog wrappers make is routed through
a single marshal hop so an integrator can pin those calls to the thread
that owns the lwIP core.

NO_SYS=1 (bare metal, no RTOS): the default direct-call marshal is
correct — there is one execution context and no core to protect.

NO_SYS=0 (RTOS with a tcpip thread): the integrator installs a marshal
that hops onto the tcpip thread — e.g. tcpip_callback_with_block, or a
LOCK_TCPIP_CORE / UNLOCK_TCPIP_CORE pair.

Contract: the marshal MUST invoke its callback synchronously — before
the marshal function returns. The wrapper reads results the callback
writes immediately after the hop, so an asynchronous marshal is caller
error. tcpip_callback_with_block(.., block=1) honours this; a bare
tcpip_callback(..) does not. */
typedef void (*SolidSyslogLwipRawCallback)(void* context);
typedef void (*SolidSyslogLwipRawMarshalFunction)(SolidSyslogLwipRawCallback callback, void* context);

/* Installs a process-global marshal. There is one lwIP instance and one
tcpip thread per process, so a single global slot suffices — same shape
as SolidSyslog_SetErrorHandler. Passing NULL restores the direct-call
default. Intended for setup-time configuration; not synchronised with
concurrent installs. */
void SolidSyslogLwipRaw_SetMarshal(SolidSyslogLwipRawMarshalFunction marshal);

EXTERN_C_END

#endif /* SOLIDSYSLOGLWIPRAWMARSHAL_H */
87 changes: 73 additions & 14 deletions Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,25 @@
#include "SolidSyslogDatagram.h"
#include "SolidSyslogDatagramDefinition.h"
#include "SolidSyslogLwipRawAddressPrivate.h"
#include "SolidSyslogLwipRawMarshalPrivate.h"
#include "SolidSyslogNullDatagram.h"
#include "SolidSyslogUdpPayload.h"

struct SolidSyslogAddress;

/* Per-operation parameters carried across one marshal hop. Only the fields
* the in-flight op needs are set (Open/Close set just Self); SendTo fills the
* rest. One struct per class so the void*-context recovery has a single cast
* site (LwipRawDatagramCallFromContext) — see D.002 in docs/misra-deviations.md. */
struct LwipRawDatagramCall
{
struct SolidSyslogLwipRawDatagram* Self;
const struct SolidSyslogLwipRawAddress* Dst;
const void* Buffer;
size_t Size;
enum SolidSyslogDatagramSendResult Result;
};

static bool LwipRawDatagram_Open(struct SolidSyslogDatagram* base);
static void LwipRawDatagram_Close(struct SolidSyslogDatagram* base);
static enum SolidSyslogDatagramSendResult LwipRawDatagram_SendTo(
Expand All @@ -26,7 +40,11 @@ static enum SolidSyslogDatagramSendResult LwipRawDatagram_SendTo(
static size_t LwipRawDatagram_MaxPayload(struct SolidSyslogDatagram* base);

static inline struct SolidSyslogLwipRawDatagram* LwipRawDatagram_SelfFromBase(struct SolidSyslogDatagram* base);
static inline struct LwipRawDatagramCall* LwipRawDatagramCallFromContext(void* context);
static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagram* self);
static void LwipRawDatagram_DoOpen(void* context);
static void LwipRawDatagram_DoClose(void* context);
static void LwipRawDatagram_DoSendTo(void* context);

void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base)
{
Expand All @@ -43,6 +61,15 @@ static inline struct SolidSyslogLwipRawDatagram* LwipRawDatagram_SelfFromBase(st
return (struct SolidSyslogLwipRawDatagram*) base;
}

/* Recovers the per-op call struct from the void* context the marshal passes
* back into each Do* callback. Single named helper so the void→struct cast
* lives in one place — one suppression site per class, not one per callback
* (the marshal-seam analogue of LwipRawTcpStream_SelfFromArg; see D.002). */
static inline struct LwipRawDatagramCall* LwipRawDatagramCallFromContext(void* context)
{
return (struct LwipRawDatagramCall*) context;
}

void LwipRawDatagram_Cleanup(struct SolidSyslogDatagram* base)
{
LwipRawDatagram_Close(base);
Expand All @@ -56,21 +83,35 @@ static void LwipRawDatagram_Close(struct SolidSyslogDatagram* base)
struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base);
if (LwipRawDatagram_IsOpen(self))
{
udp_remove(self->Pcb);
self->Pcb = NULL;
struct LwipRawDatagramCall call = {.Self = self};
SolidSyslogLwipRaw_Marshal(LwipRawDatagram_DoClose, &call);
}
}

static void LwipRawDatagram_DoClose(void* context)
{
struct SolidSyslogLwipRawDatagram* self = LwipRawDatagramCallFromContext(context)->Self;
udp_remove(self->Pcb);
self->Pcb = NULL;
}

static bool LwipRawDatagram_Open(struct SolidSyslogDatagram* base)
{
struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base);
if (!LwipRawDatagram_IsOpen(self))
{
self->Pcb = udp_new();
struct LwipRawDatagramCall call = {.Self = self};
SolidSyslogLwipRaw_Marshal(LwipRawDatagram_DoOpen, &call);
}
return LwipRawDatagram_IsOpen(self);
}

static void LwipRawDatagram_DoOpen(void* context)
{
struct SolidSyslogLwipRawDatagram* self = LwipRawDatagramCallFromContext(context)->Self;
self->Pcb = udp_new();
}

static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagram* self)
{
return self->Pcb != NULL;
Expand All @@ -87,20 +128,38 @@ static enum SolidSyslogDatagramSendResult LwipRawDatagram_SendTo(
enum SolidSyslogDatagramSendResult result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED;
if (LwipRawDatagram_IsOpen(self))
{
struct pbuf* p = pbuf_alloc(PBUF_TRANSPORT, (u16_t) size, PBUF_REF);
if (p != NULL)
struct LwipRawDatagramCall call = {
.Self = self,
.Dst = SolidSyslogLwipRawAddress_AsConst(addr),
.Buffer = buffer,
.Size = size,
.Result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED,
};
SolidSyslogLwipRaw_Marshal(LwipRawDatagram_DoSendTo, &call);
result = call.Result;
}
return result;
}

/* Runs the whole send — pbuf alloc, sendto, free — in one marshalled hop so
* a NO_SYS=0 integrator pays a single tcpip-thread context switch per Send
* rather than three. PBUF_REF points lwIP at the caller's buffer; the buffer
* outlives the synchronous hop, so no copy is needed. */
static void LwipRawDatagram_DoSendTo(void* context)
{
struct LwipRawDatagramCall* call = LwipRawDatagramCallFromContext(context);
call->Result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED;
struct pbuf* p = pbuf_alloc(PBUF_TRANSPORT, (u16_t) call->Size, PBUF_REF);
if (p != NULL)
Comment on lines +152 to +153

@coderabbitai coderabbitai Bot May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE" >&2
  exit 1
fi

echo "=== File: $FILE ==="
wc -l "$FILE"

echo "--- Surrounding lines 120-170 ---"
nl -ba "$FILE" | sed -n '120,170p'

echo "--- Find pbuf_alloc usage(s) ---"
rg -n "pbuf_alloc\(" "$FILE" || true

echo "--- Find LwipRawDatagram_DoSendTo ---"
rg -n "LwipRawDatagram_DoSendTo" "$FILE" || true

echo "--- Find Size type/assignments for SendToCall ---"
rg -n "struct LwipRawDatagram_SendToCall|SendToCall|call->Size" "$FILE" || true

echo "--- Find SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD definition/usages ---"
rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD" -S . || true

Repository: DavidCozens/solid-syslog

Length of output: 288


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c"
test -f "$FILE" || { echo "Missing file: $FILE" >&2; exit 1; }

echo "=== File: $FILE ==="
wc -l "$FILE"

echo "--- Surrounding lines 110-170 ---"
cat -n "$FILE" | sed -n '110,170p'

echo "--- Find pbuf_alloc usage(s) ---"
rg -n "pbuf_alloc\(" "$FILE" || true

echo "--- Find LwipRawDatagram_DoSendTo ---"
rg -n "LwipRawDatagram_DoSendTo" "$FILE" || true

echo "--- Find SendToCall struct and call->Size usage ---"
rg -n "LwipRawDatagram_SendToCall" "$FILE" || true
rg -n "call->Size" "$FILE" || true

echo "--- Find SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD definition/usages ---"
rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD" -S "$FILE" || true
rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD" -S . || true

Repository: DavidCozens/solid-syslog

Length of output: 6328


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the SendToCall struct + types in the target file
FILE="Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c"
echo "--- SendToCall struct (around line ~1-60) ---"
cat -n "$FILE" | sed -n '1,80p'

echo "--- SolidSyslogDatagram/UDP payload interface references in Core ---"
rg -n "SolidSyslogDatagram_MaxPayload|MaxPayload\(" Core Platform Tests -S || true

echo "--- Find call sites of datagram SendTo ---"
rg -n "SolidSyslogDatagram_SendTo\(" -S . || true
rg -n "SendTo\(" Platform/ Core/ -S || true

echo "--- Find where size is produced/trimmed for UDP payload ---"
rg -n "Trim|FromMtu|MaxPayload|UDP_IPV6_SAFE_PAYLOAD" Core Platform Tests -S || true

Repository: DavidCozens/solid-syslog

Length of output: 35125


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed_range () {
  local file="$1"
  local start="$2"
  local end="$3"
  echo "--- $file:$start-$end ---"
  cat -n "$file" | sed -n "${start},${end}p"
}

sed_range "Core/Source/SolidSyslogUdpSender.c" 150 260
sed_range "Core/Source/SolidSyslogUdpPayload.c" 1 120

sed_range "Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp" 1 120
sed_range "Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp" 130 220
sed_range "Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp" 360 450

Repository: DavidCozens/solid-syslog

Length of output: 18309


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE_TEST="Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp"
echo "--- Search for pbuf alloc length assertions in Lwip test ---"
rg -n "LastAllocLength|PbufFake_LastAllocLength|PbufAlloc|pbuf_alloc|sendBytes\(" "$FILE_TEST" || true

echo "--- Show sections around any LastAllocLength usage ---"
rg -n "LastAllocLength|PbufFake_LastAllocLength" "$FILE_TEST" -n || true
# Print a few likely ranges
cat -n "$FILE_TEST" | sed -n '240,340p'
cat -n "$FILE_TEST" | sed -n '80,140p'

FILE_FAKE="Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp"
# also inspect lwip pbuf fake header/impl for how it stores last alloc len
FILE_PBUF_FAKE_DIR="Tests/Support/LwipFakes"
echo "--- Search in Lwip fakes for PbufFake_LastAllocLength and pbuf_alloc length ---"
rg -n "LastAllocLength|pbuf_alloc|PbufAlloc" "$FILE_PBUF_FAKE_DIR" || true

# print lwip pbuf fake implementation sections
ls -R "$FILE_PBUF_FAKE_DIR" | head -n 50
fd -i "PbufFake" "$FILE_PBUF_FAKE_DIR" || true
fd -i "lwip.*pbuf" "$FILE_PBUF_FAKE_DIR" || true

Repository: DavidCozens/solid-syslog

Length of output: 8528


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show enum SolidSyslogDatagramSendResult to understand semantics
rg -n "enum SolidSyslogDatagramSendResult" -S Core/Interface Core/Source Platform/ -g'*.h' -g'*.c' || true
sed -n '1,120p' Core/Interface/SolidSyslogDatagram.h || true

# Inspect Posix + Winsock implementations for OVERSIZE handling
for f in \
  Platform/Posix/Source/SolidSyslogPosixDatagram.c \
  Platform/Windows/Source/SolidSyslogWinsockDatagram.c \
  Platform/PlusTcp/Source/SolidSyslogPlusTcpDatagram.c \
  Core/Source/SolidSyslogNullDatagram.c
do
  echo "=== $f: sendto logic (rough range) ==="
  test -f "$f" || continue
  rg -n "OVERSIZE|EMSGSIZE|WSAEMSGSIZE|sendto|sendBytes|SendTo" "$f" -S || true
done

# Also inspect the SolidSyslogDatagram_SendTo wrapper
sed -n '1,120p' Core/Source/SolidSyslogDatagram.c || true

Repository: DavidCozens/solid-syslog

Length of output: 6935


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "SolidSyslogDatagram_SendTo|Datagram_SendTo|MaxPayload\(|MaxPayload.*SendTo|SendTo.*MaxPayload|oversize" -S . \
  --glob='*.md' --glob='*.h' --glob='*.c' --glob='*.cpp' --glob='*.hpp' --glob='*.hpp.in' || true

rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD|UINT16_MAX|u16_t" -S Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c Core/Interface/Core/Source --glob='*.c' --glob='*.h' || true

Repository: DavidCozens/solid-syslog

Length of output: 17536


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show relevant docs sections
sed -n '260,360p' docs/integrating-lwip.md
sed -n '400,520p' docs/misra-deviations.md
sed -n '280,370p' DEVLOG.md
sed -n '300,340p' docs/misra-deviations.md
sed -n '120,190p' docs/bdd.md

Repository: DavidCozens/solid-syslog

Length of output: 19640


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect SolidSyslogUdpSender around where it computes/uses `size` for SendTo
FILE="Core/Source/SolidSyslogUdpSender.c"
echo "--- $FILE: show earlier transmit call site context ---"
cat -n "$FILE" | sed -n '1,280p'

echo "--- $FILE: search for SOLIDSYSLOG_MAX_MESSAGE_SIZE and max payload usage ---"
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE|MaxPayload|trim|oversize|clipLimit|trimmed" "$FILE" || true

# 2) Find and inspect SOLIDSYSLOG_MAX_MESSAGE_SIZE definition
echo "--- Search for SOLIDSYSLOG_MAX_MESSAGE_SIZE definition ---"
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE" -S . || true
# Show the definition file(s) near first match
for f in $(rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE" -S . | cut -d: -f1 | sort -u); do
  echo "=== $f ==="
  sed -n '1,120p' "$f" | cat -n | head -n 120
done

# 3) Search for any extremely large literals close to u16_t range
echo "--- Search for large numeric constants near 65535/u16 max ---"
rg -n "65535|UINT16_MAX|u16_t|cast.*u16_t|\\(u16_t\\)" -S Core Platform Tests --glob='*.c' --glob='*.h' || true

# 4) Inspect oversize-related UDP sender tests for what message sizes they use
FILE_TEST="Tests/SolidSyslogUdpSenderTest.cpp"
echo "--- $FILE_TEST: inspect oversize tests ---"
rg -n "Oversize|EMSGSIZE|MaxPayload|Retry" "$FILE_TEST" || true
# Print ranges around likely oversize tests
cat -n "$FILE_TEST" | sed -n '560,780p'

# 5) Inspect docs statements about the contract: whether SendTo is called with size<=MaxPayload
echo "--- docs/integrating-lwip.md around mentioned guarantee line ---"
rg -n "guarantee.*SolidSyslogDatagram_SendTo" docs/integrating-lwip.md || true
# show around line ~311 if exists
cat -n docs/integrating-lwip.md | sed -n '280,340p'

echo "--- DEVLOG.md around the note about honours MaxPayload before reaching SendTo ---"
rg -n "honours `MaxPayload\\(\\)`|MaxPayload\\(\\) before reaching SendTo|before reaching SendTo" DEVLOG.md || true
cat -n DEVLOG.md | sed -n '280,360p'

Repository: DavidCozens/solid-syslog

Length of output: 50453


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- Search Core for MaxPayload usage outside datagram backends ---"
rg -n "MaxPayload\(" Core/Source -S || true

echo "--- Inspect SolidSyslog_Service path (search for Sender->Send call site) ---"
rg -n "SolidSyslog_Service\\(|\\.Send\\(|Sender_.*Send\\(|Send\\(" Core/Source/SolidSyslog.c Core/Source -S || true

echo "--- Inspect SolidSyslogUdpSender integration (where buffer size passed into Sender) ---"
rg -n "SolidSyslogUdpSender|UdpSender_Send\\(" Core/Source -S || true

echo "--- Check tunable constraints for SOLIDSYSLOG_MAX_MESSAGE_SIZE vs UINT16 ---"
# show defaults
sed -n '1,120p' Core/Interface/SolidSyslogTunablesDefaults.h
# also look for other tunables headers that `#define` SOLIDSYSLOG_MAX_MESSAGE_SIZE
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE" Core/Interface -S || true

echo "--- Search for UINT16_MAX / u16 constraints anywhere ---"
rg -n "UINT16_MAX|65535|u16_t" Core/Interface Core/Source -S --glob='*.h' --glob='*.c' || true

echo "--- Locate docs statement about Service honoring MaxPayload before SendTo (integrating-lwip / misra-deviations / devlog) ---"
for f in docs/integrating-lwip.md docs/misra-deviations.md DEVLOG.md; do
  echo "=== $f ==="
  rg -n "honours .*MaxPayload|MaxPayload\\(\\).*before reaching SendTo|before reaching SendTo|already honours" "$f" -S || true
done

Repository: DavidCozens/solid-syslog

Length of output: 15914


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect SolidSyslog_Service logic around where it computes len and calls Sender_Send
FILE="Core/Source/SolidSyslog.c"
echo "--- $FILE: SolidSyslog_Service around send call sites ---"
rg -n "SolidSyslog_Service\\(|Sender_Send\\(" "$FILE"
sed -n '180,330p' "$FILE"

# 2) Look for any use of MaxPayload() in SolidSyslog.c (or call stack)
echo "--- $FILE: search for MaxPayload in SolidSyslog.c ---"
rg -n "MaxPayload\\(" "$FILE" || true

# 3) Check SOLIDSYSLOG_MAX_MESSAGE_SIZE compile-time bounds relative to u16_t/UINT16_MAX (repo-wide)
echo "--- Search for SOLIDSYSLOG_MAX_MESSAGE_SIZE upper bound / UINT16_MAX checks ---"
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE.*(65535|UINT16_MAX|u16_t)|((65535|UINT16_MAX).*(SOLIDSYSLOG_MAX_MESSAGE_SIZE))" -S . --glob='*.h' --glob='*.c' --glob='*.cpp' --glob='*.hpp' || true

# 4) Search for other casts from size_t to u16_t (to see established pattern/assumptions)
echo "--- Search for casts to u16_t in C code ---"
rg -n "\\(u16_t\\)" -S Core Platform Tests --glob='*.c' --glob='*.h' || true

# 5) Confirm integrating-lwip claims about Service honoring MaxPayload
echo "--- integrating-lwip.md: search for honours MaxPayload / before reaching SendTo ---"
rg -n "honours .*MaxPayload|MaxPayload\\(\\) before reaching SendTo|before reaching SendTo" docs/integrating-lwip.md DEVLOG.md docs -S || true

Repository: DavidCozens/solid-syslog

Length of output: 5871


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Global search for u16 bounds / truncation risk
rg -n "65535|UINT16_MAX|UINT16|u16_t.*MAX|MAX_MESSAGE_SIZE.*655|MAX_MESSAGE_SIZE.*UINT16" -S . --glob='*.h' --glob='*.c' --glob='*.cpp' --glob='*.hpp' || true

echo "--- Inspect SOLIDSYSLOG_MAX_MESSAGE_SIZE compile-time checks ---"
# show the whole tunables defaults header around the MAX_MESSAGE_SIZE definition area
sed -n '1,120p' Core/Interface/SolidSyslogTunablesDefaults.h

echo "--- Find all occurrences of SOLIDSYSLOG_MAX_MESSAGE_SIZE in Core/Interface (for any additional checks) ---"
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE" Core/Interface -S || true

echo "--- Inspect SolidSyslogTunables.h umbrella (if it adds extra checks) ---"
if [ -f Core/Interface/SolidSyslogTunables.h ]; then
  sed -n '1,160p' Core/Interface/SolidSyslogTunables.h
else
  echo "Missing Core/Interface/SolidSyslogTunables.h"
fi

echo "--- Inspect any message-length type conversions (size_t->uint16_t/u16_t) besides lwIP/TCP stream ---"
rg -n "\\(u16_t\\)|\\(uint16_t\\)" -S Core Platform --glob='*.c' --glob='*.h' || true

echo "--- Inspect CMake validation for SOLIDSYSLOG_MAX_MESSAGE_SIZE ---"
rg -n "SOLIDSYSLOG_MAX_MESSAGE_SIZE" CMakeLists.txt -S || true
sed -n '160,260p' CMakeLists.txt

Repository: DavidCozens/solid-syslog

Length of output: 12499


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- DEVLOG.md around line 300-330 ---"
sed -n '280,340p' DEVLOG.md | cat -n

echo "--- docs/integrating-lwip.md around the 'guarantee SolidSyslogDatagram_SendTo already provides' line (~311) ---"
rg -n "guarantee .*SolidSyslogDatagram_SendTo" docs/integrating-lwip.md
LINE="$(rg -n "guarantee .*SolidSyslogDatagram_SendTo" docs/integrating-lwip.md | head -n1 | cut -d: -f1)"
START=$((LINE-60))
END=$((LINE+60))
sed -n "${START},${END}p" docs/integrating-lwip.md | cat -n

echo "--- docs/integrating-lwip.md around 'SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD' unconditionally (~416) ---"
rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD" docs/integrating-lwip.md
LINE2="$(rg -n "SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD" docs/integrating-lwip.md | head -n1 | cut -d: -f1)"
START2=$((LINE2-40))
END2=$((LINE2+80))
sed -n "${START2},${END2}p" docs/integrating-lwip.md | cat -n

Repository: DavidCozens/solid-syslog

Length of output: 12545


Prevent silent truncation of the pbuf_alloc length cast to u16_t.

SolidSyslogLwipRawDatagram_DoSendTo casts call->Size to u16_t in pbuf_alloc(..., (u16_t) call->Size, ...) (line 142). If call->Size > UINT16_MAX, the value can wrap and lwIP may allocate/send the wrong length. Add a bound check against UINT16_MAX (and keep call->Result as FAILED/return early) before the cast.

🤖 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/LwipRaw/Source/SolidSyslogLwipRawDatagram.c` around lines 142 - 143,
SolidSyslogLwipRawDatagram_DoSendTo currently casts call->Size to u16_t for
pbuf_alloc which can wrap if call->Size > UINT16_MAX; add a guard before calling
pbuf_alloc that checks if call->Size > UINT16_MAX, set call->Result to FAILED
(or appropriate failure constant) and return early to avoid truncation, then
proceed to call pbuf_alloc with the safe cast when the size is within bounds.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declined as out-of-scope. This (u16_t) cast is pre-existing (S28.04 baseline), not introduced here — the redesign just moved the line. It's lwIP-specific (pbuf_alloc takes u16_t; the Posix/PlusTcp datagram siblings pass size_t to their socket APIs so have no such cast), and size is contract-bounded by Datagram_MaxPayload (~1232 B IPv6-safe) with UdpSender running an oversize-retry path above it, so a size > UINT16_MAX datagram cannot reach here. Not changing it in this PR to keep scope to the marshal seam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidCozens, understood — the contract layering (caller-side MaxPayloadUdpSender oversize-retry → SendTo) means an out-of-range value cannot reach the cast, and it's a pre-existing S28.04 baseline pattern rather than anything introduced here. I'll note this for future reviews.

Would you like me to resolve this comment?

(>_<) 🐇


✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 470
File: Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:152-153
Timestamp: 2026-05-28T22:43:31.454Z
Learning: In DavidCozens/solid-syslog, `LwipRawDatagram_DoSendTo` (Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c) casts `call->Size` to `u16_t` for `pbuf_alloc(PBUF_TRANSPORT, (u16_t) call->Size, PBUF_REF)`. This cast is safe and intentional: `size` is contract-bounded by `Datagram_MaxPayload()` (~1232 B, IPv6-safe), and `UdpSender` enforces an oversize-retry path above `SendTo`, so a value exceeding `UINT16_MAX` cannot reach this code path. The cast is a pre-existing S28.04 baseline pattern (not introduced by the marshal-seam refactor) required by the lwIP `pbuf_alloc` API signature. Do not flag this cast as a truncation risk in future reviews.

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: 283
File: Platform/OpenSsl/Source/SolidSyslogTlsStream.c:363-363
Timestamp: 2026-05-07T08:22:59.856Z
Learning: In this solid-syslog codebase, follow the documented contract: do not add NULL/defensive checks inside library/internal code paths (e.g., `_Create` functions) when the NULL case cannot occur under normal internal usage and is covered by framework or internal wiring guarantees. Only add validation at true system boundaries such as user input and external APIs. Where setup-time wiring contracts between the integrator and the library require specific function-pointer fields (e.g., `SolidSyslogTlsStreamConfig.sleep`), assume those required fields are present and intentionally omit NULL guards in internal creation functions.

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 291
File: Example/FreeRtos/HelloWorld/CmsdkUart.c:17-42
Timestamp: 2026-05-08T17:19:38.382Z
Learning: In this codebase (DavidCozens/solid-syslog), internal driver/config/access structs may intentionally store raw pointers without NULL checks, `isInitialized` flags, or defensive struct copies. Treat this as an explicit project policy: do not flag store-by-pointer usage or missing NULL/initialization guards for these internal config/access structs when the pointer is provided by production callers and is guaranteed to point to `static const` objects with program lifetime (per CLAUDE.md). Only flag NULL/initialization issues when the pointer originates from potentially invalid/external/untrusted sources or when the code does not establish lifetime/guarantees.

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: 372
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosDatagram.c:0-0
Timestamp: 2026-05-15T13:09:11.811Z
Learning: When reviewing PRs that bulk-rename identifiers (especially “static function rename” changes) in this repo, check for unintended renames inside FreeRTOS macro usages. A too-broad regex can incorrectly prefix macro tokens used in `static const` initializers (e.g., `pdMS_TO_TICKS` from FreeRTOS). Look for signs like a renamed macro name being incorrectly prefixed (e.g., `FreeRtosDatagram_pdMS_TO_TICKS`), and if present, treat it as evidence the identifier-extraction regex was overly broad and requires correction/revert of the affected rename changes.

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: 411
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosDatagramStatic.c:42-50
Timestamp: 2026-05-20T12:23:42.200Z
Learning: In the solid-syslog (DavidCozens) codebase, review pool-backed `_Destroy` implementations to ensure they *uniformly* warn on any handle not present in the relevant pool (the established E11 pattern). This intentionally covers: (1) stale/already-destroyed handles, (2) never-issued handles, and (3) the shared-null-sentinel returned on pool exhaustion. Do not ask for a special-case early-exit/silent no-op when the handle equals the shared null sentinel—integrators already receive an error at `_Create` time on pool exhaustion, and the `_Destroy` warning is the deliberate secondary safety net. If a change to this behavior is proposed (e.g., suppressing warnings / changing null-sentinel semantics), it must be applied epic-wide and consistently across all migrated classes/platform variants, not as a per-class or per-platform deviation.

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 416
File: DEVLOG.md:155-155
Timestamp: 2026-05-20T17:56:08.543Z
Learning: When a source file needs the public facility/severity enums used by `SolidSyslog_Error` (or other public APIs), include the public header `Core/Interface/SolidSyslogPrival.h` rather than `Core/Source/SolidSyslogPrivate.h`. Reserve `Core/Source/SolidSyslogPrivate.h` (TU-internal structs like `SolidSyslog`) for the translation unit that defines/uses them (e.g., `SolidSyslogStatic.c`).

{
p->payload = (void*) call->Buffer;
err_t err = udp_sendto(call->Self->Pcb, p, &call->Dst->Ip, call->Dst->Port);
(void) pbuf_free(p);
if (err == ERR_OK)
{
const struct SolidSyslogLwipRawAddress* dst = SolidSyslogLwipRawAddress_AsConst(addr);
p->payload = (void*) buffer;
err_t err = udp_sendto(self->Pcb, p, &dst->Ip, dst->Port);
(void) pbuf_free(p);
if (err == ERR_OK)
{
result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT;
}
call->Result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT;
}
}
return result;
}

static size_t LwipRawDatagram_MaxPayload(struct SolidSyslogDatagram* base)
Expand Down
27 changes: 27 additions & 0 deletions Platform/LwipRaw/Source/SolidSyslogLwipRawMarshal.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include "SolidSyslogLwipRawMarshalPrivate.h"

#include <stddef.h>

static void Marshal_NullMarshal(SolidSyslogLwipRawCallback callback, void* context)
{
callback(context);
}

static SolidSyslogLwipRawMarshalFunction currentMarshal = Marshal_NullMarshal;

void SolidSyslogLwipRaw_SetMarshal(SolidSyslogLwipRawMarshalFunction marshal)
{
if (marshal == NULL)
{
currentMarshal = Marshal_NullMarshal;
}
else
{
currentMarshal = marshal;
}
}

void SolidSyslogLwipRaw_Marshal(SolidSyslogLwipRawCallback callback, void* context)
{
currentMarshal(callback, context);
}
17 changes: 17 additions & 0 deletions Platform/LwipRaw/Source/SolidSyslogLwipRawMarshalPrivate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#ifndef SOLIDSYSLOGLWIPRAWMARSHALPRIVATE_H
#define SOLIDSYSLOGLWIPRAWMARSHALPRIVATE_H

#include "ExternC.h"
#include "SolidSyslogLwipRawMarshal.h"

EXTERN_C_BEGIN

/* Library-internal dispatch call site. The LwipRaw wrapper classes batch
the lwIP API calls for one public operation into a single callback and
hand it here; we run it through the currently-installed marshal. No NULL
check — the null-object default direct-calls. */
void SolidSyslogLwipRaw_Marshal(SolidSyslogLwipRawCallback callback, void* context);

EXTERN_C_END

#endif /* SOLIDSYSLOGLWIPRAWMARSHALPRIVATE_H */
5 changes: 5 additions & 0 deletions Platform/LwipRaw/Source/SolidSyslogLwipRawResolver.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ static bool LwipRawResolver_Resolve(
(void) base;
(void) transport;
struct SolidSyslogLwipRawAddress* self = SolidSyslogLwipRawAddress_As(result);
/* No SolidSyslogLwipRaw_Marshal hop here: ipaddr_aton is a pure string
* parser that touches no lwIP core state, so it is safe on any thread.
* The sibling SolidSyslogLwipRawDnsResolver (S28.08) WILL call lwIP DNS
* APIs (dns_gethostbyname) and must marshal them — that is where the hop
* belongs, not here. */
bool resolved = ipaddr_aton(host, &self->Ip) != 0;
if (resolved)
{
Expand Down
Loading
Loading