From fb767e88fccaaf6a128160e9fcab72e8d17652c5 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 16:54:35 +0000 Subject: [PATCH 1/8] chore: S28.05 LwipRawTcpStream pool plumbing + tunables + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- Core/Interface/SolidSyslogTunablesDefaults.h | 63 +++++ .../Interface/SolidSyslogLwipRawTcpStream.h | 26 ++ .../SolidSyslogLwipRawTcpStreamErrors.h | 21 ++ .../Source/SolidSyslogLwipRawTcpStream.c | 34 +++ .../SolidSyslogLwipRawTcpStreamMessages.c | 23 ++ .../SolidSyslogLwipRawTcpStreamPrivate.h | 33 +++ .../SolidSyslogLwipRawTcpStreamStatic.c | 95 +++++++ Tests/Lwip/CMakeLists.txt | 26 ++ .../Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 258 ++++++++++++++++++ 9 files changed, 579 insertions(+) create mode 100644 Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h create mode 100644 Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStreamErrors.h create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c create mode 100644 Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index de472072..3e934519 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -588,6 +588,69 @@ #error "SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogLwipRawTcpStream instances the library's internal + * static pool can simultaneously hold. Each instance carries a struct tcp_pcb + * pointer, the Connected / Errored flags, and a bounded RX pbuf ring sized + * by SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE. + * + * Default 2 — matches SOLIDSYSLOG_POSIX_TCP_STREAM_POOL_SIZE and the other + * TCP-stream-backend defaults so the canonical TLS-over-plain-TCP pair does + * not silently fall back to NullStream on the second Create. Bump via + * SOLIDSYSLOG_USER_TUNABLES_FILE if more are needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE +#define SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE 2U +#endif + +#if SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE < 1 +#error "SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE must be >= 1" +#endif + +/* + * Period (milliseconds) the SolidSyslogLwipRawTcpStream bounded-connect + * spin loop sleeps between polls of the lwIP-side connected_cb flag. + * Each iteration calls the integrator-injected SolidSyslogSleepFunction + * so the loop never busy-waits — under NO_SYS=1 the integrator's Sleep + * implementation ticks sys_check_timeouts and drives RX; under NO_SYS=0 + * it yields to the tcpip thread (vTaskDelay or equivalent). + * + * Default 10 ms gives 20 polls inside the default 200 ms connect deadline. + * + * Floor: 1 ms. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS +#define SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS 10U +#endif + +#if SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS < 1 +#error "SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS must be >= 1" +#endif + +/* + * Maximum number of struct pbuf* the SolidSyslogLwipRawTcpStream RX queue + * holds before backpressuring lwIP. Bounds the *count* of queued pbufs, + * not their byte volume — lwIP's TCP_WND and MEMP_NUM_PBUF cap upstream + * receive bytes; this knob caps how many segment-sized pbufs can pile up + * behind a slow Stream_Read drain before the tcp_recv callback returns + * non-ERR_OK so lwIP retains the pbuf and replays the callback later. + * + * Default 8 — sized for the typical mTLS handshake flight (ServerHello + + * Certificate + ServerKeyExchange + ServerHelloDone is 2-4 segments; 8 + * leaves margin for cert chains and renegotiation traffic). + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE +#define SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE 8U +#endif + +#if SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE < 1 +#error "SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE must be >= 1" +#endif + /* * Number of SolidSyslogPlusTcpTcpStream instances the library's * internal static pool can simultaneously hold. Each instance carries diff --git a/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h new file mode 100644 index 00000000..08d4e272 --- /dev/null +++ b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h @@ -0,0 +1,26 @@ +#ifndef SOLIDSYSLOGLWIPRAWTCPSTREAM_H +#define SOLIDSYSLOGLWIPRAWTCPSTREAM_H + +#include "ExternC.h" +#include "SolidSyslogSleep.h" +#include "SolidSyslogTcpConnectTimeoutFunction.h" + +EXTERN_C_BEGIN + + struct SolidSyslogStream; + + struct SolidSyslogLwipRawTcpStreamConfig + { + SolidSyslogTcpConnectTimeoutFunction + GetConnectTimeoutMs; /* NULL → use SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS tunable */ + void* ConnectTimeoutContext; /* passed through to GetConnectTimeoutMs; NULL is fine */ + SolidSyslogSleepFunction + Sleep; /* required — drives the bounded-connect spin; NULL config falls back to NullStream */ + }; + + struct SolidSyslogStream* SolidSyslogLwipRawTcpStream_Create(const struct SolidSyslogLwipRawTcpStreamConfig* config); + void SolidSyslogLwipRawTcpStream_Destroy(struct SolidSyslogStream * base); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGLWIPRAWTCPSTREAM_H */ diff --git a/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStreamErrors.h b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStreamErrors.h new file mode 100644 index 00000000..6bc7825d --- /dev/null +++ b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStreamErrors.h @@ -0,0 +1,21 @@ +#ifndef SOLIDSYSLOGLWIPRAWTCPSTREAMERRORS_H +#define SOLIDSYSLOGLWIPRAWTCPSTREAMERRORS_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogErrorSource; + + enum SolidSyslogLwipRawTcpStreamErrors + { + LWIPRAWTCPSTREAM_ERROR_POOL_EXHAUSTED, + LWIPRAWTCPSTREAM_ERROR_UNKNOWN_DESTROY, + LWIPRAWTCPSTREAM_ERROR_MAX + }; + + extern const struct SolidSyslogErrorSource LwipRawTcpStreamErrorSource; + +EXTERN_C_END + +#endif /* SOLIDSYSLOGLWIPRAWTCPSTREAMERRORS_H */ diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c new file mode 100644 index 00000000..0bc19be2 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c @@ -0,0 +1,34 @@ +#include "SolidSyslogLwipRawTcpStreamPrivate.h" + +#include +#include + +#include "SolidSyslogNullStream.h" +#include "SolidSyslogStream.h" +#include "SolidSyslogStreamDefinition.h" + +static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base); + +void LwipRawTcpStream_Initialise( + struct SolidSyslogStream* base, + const struct SolidSyslogLwipRawTcpStreamConfig* config +) +{ + static const struct SolidSyslogLwipRawTcpStream DefaultLwipRawTcpStream = {0}; + + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); + *self = DefaultLwipRawTcpStream; + self->Config = *config; +} + +static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base) +{ + return (struct SolidSyslogLwipRawTcpStream*) base; +} + +void LwipRawTcpStream_Cleanup(struct SolidSyslogStream* base) +{ + /* Overwrite the abstract base with the shared NullStream vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullStream_Get(); +} diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c new file mode 100644 index 00000000..dc3054e0 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c @@ -0,0 +1,23 @@ +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogLwipRawTcpStreamErrors.h" + +static const char* LwipRawTcpStreamError_AsString(uint8_t code) +{ + static const char* const messages[LWIPRAWTCPSTREAM_ERROR_MAX] = { + [LWIPRAWTCPSTREAM_ERROR_POOL_EXHAUSTED] = + "SolidSyslogLwipRawTcpStream_Create pool exhausted; returning fallback NullStream", + [LWIPRAWTCPSTREAM_ERROR_UNKNOWN_DESTROY] = + "SolidSyslogLwipRawTcpStream_Destroy called with a handle not issued by this pool", + }; + const char* result = "unknown"; + if (code < (uint8_t) LWIPRAWTCPSTREAM_ERROR_MAX) + { + enum SolidSyslogLwipRawTcpStreamErrors typed = (enum SolidSyslogLwipRawTcpStreamErrors) code; + result = messages[typed]; + } + return result; +} + +const struct SolidSyslogErrorSource LwipRawTcpStreamErrorSource = {"LwipRawTcpStream", LwipRawTcpStreamError_AsString}; diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h new file mode 100644 index 00000000..6e539669 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h @@ -0,0 +1,33 @@ +#ifndef SOLIDSYSLOGLWIPRAWTCPSTREAMPRIVATE_H +#define SOLIDSYSLOGLWIPRAWTCPSTREAMPRIVATE_H + +#include +#include + +#include "SolidSyslogLwipRawTcpStream.h" +#include "SolidSyslogStreamDefinition.h" +#include "SolidSyslogTunables.h" + +struct tcp_pcb; +struct pbuf; + +struct SolidSyslogLwipRawTcpStream +{ + struct SolidSyslogStream Base; + struct SolidSyslogLwipRawTcpStreamConfig Config; + struct tcp_pcb* Pcb; + bool Connected; + bool Errored; + struct pbuf* RxQueue[SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE]; + size_t RxQueueHead; + size_t RxQueueCount; + size_t RxHeadOffset; +}; + +void LwipRawTcpStream_Initialise( + struct SolidSyslogStream* base, + const struct SolidSyslogLwipRawTcpStreamConfig* config +); +void LwipRawTcpStream_Cleanup(struct SolidSyslogStream* base); + +#endif /* SOLIDSYSLOGLWIPRAWTCPSTREAMPRIVATE_H */ diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c new file mode 100644 index 00000000..69b4a7b0 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c @@ -0,0 +1,95 @@ +#include "SolidSyslogLwipRawTcpStream.h" + +#include +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogLwipRawTcpStreamErrors.h" +#include "SolidSyslogLwipRawTcpStreamPrivate.h" +#include "SolidSyslogNullStream.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogStream; + +static inline bool LwipRawTcpStream_IsValidConfig(const struct SolidSyslogLwipRawTcpStreamConfig* config); +static inline size_t LwipRawTcpStream_IndexFromHandle(const struct SolidSyslogStream* base); +static inline void LwipRawTcpStream_CleanupAtIndex(size_t index, void* context); + +static bool LwipRawTcpStream_InUse[SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE]; +static struct SolidSyslogLwipRawTcpStream LwipRawTcpStream_Pool[SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE]; +static struct SolidSyslogPoolAllocator LwipRawTcpStream_Allocator = { + LwipRawTcpStream_InUse, + SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE +}; + +struct SolidSyslogStream* SolidSyslogLwipRawTcpStream_Create(const struct SolidSyslogLwipRawTcpStreamConfig* config) +{ + struct SolidSyslogStream* handle = SolidSyslogNullStream_Get(); + if (LwipRawTcpStream_IsValidConfig(config)) + { + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&LwipRawTcpStream_Allocator); + if (SolidSyslogPoolAllocator_IndexIsValid(&LwipRawTcpStream_Allocator, index)) + { + LwipRawTcpStream_Initialise(&LwipRawTcpStream_Pool[index].Base, config); + handle = &LwipRawTcpStream_Pool[index].Base; + } + else + { + SolidSyslog_Error( + SOLIDSYSLOG_SEVERITY_ERROR, + &LwipRawTcpStreamErrorSource, + (uint8_t) LWIPRAWTCPSTREAM_ERROR_POOL_EXHAUSTED + ); + } + } + return handle; +} + +void SolidSyslogLwipRawTcpStream_Destroy(struct SolidSyslogStream* base) +{ + size_t index = LwipRawTcpStream_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&LwipRawTcpStream_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse( + &LwipRawTcpStream_Allocator, + index, + LwipRawTcpStream_CleanupAtIndex, + NULL + ); + if (!released) + { + SolidSyslog_Error( + SOLIDSYSLOG_SEVERITY_WARNING, + &LwipRawTcpStreamErrorSource, + (uint8_t) LWIPRAWTCPSTREAM_ERROR_UNKNOWN_DESTROY + ); + } +} + +static inline bool LwipRawTcpStream_IsValidConfig(const struct SolidSyslogLwipRawTcpStreamConfig* config) +{ + return (config != NULL) && (config->Sleep != NULL); +} + +static inline size_t LwipRawTcpStream_IndexFromHandle(const struct SolidSyslogStream* base) +{ + size_t result = SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE; poolIndex++) + { + if (base == &LwipRawTcpStream_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static inline void LwipRawTcpStream_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + LwipRawTcpStream_Cleanup(&LwipRawTcpStream_Pool[index].Base); +} diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index 0eaa1af5..f72b9dab 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -138,3 +138,29 @@ set_target_properties(SolidSyslogLwipRawDatagramTest PROPERTIES ) add_test(NAME SolidSyslogLwipRawDatagramTest COMMAND SolidSyslogLwipRawDatagramTest) + +# ---- SolidSyslogLwipRawTcpStreamTest ---- + +add_executable(SolidSyslogLwipRawTcpStreamTest + SolidSyslogLwipRawTcpStreamTest.cpp + main.cpp + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c +) + +target_link_libraries(SolidSyslogLwipRawTcpStreamTest PRIVATE + ${PROJECT_NAME} + ConfigLockFake + ErrorHandlerFake + CppUTest + CppUTestExt +) + +target_include_directories(SolidSyslogLwipRawTcpStreamTest PRIVATE ${LWIP_TEST_INCLUDE_DIRS}) + +set_target_properties(SolidSyslogLwipRawTcpStreamTest PROPERTIES + CXX_CPPCHECK "" +) + +add_test(NAME SolidSyslogLwipRawTcpStreamTest COMMAND SolidSyslogLwipRawTcpStreamTest) diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp new file mode 100644 index 00000000..ca763eb1 --- /dev/null +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -0,0 +1,258 @@ +#include +#include + +#include "TestUtils.h" +#include "CppUTest/TestHarness.h" + +using namespace CososoTesting; + +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" +#include "SolidSyslogLwipRawTcpStream.h" +#include "SolidSyslogLwipRawTcpStreamErrors.h" +#include "SolidSyslogNullStream.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogStream.h" +#include "SolidSyslogStreamDefinition.h" +#include "SolidSyslogTunables.h" + +// Asserts handle is non-null and not one of the slots in pool. +#define CHECK_IS_FALLBACK(handle, pool) \ + do \ + { \ + CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr"); \ + for (auto* slot : (pool)) \ + { \ + CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)"); \ + CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \ + } \ + } while (0) + +// Asserts the most recent ErrorHandlerFake call matched (severity, source, code). +#define CHECK_REPORTED(severity, source, code) \ + do \ + { \ + CALLED_FAKE(ErrorHandlerFake_Handle, ONCE); \ + LONGS_EQUAL((severity), ErrorHandlerFake_LastSeverity()); \ + POINTERS_EQUAL(&(source), ErrorHandlerFake_LastSource()); \ + UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastCode()); \ + } while (0) + +namespace +{ +int FakeSleep_CallCount = 0; +int FakeSleep_LastMs = 0; + +void FakeSleep_Reset() +{ + FakeSleep_CallCount = 0; + FakeSleep_LastMs = 0; +} + +extern "C" void FakeSleep(int milliseconds) +{ + FakeSleep_CallCount++; + FakeSleep_LastMs = milliseconds; +} +} // namespace + +// clang-format off +TEST_GROUP(SolidSyslogLwipRawTcpStream) +{ + struct SolidSyslogLwipRawTcpStreamConfig validConfig{}; + struct SolidSyslogStream* stream = nullptr; + + void setup() override + { + FakeSleep_Reset(); + validConfig = {}; + validConfig.Sleep = FakeSleep; + stream = SolidSyslogLwipRawTcpStream_Create(&validConfig); + } + + void teardown() override + { + if (stream != nullptr) + { + SolidSyslogLwipRawTcpStream_Destroy(stream); + } + } +}; +// clang-format on + +TEST(SolidSyslogLwipRawTcpStream, CreateReturnsNonNullStream) +{ + CHECK(stream != nullptr); +} + +TEST(SolidSyslogLwipRawTcpStream, CreatedStreamIsNotTheNullStreamSingleton) +{ + CHECK(stream != SolidSyslogNullStream_Get()); +} + +TEST(SolidSyslogLwipRawTcpStream, DestroyReleasesSlotToPool) +{ + SolidSyslogLwipRawTcpStream_Destroy(stream); + + stream = SolidSyslogLwipRawTcpStream_Create(&validConfig); + + CHECK(stream != SolidSyslogNullStream_Get()); +} + +TEST(SolidSyslogLwipRawTcpStream, CreateWithNullConfigReturnsFallback) +{ + struct SolidSyslogStream* fallback = SolidSyslogLwipRawTcpStream_Create(nullptr); + + POINTERS_EQUAL(SolidSyslogNullStream_Get(), fallback); +} + +TEST(SolidSyslogLwipRawTcpStream, CreateWithNullSleepReturnsFallback) +{ + struct SolidSyslogLwipRawTcpStreamConfig badConfig{}; + badConfig.Sleep = nullptr; + + struct SolidSyslogStream* fallback = SolidSyslogLwipRawTcpStream_Create(&badConfig); + + POINTERS_EQUAL(SolidSyslogNullStream_Get(), fallback); +} + +// clang-format off +TEST_GROUP(SolidSyslogLwipRawTcpStreamPool) +{ + struct SolidSyslogLwipRawTcpStreamConfig validConfig{}; + struct SolidSyslogStream* pooled[SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE] = {}; + struct SolidSyslogStream* overflow = nullptr; + + void setup() override + { + FakeSleep_Reset(); + validConfig = {}; + validConfig.Sleep = FakeSleep; + } + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogLwipRawTcpStream_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogLwipRawTcpStream_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogLwipRawTcpStream_Create(&validConfig); + } + } +}; +// clang-format on + +TEST(SolidSyslogLwipRawTcpStreamPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogLwipRawTcpStream_Create(&validConfig); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogLwipRawTcpStream_Create(&validConfig); + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_ERROR, LwipRawTcpStreamErrorSource, LWIPRAWTCPSTREAM_ERROR_POOL_EXHAUSTED); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, FallbackVtableMethodsAreNoOps) +{ + FillPool(); + overflow = SolidSyslogLwipRawTcpStream_Create(&validConfig); + char buffer[1] = {0}; + + /* NullStream's Open / Send return true so caller success paths are not + * tripped; Read returns 0 (would-block) so callers don't tear the + * connection down; Close is a no-op. */ + CHECK_TRUE(SolidSyslogStream_Open(overflow, nullptr)); + CHECK_TRUE(SolidSyslogStream_Send(overflow, "x", 1)); + LONGS_EQUAL(0, SolidSyslogStream_Read(overflow, buffer, sizeof(buffer))); + SolidSyslogStream_Close(overflow); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogLwipRawTcpStream_Create(&validConfig); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogLwipRawTcpStream_Create(&validConfig); + + LONGS_EQUAL(SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogLwipRawTcpStream_Create(&validConfig); + ConfigLockFake_Install(); + + SolidSyslogLwipRawTcpStream_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogStream stranger = {}; + + SolidSyslogLwipRawTcpStream_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogStream stranger = {}; + + SolidSyslogLwipRawTcpStream_Destroy(&stranger); + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_WARNING, LwipRawTcpStreamErrorSource, LWIPRAWTCPSTREAM_ERROR_UNKNOWN_DESTROY); +} + +TEST(SolidSyslogLwipRawTcpStreamPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogLwipRawTcpStream_Create(&validConfig); + struct SolidSyslogStream* stale = pooled[0]; + SolidSyslogLwipRawTcpStream_Destroy(pooled[0]); + pooled[0] = nullptr; + ErrorHandlerFake_Install(nullptr); + + SolidSyslogLwipRawTcpStream_Destroy(stale); + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_WARNING, LwipRawTcpStreamErrorSource, LWIPRAWTCPSTREAM_ERROR_UNKNOWN_DESTROY); +} From e3c86eb1dd8377e7729e4ac899e4cdb2e85e861d Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 17:17:42 +0000 Subject: [PATCH 2/8] feat: S28.05 LwipRawTcpStream Open/Close lifecycle + connect callback bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Source/SolidSyslogLwipRawTcpStream.c | 236 ++++++++++++- Tests/Lwip/CMakeLists.txt | 5 + .../Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 331 +++++++++++++++++- .../Support/LwipFakes/Interface/LwipTcpFake.h | 77 ++++ Tests/Support/LwipFakes/Source/LwipTcpFake.c | 288 +++++++++++++++ 5 files changed, 920 insertions(+), 17 deletions(-) create mode 100644 Tests/Support/LwipFakes/Interface/LwipTcpFake.h create mode 100644 Tests/Support/LwipFakes/Source/LwipTcpFake.c diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c index 0bc19be2..b21a326f 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c @@ -2,23 +2,89 @@ #include #include +#include +#include "lwip/arch.h" +#include "lwip/err.h" +#include "lwip/ip.h" +#include "lwip/ip_addr.h" +#include "lwip/tcp.h" +#include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogNullStream.h" #include "SolidSyslogStream.h" #include "SolidSyslogStreamDefinition.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogAddress; + +static uint32_t LwipRawTcpStream_NullConnectTimeoutGetter(void* context); + +static bool LwipRawTcpStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr); +static void LwipRawTcpStream_Close(struct SolidSyslogStream* base); static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base); +static inline bool LwipRawTcpStream_ConfigProvidesGetter(const struct SolidSyslogLwipRawTcpStreamConfig* config); +static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStream* self); +static struct tcp_pcb* LwipRawTcpStream_OpenAndConfigurePcb(struct SolidSyslogLwipRawTcpStream* self); +static bool LwipRawTcpStream_ConnectOrAbortOnFailure( + struct SolidSyslogLwipRawTcpStream* self, + const struct SolidSyslogAddress* addr +); +static bool LwipRawTcpStream_TryConnect( + struct SolidSyslogLwipRawTcpStream* self, + const struct SolidSyslogAddress* addr +); +static bool LwipRawTcpStream_WaitForConnectedCallback(struct SolidSyslogLwipRawTcpStream* self); +static uint32_t LwipRawTcpStream_ResolveConnectTimeoutMs(struct SolidSyslogLwipRawTcpStream* self); +static void LwipRawTcpStream_AbortAndForgetPcb(struct SolidSyslogLwipRawTcpStream* self); +static void LwipRawTcpStream_ClosePcb(struct SolidSyslogLwipRawTcpStream* self); + +static err_t LwipRawTcpStream_ConnectedCallback(void* arg, struct tcp_pcb* pcb, err_t err); +static err_t LwipRawTcpStream_RecvCallback(void* arg, struct tcp_pcb* tpcb, struct pbuf* p, err_t err); +static err_t LwipRawTcpStream_SentCallback(void* arg, struct tcp_pcb* tpcb, u16_t len); +static void LwipRawTcpStream_ErrCallback(void* arg, err_t err); void LwipRawTcpStream_Initialise( struct SolidSyslogStream* base, const struct SolidSyslogLwipRawTcpStreamConfig* config ) { - static const struct SolidSyslogLwipRawTcpStream DefaultLwipRawTcpStream = {0}; + static const struct SolidSyslogLwipRawTcpStream DefaultLwipRawTcpStream = { + .Base = {.Open = LwipRawTcpStream_Open, .Send = NULL, .Read = NULL, .Close = LwipRawTcpStream_Close}, + .Config = {.GetConnectTimeoutMs = LwipRawTcpStream_NullConnectTimeoutGetter, + .ConnectTimeoutContext = NULL, + .Sleep = NULL}, + .Pcb = NULL, + .Connected = false, + .Errored = false, + .RxQueue = {0}, + .RxQueueHead = 0, + .RxQueueCount = 0, + .RxHeadOffset = 0, + }; struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); *self = DefaultLwipRawTcpStream; - self->Config = *config; + self->Config.Sleep = config->Sleep; + self->Config.ConnectTimeoutContext = config->ConnectTimeoutContext; + if (LwipRawTcpStream_ConfigProvidesGetter(config)) + { + self->Config.GetConnectTimeoutMs = config->GetConnectTimeoutMs; + } +} + +static inline bool LwipRawTcpStream_ConfigProvidesGetter(const struct SolidSyslogLwipRawTcpStreamConfig* config) +{ + return config->GetConnectTimeoutMs != NULL; +} + +/* Null Object substituted when the integrator does not install a getter — + * returns the compile-time tunable so the bounded-wait path has a single + * code path regardless of whether the integrator wired runtime tuning. */ +static uint32_t LwipRawTcpStream_NullConnectTimeoutGetter(void* context) +{ + (void) context; + return (uint32_t) SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS; } static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base) @@ -28,7 +94,173 @@ static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase( void LwipRawTcpStream_Cleanup(struct SolidSyslogStream* base) { + LwipRawTcpStream_Close(base); /* Overwrite the abstract base with the shared NullStream vtable so * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ *base = *SolidSyslogNullStream_Get(); } + +static bool LwipRawTcpStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr) +{ + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); + if (!LwipRawTcpStream_IsOpen(self)) + { + self->Pcb = LwipRawTcpStream_OpenAndConfigurePcb(self); + if (LwipRawTcpStream_IsOpen(self)) + { + (void) LwipRawTcpStream_ConnectOrAbortOnFailure(self, addr); + } + } + return LwipRawTcpStream_IsOpen(self); +} + +static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStream* self) +{ + return self->Pcb != NULL; +} + +static struct tcp_pcb* LwipRawTcpStream_OpenAndConfigurePcb(struct SolidSyslogLwipRawTcpStream* self) +{ + struct tcp_pcb* pcb = tcp_new(); + if (pcb != NULL) + { + ip_set_option(pcb, SOF_KEEPALIVE); + tcp_arg(pcb, self); + tcp_recv(pcb, LwipRawTcpStream_RecvCallback); + tcp_sent(pcb, LwipRawTcpStream_SentCallback); + tcp_err(pcb, LwipRawTcpStream_ErrCallback); + } + return pcb; +} + +static bool LwipRawTcpStream_ConnectOrAbortOnFailure( + struct SolidSyslogLwipRawTcpStream* self, + const struct SolidSyslogAddress* addr +) +{ + bool connected = LwipRawTcpStream_TryConnect(self, addr); + if (!connected) + { + LwipRawTcpStream_AbortAndForgetPcb(self); + } + return connected; +} + +static bool LwipRawTcpStream_TryConnect( + struct SolidSyslogLwipRawTcpStream* self, + const struct SolidSyslogAddress* addr +) +{ + const struct SolidSyslogLwipRawAddress* dst = SolidSyslogLwipRawAddress_AsConst(addr); + self->Connected = false; + self->Errored = false; + err_t connectErr = tcp_connect(self->Pcb, &dst->Ip, dst->Port, LwipRawTcpStream_ConnectedCallback); + bool ok = false; + if (connectErr == ERR_OK) + { + ok = LwipRawTcpStream_WaitForConnectedCallback(self); + } + return ok; +} + +/* Bounded synchronous-Open spin: each iteration sleeps via the + * integrator-injected Sleep so lwIP's timer / RX paths get cycles to + * advance the SYN/SYN-ACK exchange. Exits on Connected (success), + * Errored (set by connected_cb on non-ERR_OK or by tcp_err), or + * elapsed >= deadline (timeout). */ +static bool LwipRawTcpStream_WaitForConnectedCallback(struct SolidSyslogLwipRawTcpStream* self) +{ + const uint32_t pollMs = (uint32_t) SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS; + const uint32_t deadlineMs = LwipRawTcpStream_ResolveConnectTimeoutMs(self); + uint32_t elapsedMs = 0; + while (!self->Connected && !self->Errored && (elapsedMs < deadlineMs)) + { + self->Config.Sleep((int) pollMs); + elapsedMs += pollMs; + } + return self->Connected; +} + +/* Bridges the integrator-installed getter (or the Null Object substituted + * in Initialise) to the bounded spin deadline. Invoked on every connect + * attempt so a runtime-tunable value takes effect on the next reconnect. */ +static uint32_t LwipRawTcpStream_ResolveConnectTimeoutMs(struct SolidSyslogLwipRawTcpStream* self) +{ + return self->Config.GetConnectTimeoutMs(self->Config.ConnectTimeoutContext); +} + +static void LwipRawTcpStream_AbortAndForgetPcb(struct SolidSyslogLwipRawTcpStream* self) +{ + tcp_abort(self->Pcb); + self->Pcb = NULL; +} + +static void LwipRawTcpStream_Close(struct SolidSyslogStream* base) +{ + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); + LwipRawTcpStream_ClosePcb(self); +} + +/* tcp_close must NOT be called on a pcb that has already been released by + * tcp_err — that's a use-after-free in lwIP. The Pcb != NULL guard works + * because LwipRawTcpStream_ErrCallback nulls Pcb when lwIP releases the + * pcb on its side. */ +static void LwipRawTcpStream_ClosePcb(struct SolidSyslogLwipRawTcpStream* self) +{ + if (LwipRawTcpStream_IsOpen(self)) + { + (void) tcp_close(self->Pcb); + self->Pcb = NULL; + } +} + +static err_t LwipRawTcpStream_ConnectedCallback(void* arg, struct tcp_pcb* pcb, err_t err) +{ + (void) pcb; + struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + if (err == ERR_OK) + { + self->Connected = true; + } + else + { + self->Errored = true; + } + return ERR_OK; +} + +/* RX queue + tcp_recved drain (and pbuf_free of the head pbuf when drained) + * land in the Send/Read slice. For now the callback is a no-op stub — lwIP + * requires the slot wired before tcp_connect so the recv path is set up by + * the time the peer sends data, even though the wrapper's Stream_Read does + * not yet drain. */ +static err_t LwipRawTcpStream_RecvCallback(void* arg, struct tcp_pcb* tpcb, struct pbuf* p, err_t err) +{ + (void) arg; + (void) tpcb; + (void) p; + (void) err; + return ERR_OK; +} + +/* Real tcp_sent handling is unused under TCP_WRITE_FLAG_COPY (Commit 3 + * decision 1) — caller buffers are released at Send return, not at + * peer-ACK time. The callback exists because lwIP requires the slot set. */ +static err_t LwipRawTcpStream_SentCallback(void* arg, struct tcp_pcb* tpcb, u16_t len) +{ + (void) arg; + (void) tpcb; + (void) len; + return ERR_OK; +} + +/* lwIP fires tcp_err for fatal events (RST, OOM, ABRT) AFTER releasing the + * pcb upstream — we must null our Pcb pointer and NOT call tcp_close. + * Subsequent Stream_Close sees Pcb == NULL and is a safe no-op. */ +static void LwipRawTcpStream_ErrCallback(void* arg, err_t err) +{ + (void) err; + struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + self->Pcb = NULL; + self->Errored = true; +} diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index f72b9dab..07e5fce8 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -32,6 +32,7 @@ set(LWIP_TEST_INCLUDE_DIRS # pulled inline rather than wrapped as a static lib). set(LWIP_FAKE_SOURCES ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipPbufFake.c + ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipTcpFake.c ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipUdpFake.c ) @@ -144,6 +145,10 @@ add_test(NAME SolidSyslogLwipRawDatagramTest COMMAND SolidSyslogLwipRawDatagramT add_executable(SolidSyslogLwipRawTcpStreamTest SolidSyslogLwipRawTcpStreamTest.cpp main.cpp + ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipTcpFake.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddress.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddressMessages.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamMessages.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index ca763eb1..7d87076e 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -8,6 +8,9 @@ using namespace CososoTesting; #include "ConfigLockFake.h" #include "ErrorHandlerFake.h" +#include "LwipTcpFake.h" +#include "SolidSyslogLwipRawAddress.h" +#include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogLwipRawTcpStream.h" #include "SolidSyslogLwipRawTcpStreamErrors.h" #include "SolidSyslogNullStream.h" @@ -15,6 +18,12 @@ using namespace CososoTesting; #include "SolidSyslogStream.h" #include "SolidSyslogStreamDefinition.h" #include "SolidSyslogTunables.h" +#include "lwip/err.h" +#include "lwip/ip4_addr.h" +#include "lwip/ip_addr.h" +#include "lwip/tcp.h" + +static const uint16_t TEST_PORT = 514; // Asserts handle is non-null and not one of the slots in pool. #define CHECK_IS_FALLBACK(handle, pool) \ @@ -40,7 +49,7 @@ using namespace CososoTesting; namespace { -int FakeSleep_CallCount = 0; +unsigned FakeSleep_CallCount = 0; int FakeSleep_LastMs = 0; void FakeSleep_Reset() @@ -54,32 +63,91 @@ extern "C" void FakeSleep(int milliseconds) FakeSleep_CallCount++; FakeSleep_LastMs = milliseconds; } + +unsigned FakeGetConnectTimeoutMs_CallCount = 0; +void* FakeGetConnectTimeoutMs_LastContext = nullptr; +uint32_t FakeGetConnectTimeoutMs_ReturnValue = SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS; + +void FakeGetConnectTimeoutMs_Reset() +{ + FakeGetConnectTimeoutMs_CallCount = 0; + FakeGetConnectTimeoutMs_LastContext = reinterpret_cast(0x1U); + FakeGetConnectTimeoutMs_ReturnValue = SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS; +} + +extern "C" uint32_t FakeGetConnectTimeoutMs(void* context) +{ + FakeGetConnectTimeoutMs_CallCount++; + FakeGetConnectTimeoutMs_LastContext = context; + return FakeGetConnectTimeoutMs_ReturnValue; +} } // namespace +/* Shared fixture: every TcpStream lifecycle test needs the fakes reset, a + * fresh stream + address handle pair, teardown of both, and the leak + * invariant — every tcp_pcb handed out by tcp_new must come back via + * tcp_close / tcp_abort / null-via-tcp_err by the end of the test. */ // clang-format off -TEST_GROUP(SolidSyslogLwipRawTcpStream) +TEST_BASE(LwipRawTcpStreamTestBase) { - struct SolidSyslogLwipRawTcpStreamConfig validConfig{}; + struct SolidSyslogLwipRawTcpStreamConfig config{}; struct SolidSyslogStream* stream = nullptr; + struct SolidSyslogAddress* address = nullptr; - void setup() override + void createFakesAndHandles() { + LwipTcpFake_Reset(); FakeSleep_Reset(); - validConfig = {}; - validConfig.Sleep = FakeSleep; - stream = SolidSyslogLwipRawTcpStream_Create(&validConfig); + FakeGetConnectTimeoutMs_Reset(); + config = {}; + config.Sleep = FakeSleep; + stream = SolidSyslogLwipRawTcpStream_Create(&config); + address = SolidSyslogLwipRawAddress_Create(); + struct SolidSyslogLwipRawAddress* lwipAddress = SolidSyslogLwipRawAddress_As(address); + IP4_ADDR(&lwipAddress->Ip, 127, 0, 0, 1); + lwipAddress->Port = TEST_PORT; + } + + void destroyHandlesAndCheckNoLeak() const + { + SolidSyslogLwipRawAddress_Destroy(address); + SolidSyslogLwipRawTcpStream_Destroy(stream); + LONGS_EQUAL_TEXT(0, LwipTcpFake_OutstandingPcbCount(), "leaked tcp_pcb past teardown"); + } +}; + +TEST_GROUP_BASE(SolidSyslogLwipRawTcpStream, LwipRawTcpStreamTestBase) +{ + void setup() override + { + createFakesAndHandles(); } void teardown() override { - if (stream != nullptr) - { - SolidSyslogLwipRawTcpStream_Destroy(stream); - } + destroyHandlesAndCheckNoLeak(); + } +}; + +TEST_GROUP_BASE(SolidSyslogLwipRawTcpStreamConnected, LwipRawTcpStreamTestBase) +{ + void setup() override + { + createFakesAndHandles(); + SolidSyslogStream_Open(stream, address); + } + + void teardown() override + { + destroyHandlesAndCheckNoLeak(); } }; // clang-format on +/* ------------------------------------------------------------------ + * Created-but-not-opened tests + * ----------------------------------------------------------------*/ + TEST(SolidSyslogLwipRawTcpStream, CreateReturnsNonNullStream) { CHECK(stream != nullptr); @@ -94,7 +162,7 @@ TEST(SolidSyslogLwipRawTcpStream, DestroyReleasesSlotToPool) { SolidSyslogLwipRawTcpStream_Destroy(stream); - stream = SolidSyslogLwipRawTcpStream_Create(&validConfig); + stream = SolidSyslogLwipRawTcpStream_Create(&config); CHECK(stream != SolidSyslogNullStream_Get()); } @@ -116,6 +184,241 @@ TEST(SolidSyslogLwipRawTcpStream, CreateWithNullSleepReturnsFallback) POINTERS_EQUAL(SolidSyslogNullStream_Get(), fallback); } +TEST(SolidSyslogLwipRawTcpStream, CloseBeforeOpenIsNoOp) +{ + SolidSyslogStream_Close(stream); + + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); + CALLED_FAKE(LwipTcpFake_TcpAbort, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenCallsTcpNew) +{ + SolidSyslogStream_Open(stream, address); + + CALLED_FAKE(LwipTcpFake_TcpNew, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenReturnsTrueOnSuccessfulConnect) +{ + CHECK_TRUE(SolidSyslogStream_Open(stream, address)); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenReturnsFalseWhenTcpNewFails) +{ + LwipTcpFake_SetTcpNewFails(true); + + CHECK_FALSE(SolidSyslogStream_Open(stream, address)); + CALLED_FAKE(LwipTcpFake_TcpConnect, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenSetsKeepaliveOnPcb) +{ + SolidSyslogStream_Open(stream, address); + + CHECK((LwipTcpFake_LastTcpNewReturned()->so_options & SOF_KEEPALIVE) != 0); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenRegistersTcpArgRecvErrSentCallbacks) +{ + SolidSyslogStream_Open(stream, address); + + CALLED_FAKE(LwipTcpFake_TcpArg, ONCE); + CALLED_FAKE(LwipTcpFake_TcpRecv, ONCE); + CALLED_FAKE(LwipTcpFake_TcpErr, ONCE); + CALLED_FAKE(LwipTcpFake_TcpSent, ONCE); + CHECK(LwipTcpFake_LastRecvFn() != nullptr); + CHECK(LwipTcpFake_LastErrFn() != nullptr); + CHECK(LwipTcpFake_LastSentFn() != nullptr); + CHECK(LwipTcpFake_LastCallbackArg() != nullptr); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenCallsTcpConnectWithAddressIpAndPort) +{ + SolidSyslogStream_Open(stream, address); + + CALLED_FAKE(LwipTcpFake_TcpConnect, ONCE); + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastConnectPcb()); + POINTERS_EQUAL(&SolidSyslogLwipRawAddress_AsConst(address)->Ip, LwipTcpFake_LastConnectIpaddr()); + LONGS_EQUAL(TEST_PORT, LwipTcpFake_LastConnectPort()); + CHECK(LwipTcpFake_LastConnectedFn() != nullptr); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenReturnsFalseAndAbortsWhenConnectedCallbackFiresErrored) +{ + LwipTcpFake_SetConnectCallbackResult(ERR_RST); + + CHECK_FALSE(SolidSyslogStream_Open(stream, address)); + CALLED_FAKE(LwipTcpFake_TcpAbort, ONCE); + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenReturnsFalseAndAbortsOnImmediateTcpConnectError) +{ + LwipTcpFake_SetTcpConnectError(ERR_VAL); + + CHECK_FALSE(SolidSyslogStream_Open(stream, address)); + CALLED_FAKE(LwipTcpFake_TcpAbort, ONCE); + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenReturnsFalseAndAbortsOnConnectTimeout) +{ + LwipTcpFake_SetConnectCallbackFires(false); + + CHECK_FALSE(SolidSyslogStream_Open(stream, address)); + CALLED_FAKE(LwipTcpFake_TcpAbort, ONCE); + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenSleepsBetweenPollsDuringTimeoutPath) +{ + LwipTcpFake_SetConnectCallbackFires(false); + + SolidSyslogStream_Open(stream, address); + + /* timeout / poll periods → exactly that many sleeps before giving up. */ + LONGS_EQUAL(SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS / SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS, FakeSleep_CallCount); + LONGS_EQUAL(SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS, FakeSleep_LastMs); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenHappyPathDoesNotSleep) +{ + SolidSyslogStream_Open(stream, address); + + LONGS_EQUAL(0, FakeSleep_CallCount); +} + +TEST(SolidSyslogLwipRawTcpStream, OpenRespectsRuntimeTunableConnectTimeout) +{ + LwipTcpFake_SetConnectCallbackFires(false); + /* Re-create the stream with a getter installed. The default fixture's + * stream uses NULL getter (falls back to the compile-time tunable); + * we want to verify the getter is honoured on the timeout deadline. */ + SolidSyslogLwipRawTcpStream_Destroy(stream); + config.GetConnectTimeoutMs = FakeGetConnectTimeoutMs; + config.ConnectTimeoutContext = reinterpret_cast(0xABCDU); + FakeGetConnectTimeoutMs_ReturnValue = 20U; + stream = SolidSyslogLwipRawTcpStream_Create(&config); + + SolidSyslogStream_Open(stream, address); + + /* 20 ms / 10 ms poll = 2 polls. */ + LONGS_EQUAL(2, FakeSleep_CallCount); + CHECK(FakeGetConnectTimeoutMs_CallCount >= 1U); + POINTERS_EQUAL(reinterpret_cast(0xABCDU), FakeGetConnectTimeoutMs_LastContext); +} + +/* ------------------------------------------------------------------ + * Open-already-called tests + * ----------------------------------------------------------------*/ + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReopenDoesNotAllocateNewPcb) +{ + CHECK_TRUE(SolidSyslogStream_Open(stream, address)); + + CALLED_FAKE(LwipTcpFake_TcpNew, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, CloseCallsTcpCloseOnOpenPcb) +{ + SolidSyslogStream_Close(stream); + + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastClosePcb()); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SecondCloseDoesNotCloseAgain) +{ + SolidSyslogStream_Close(stream); + + SolidSyslogStream_Close(stream); + + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, CloseThenOpenAllocatesFreshPcb) +{ + SolidSyslogStream_Close(stream); + + SolidSyslogStream_Open(stream, address); + + CALLED_FAKE(LwipTcpFake_TcpNew, TWICE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyClosesOpenPcb) +{ + SolidSyslogLwipRawTcpStream_Destroy(stream); + stream = nullptr; + + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyAfterCloseDoesNotCloseAgain) +{ + SolidSyslogStream_Close(stream); + + SolidSyslogLwipRawTcpStream_Destroy(stream); + stream = nullptr; + + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, TcpErrCallbackReleasesPcbWithoutCallingTcpClose) +{ + /* Drive the err callback the wrapper registered. lwIP releases the + * pcb upstream before invoking err — the wrapper must null its Pcb + * field and NOT call tcp_close (use-after-free). */ + tcp_err_fn errCb = LwipTcpFake_LastErrFn(); + void* arg = LwipTcpFake_LastCallbackArg(); + errCb(arg, ERR_RST); + LwipTcpFake_NotePcbReleasedByErr(); + + SolidSyslogStream_Close(stream); + + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, RecvCallbackReturnsErrOkAsNoOpStub) +{ + /* Real RX queue handling lands in the Send/Read slice — this slot's + * stub returns ERR_OK so lwIP sees the bytes as accepted. */ + tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); + + LONGS_EQUAL( + ERR_OK, + recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), nullptr, ERR_OK) + ); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SentCallbackReturnsErrOkAsNoOpStub) +{ + /* TCP_WRITE_FLAG_COPY means caller buffers are released at Send return — + * no per-ACK accounting needed. The slot exists because lwIP wants the + * callback set when the pcb is wired. */ + tcp_sent_fn sentCb = LwipTcpFake_LastSentFn(); + + LONGS_EQUAL(ERR_OK, sentCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), 0)); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyAfterTcpErrDoesNotCallTcpClose) +{ + tcp_err_fn errCb = LwipTcpFake_LastErrFn(); + void* arg = LwipTcpFake_LastCallbackArg(); + errCb(arg, ERR_RST); + LwipTcpFake_NotePcbReleasedByErr(); + + SolidSyslogLwipRawTcpStream_Destroy(stream); + stream = nullptr; + + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +/* ------------------------------------------------------------------ + * Pool tests — handed-out handles never call lwIP, so they don't need + * the fake state. Same TEST_GROUP shape as Commit 1. + * ----------------------------------------------------------------*/ + // clang-format off TEST_GROUP(SolidSyslogLwipRawTcpStreamPool) { @@ -125,6 +428,7 @@ TEST_GROUP(SolidSyslogLwipRawTcpStreamPool) void setup() override { + LwipTcpFake_Reset(); FakeSleep_Reset(); validConfig = {}; validConfig.Sleep = FakeSleep; @@ -181,9 +485,6 @@ TEST(SolidSyslogLwipRawTcpStreamPool, FallbackVtableMethodsAreNoOps) overflow = SolidSyslogLwipRawTcpStream_Create(&validConfig); char buffer[1] = {0}; - /* NullStream's Open / Send return true so caller success paths are not - * tripped; Read returns 0 (would-block) so callers don't tear the - * connection down; Close is a no-op. */ CHECK_TRUE(SolidSyslogStream_Open(overflow, nullptr)); CHECK_TRUE(SolidSyslogStream_Send(overflow, "x", 1)); LONGS_EQUAL(0, SolidSyslogStream_Read(overflow, buffer, sizeof(buffer))); diff --git a/Tests/Support/LwipFakes/Interface/LwipTcpFake.h b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h new file mode 100644 index 00000000..5f38cd16 --- /dev/null +++ b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h @@ -0,0 +1,77 @@ +#ifndef LWIPTCPFAKE_H +#define LWIPTCPFAKE_H + +#include "ExternC.h" + +#include +#include + +#include "lwip/arch.h" +#include "lwip/err.h" +#include "lwip/ip_addr.h" +#include "lwip/tcp.h" + +EXTERN_C_BEGIN + + void LwipTcpFake_Reset(void); + + /* tcp_new configuration */ + void LwipTcpFake_SetTcpNewFails(bool fails); + + /* tcp_new spy */ + unsigned LwipTcpFake_TcpNewCallCount(void); + struct tcp_pcb* LwipTcpFake_LastTcpNewReturned(void); + + /* tcp_arg spy — last (pcb, arg) pair captured */ + unsigned LwipTcpFake_TcpArgCallCount(void); + void* LwipTcpFake_LastCallbackArg(void); + + /* tcp_recv / tcp_err / tcp_sent spies — last registered callback fn captured */ + unsigned LwipTcpFake_TcpRecvCallCount(void); + tcp_recv_fn LwipTcpFake_LastRecvFn(void); + unsigned LwipTcpFake_TcpErrCallCount(void); + tcp_err_fn LwipTcpFake_LastErrFn(void); + unsigned LwipTcpFake_TcpSentCallCount(void); + tcp_sent_fn LwipTcpFake_LastSentFn(void); + + /* tcp_connect configuration */ + + /* Immediate err returned by tcp_connect itself. Default ERR_OK. */ + void LwipTcpFake_SetTcpConnectError(int8_t err); + + /* Whether tcp_connect synchronously invokes the registered connected_cb + * before returning. Default true — happy-path successful connect. Set to + * false for tests that drive the timeout path (no callback fires). */ + void LwipTcpFake_SetConnectCallbackFires(bool fires); + + /* The err passed to the connected_cb when it fires. Default ERR_OK. */ + void LwipTcpFake_SetConnectCallbackResult(int8_t err); + + /* tcp_connect spy */ + unsigned LwipTcpFake_TcpConnectCallCount(void); + struct tcp_pcb* LwipTcpFake_LastConnectPcb(void); + const ip_addr_t* LwipTcpFake_LastConnectIpaddr(void); + uint16_t LwipTcpFake_LastConnectPort(void); + tcp_connected_fn LwipTcpFake_LastConnectedFn(void); + + /* tcp_close configuration */ + void LwipTcpFake_SetTcpCloseError(int8_t err); + + /* tcp_close spy */ + unsigned LwipTcpFake_TcpCloseCallCount(void); + struct tcp_pcb* LwipTcpFake_LastClosePcb(void); + + /* tcp_abort spy */ + unsigned LwipTcpFake_TcpAbortCallCount(void); + struct tcp_pcb* LwipTcpFake_LastAbortPcb(void); + + /* Allocated-but-not-yet-freed PCB count. Successful tcp_new bumps it; + * tcp_close / tcp_abort decrement. The tcp_err callback releases the + * pcb upstream — tests that fire it via LwipTcpFake_LastErrFn must call + * LwipTcpFake_NotePcbReleasedByErr() to keep the leak invariant honest. */ + int LwipTcpFake_OutstandingPcbCount(void); + void LwipTcpFake_NotePcbReleasedByErr(void); + +EXTERN_C_END + +#endif /* LWIPTCPFAKE_H */ diff --git a/Tests/Support/LwipFakes/Source/LwipTcpFake.c b/Tests/Support/LwipFakes/Source/LwipTcpFake.c new file mode 100644 index 00000000..bddaaee5 --- /dev/null +++ b/Tests/Support/LwipFakes/Source/LwipTcpFake.c @@ -0,0 +1,288 @@ +#include "LwipTcpFake.h" + +#include + +#include "lwip/arch.h" +#include "lwip/err.h" +#include "lwip/ip_addr.h" +#include "lwip/tcp.h" + +static unsigned tcpNewCallCount = 0; +static struct tcp_pcb fakePcb; +static struct tcp_pcb* lastTcpNewReturned = NULL; +static bool tcpNewFails = false; + +static unsigned tcpArgCallCount = 0; +static void* lastCallbackArg = NULL; + +static unsigned tcpRecvCallCount = 0; +static tcp_recv_fn lastRecvFn = NULL; +static unsigned tcpErrCallCount = 0; +static tcp_err_fn lastErrFn = NULL; +static unsigned tcpSentCallCount = 0; +static tcp_sent_fn lastSentFn = NULL; + +static unsigned tcpConnectCallCount = 0; +static struct tcp_pcb* lastConnectPcb = NULL; +static const ip_addr_t* lastConnectIpaddr = NULL; +static u16_t lastConnectPort = 0; +static tcp_connected_fn lastConnectedFn = NULL; +static err_t tcpConnectError = ERR_OK; +static bool connectCallbackFires = true; +static err_t connectCallbackResult = ERR_OK; + +static unsigned tcpCloseCallCount = 0; +static struct tcp_pcb* lastClosePcb = NULL; +static err_t tcpCloseError = ERR_OK; + +static unsigned tcpAbortCallCount = 0; +static struct tcp_pcb* lastAbortPcb = NULL; + +static int outstandingPcbCount = 0; + +void LwipTcpFake_Reset(void) +{ + tcpNewCallCount = 0; + /* Zero the fake pcb so so_options / state writes from prior tests don't + * leak into the current one. */ + fakePcb = (struct tcp_pcb){0}; + lastTcpNewReturned = NULL; + tcpNewFails = false; + + tcpArgCallCount = 0; + lastCallbackArg = NULL; + + tcpRecvCallCount = 0; + lastRecvFn = NULL; + tcpErrCallCount = 0; + lastErrFn = NULL; + tcpSentCallCount = 0; + lastSentFn = NULL; + + tcpConnectCallCount = 0; + lastConnectPcb = NULL; + lastConnectIpaddr = NULL; + lastConnectPort = 0; + lastConnectedFn = NULL; + tcpConnectError = ERR_OK; + connectCallbackFires = true; + connectCallbackResult = ERR_OK; + + tcpCloseCallCount = 0; + lastClosePcb = NULL; + tcpCloseError = ERR_OK; + + tcpAbortCallCount = 0; + lastAbortPcb = NULL; + + outstandingPcbCount = 0; +} + +void LwipTcpFake_SetTcpNewFails(bool fails) +{ + tcpNewFails = fails; +} + +unsigned LwipTcpFake_TcpNewCallCount(void) +{ + return tcpNewCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastTcpNewReturned(void) +{ + return lastTcpNewReturned; +} + +unsigned LwipTcpFake_TcpArgCallCount(void) +{ + return tcpArgCallCount; +} + +void* LwipTcpFake_LastCallbackArg(void) +{ + return lastCallbackArg; +} + +unsigned LwipTcpFake_TcpRecvCallCount(void) +{ + return tcpRecvCallCount; +} + +tcp_recv_fn LwipTcpFake_LastRecvFn(void) +{ + return lastRecvFn; +} + +unsigned LwipTcpFake_TcpErrCallCount(void) +{ + return tcpErrCallCount; +} + +tcp_err_fn LwipTcpFake_LastErrFn(void) +{ + return lastErrFn; +} + +unsigned LwipTcpFake_TcpSentCallCount(void) +{ + return tcpSentCallCount; +} + +tcp_sent_fn LwipTcpFake_LastSentFn(void) +{ + return lastSentFn; +} + +void LwipTcpFake_SetTcpConnectError(int8_t err) +{ + tcpConnectError = (err_t) err; +} + +void LwipTcpFake_SetConnectCallbackFires(bool fires) +{ + connectCallbackFires = fires; +} + +void LwipTcpFake_SetConnectCallbackResult(int8_t err) +{ + connectCallbackResult = (err_t) err; +} + +unsigned LwipTcpFake_TcpConnectCallCount(void) +{ + return tcpConnectCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastConnectPcb(void) +{ + return lastConnectPcb; +} + +const ip_addr_t* LwipTcpFake_LastConnectIpaddr(void) +{ + return lastConnectIpaddr; +} + +uint16_t LwipTcpFake_LastConnectPort(void) +{ + return lastConnectPort; +} + +tcp_connected_fn LwipTcpFake_LastConnectedFn(void) +{ + return lastConnectedFn; +} + +void LwipTcpFake_SetTcpCloseError(int8_t err) +{ + tcpCloseError = (err_t) err; +} + +unsigned LwipTcpFake_TcpCloseCallCount(void) +{ + return tcpCloseCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastClosePcb(void) +{ + return lastClosePcb; +} + +unsigned LwipTcpFake_TcpAbortCallCount(void) +{ + return tcpAbortCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastAbortPcb(void) +{ + return lastAbortPcb; +} + +int LwipTcpFake_OutstandingPcbCount(void) +{ + return outstandingPcbCount; +} + +void LwipTcpFake_NotePcbReleasedByErr(void) +{ + --outstandingPcbCount; +} + +/* ------------------------------------------------------------------ + * lwIP API replacements + * ----------------------------------------------------------------*/ + +struct tcp_pcb* tcp_new(void) +{ + ++tcpNewCallCount; + if (tcpNewFails) + { + lastTcpNewReturned = NULL; + } + else + { + lastTcpNewReturned = &fakePcb; + ++outstandingPcbCount; + } + return lastTcpNewReturned; +} + +void tcp_arg(struct tcp_pcb* pcb, void* arg) +{ + (void) pcb; + ++tcpArgCallCount; + lastCallbackArg = arg; +} + +void tcp_recv(struct tcp_pcb* pcb, tcp_recv_fn recv) +{ + (void) pcb; + ++tcpRecvCallCount; + lastRecvFn = recv; +} + +void tcp_err(struct tcp_pcb* pcb, tcp_err_fn err) +{ + (void) pcb; + ++tcpErrCallCount; + lastErrFn = err; +} + +void tcp_sent(struct tcp_pcb* pcb, tcp_sent_fn sent) +{ + (void) pcb; + ++tcpSentCallCount; + lastSentFn = sent; +} + +err_t tcp_connect(struct tcp_pcb* pcb, const ip_addr_t* ipaddr, u16_t port, tcp_connected_fn connected) +{ + ++tcpConnectCallCount; + lastConnectPcb = pcb; + lastConnectIpaddr = ipaddr; + lastConnectPort = port; + lastConnectedFn = connected; + if ((tcpConnectError == ERR_OK) && connectCallbackFires && (connected != NULL)) + { + (void) connected(lastCallbackArg, pcb, connectCallbackResult); + } + return tcpConnectError; +} + +err_t tcp_close(struct tcp_pcb* pcb) +{ + ++tcpCloseCallCount; + lastClosePcb = pcb; + if (tcpCloseError == ERR_OK) + { + --outstandingPcbCount; + } + return tcpCloseError; +} + +void tcp_abort(struct tcp_pcb* pcb) +{ + ++tcpAbortCallCount; + lastAbortPcb = pcb; + --outstandingPcbCount; +} From 289d8f53b56ac6eb37efa4902537cbe56bcb46d8 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 21:34:10 +0000 Subject: [PATCH 3/8] feat: S28.05 LwipRawTcpStream Send/Read + RX queue + tcp_err handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Source/SolidSyslogLwipRawTcpStream.c | 181 ++++++++++- Tests/Lwip/CMakeLists.txt | 1 + .../Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 295 ++++++++++++++++-- .../LwipFakes/Interface/LwipPbufFake.h | 7 + .../Support/LwipFakes/Interface/LwipTcpFake.h | 22 ++ Tests/Support/LwipFakes/Source/LwipPbufFake.c | 5 + Tests/Support/LwipFakes/Source/LwipTcpFake.c | 114 +++++++ 7 files changed, 592 insertions(+), 33 deletions(-) diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c index b21a326f..cbf13962 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c @@ -3,12 +3,15 @@ #include #include #include +#include #include "lwip/arch.h" #include "lwip/err.h" #include "lwip/ip.h" #include "lwip/ip_addr.h" +#include "lwip/pbuf.h" #include "lwip/tcp.h" +#include "lwip/tcpbase.h" #include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogNullStream.h" #include "SolidSyslogStream.h" @@ -17,14 +20,22 @@ struct SolidSyslogAddress; +/* SolidSyslogStream_Read returns < 0 to signal EOF/error (socket closed + * internally); -1 is the in-tree convention shared with Posix/Winsock/PlusTcp. */ +static const SolidSyslogSsize READ_FAILED = -1; + static uint32_t LwipRawTcpStream_NullConnectTimeoutGetter(void* context); static bool LwipRawTcpStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr); +static bool LwipRawTcpStream_Send(struct SolidSyslogStream* base, const void* buffer, size_t size); +static SolidSyslogSsize LwipRawTcpStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size); static void LwipRawTcpStream_Close(struct SolidSyslogStream* base); static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base); static inline bool LwipRawTcpStream_ConfigProvidesGetter(const struct SolidSyslogLwipRawTcpStreamConfig* config); static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStream* self); +static inline bool LwipRawTcpStream_HasQueuedRx(const struct SolidSyslogLwipRawTcpStream* self); +static inline bool LwipRawTcpStream_RxQueueIsFull(const struct SolidSyslogLwipRawTcpStream* self); static struct tcp_pcb* LwipRawTcpStream_OpenAndConfigurePcb(struct SolidSyslogLwipRawTcpStream* self); static bool LwipRawTcpStream_ConnectOrAbortOnFailure( struct SolidSyslogLwipRawTcpStream* self, @@ -37,6 +48,15 @@ static bool LwipRawTcpStream_TryConnect( static bool LwipRawTcpStream_WaitForConnectedCallback(struct SolidSyslogLwipRawTcpStream* self); static uint32_t LwipRawTcpStream_ResolveConnectTimeoutMs(struct SolidSyslogLwipRawTcpStream* self); static void LwipRawTcpStream_AbortAndForgetPcb(struct SolidSyslogLwipRawTcpStream* self); +static bool LwipRawTcpStream_SendOrCloseOnFailure( + struct SolidSyslogLwipRawTcpStream* self, + const void* buffer, + size_t size +); +static bool LwipRawTcpStream_OutputResultIsAcceptable(err_t outputErr); +static size_t LwipRawTcpStream_DrainHeadBytes(struct SolidSyslogLwipRawTcpStream* self, void* buffer, size_t size); +static void LwipRawTcpStream_EnqueueRxPbuf(struct SolidSyslogLwipRawTcpStream* self, struct pbuf* p); +static void LwipRawTcpStream_DrainAllQueuedPbufs(struct SolidSyslogLwipRawTcpStream* self); static void LwipRawTcpStream_ClosePcb(struct SolidSyslogLwipRawTcpStream* self); static err_t LwipRawTcpStream_ConnectedCallback(void* arg, struct tcp_pcb* pcb, err_t err); @@ -50,7 +70,10 @@ void LwipRawTcpStream_Initialise( ) { static const struct SolidSyslogLwipRawTcpStream DefaultLwipRawTcpStream = { - .Base = {.Open = LwipRawTcpStream_Open, .Send = NULL, .Read = NULL, .Close = LwipRawTcpStream_Close}, + .Base = {.Open = LwipRawTcpStream_Open, + .Send = LwipRawTcpStream_Send, + .Read = LwipRawTcpStream_Read, + .Close = LwipRawTcpStream_Close}, .Config = {.GetConnectTimeoutMs = LwipRawTcpStream_NullConnectTimeoutGetter, .ConnectTimeoutContext = NULL, .Sleep = NULL}, @@ -119,6 +142,16 @@ static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStr return self->Pcb != NULL; } +static inline bool LwipRawTcpStream_HasQueuedRx(const struct SolidSyslogLwipRawTcpStream* self) +{ + return self->RxQueueCount > 0; +} + +static inline bool LwipRawTcpStream_RxQueueIsFull(const struct SolidSyslogLwipRawTcpStream* self) +{ + return self->RxQueueCount >= SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; +} + static struct tcp_pcb* LwipRawTcpStream_OpenAndConfigurePcb(struct SolidSyslogLwipRawTcpStream* self) { struct tcp_pcb* pcb = tcp_new(); @@ -195,6 +228,115 @@ static void LwipRawTcpStream_AbortAndForgetPcb(struct SolidSyslogLwipRawTcpStrea self->Pcb = NULL; } +static bool LwipRawTcpStream_Send(struct SolidSyslogStream* base, const void* buffer, size_t size) +{ + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); + bool sent = false; + if (LwipRawTcpStream_IsOpen(self)) + { + sent = LwipRawTcpStream_SendOrCloseOnFailure(self, buffer, size); + } + return sent; +} + +static bool LwipRawTcpStream_SendOrCloseOnFailure( + struct SolidSyslogLwipRawTcpStream* self, + const void* buffer, + size_t size +) +{ + /* TCP_WRITE_FLAG_COPY hands the caller's buffer to lwIP-owned pbufs + * before tcp_write returns — caller buffer lifetime ends here. + * tcp_output nudges transmission; ERR_MEM there is "queued, lwIP + * retries" so we still report success (lwIP owns the bytes). */ + err_t writeErr = tcp_write(self->Pcb, buffer, (u16_t) size, TCP_WRITE_FLAG_COPY); + bool ok = (writeErr == ERR_OK); + if (ok) + { + err_t outputErr = tcp_output(self->Pcb); + ok = LwipRawTcpStream_OutputResultIsAcceptable(outputErr); + } + if (!ok) + { + LwipRawTcpStream_ClosePcb(self); + } + return ok; +} + +static bool LwipRawTcpStream_OutputResultIsAcceptable(err_t outputErr) +{ + return (outputErr == ERR_OK) || (outputErr == ERR_MEM); +} + +static SolidSyslogSsize LwipRawTcpStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size) +{ + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); + SolidSyslogSsize result = READ_FAILED; + if (LwipRawTcpStream_IsOpen(self)) + { + if (LwipRawTcpStream_HasQueuedRx(self)) + { + size_t copied = LwipRawTcpStream_DrainHeadBytes(self, buffer, size); + tcp_recved(self->Pcb, (u16_t) copied); + result = (SolidSyslogSsize) copied; + } + else if (self->Errored) + { + /* Peer FIN drained → close internally per the Stream contract + * ("< 0 means EOF AND socket closed internally"). */ + LwipRawTcpStream_ClosePcb(self); + } + else + { + result = 0; /* would-block */ + } + } + return result; +} + +/* Copies up to `size` bytes out of the head pbuf, advancing the read + * cursor. When the head is fully drained, pbuf_free's it and advances + * the queue head — tail entries shift up through the bounded ring via + * modular arithmetic, no compaction. */ +static size_t LwipRawTcpStream_DrainHeadBytes(struct SolidSyslogLwipRawTcpStream* self, void* buffer, size_t size) +{ + struct pbuf* head = self->RxQueue[self->RxQueueHead]; + size_t available = (size_t) head->len - self->RxHeadOffset; + size_t toCopy = (size < available) ? size : available; + (void) memcpy(buffer, ((const uint8_t*) head->payload) + self->RxHeadOffset, toCopy); + self->RxHeadOffset += toCopy; + if (self->RxHeadOffset >= (size_t) head->len) + { + (void) pbuf_free(head); + self->RxQueueHead = (self->RxQueueHead + 1U) % SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; + self->RxQueueCount--; + self->RxHeadOffset = 0; + } + return toCopy; +} + +static void LwipRawTcpStream_EnqueueRxPbuf(struct SolidSyslogLwipRawTcpStream* self, struct pbuf* p) +{ + size_t tail = (self->RxQueueHead + self->RxQueueCount) % SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; + self->RxQueue[tail] = p; + self->RxQueueCount++; +} + +/* Drains every queued pbuf via pbuf_free. Used by ClosePcb so an explicit + * Close, Send-failure-induced Close, or Destroy never leaks the pbufs lwIP + * handed us via tcp_recv. After tcp_err nulls Pcb the queue may still hold + * pbufs we accepted before the error — those need freeing too. */ +static void LwipRawTcpStream_DrainAllQueuedPbufs(struct SolidSyslogLwipRawTcpStream* self) +{ + while (LwipRawTcpStream_HasQueuedRx(self)) + { + (void) pbuf_free(self->RxQueue[self->RxQueueHead]); + self->RxQueueHead = (self->RxQueueHead + 1U) % SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; + self->RxQueueCount--; + } + self->RxHeadOffset = 0; +} + static void LwipRawTcpStream_Close(struct SolidSyslogStream* base) { struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); @@ -204,9 +346,11 @@ static void LwipRawTcpStream_Close(struct SolidSyslogStream* base) /* tcp_close must NOT be called on a pcb that has already been released by * tcp_err — that's a use-after-free in lwIP. The Pcb != NULL guard works * because LwipRawTcpStream_ErrCallback nulls Pcb when lwIP releases the - * pcb on its side. */ + * pcb on its side. The queue drain runs unconditionally — pbufs we + * accepted via tcp_recv are ours to free regardless of pcb state. */ static void LwipRawTcpStream_ClosePcb(struct SolidSyslogLwipRawTcpStream* self) { + LwipRawTcpStream_DrainAllQueuedPbufs(self); if (LwipRawTcpStream_IsOpen(self)) { (void) tcp_close(self->Pcb); @@ -229,23 +373,34 @@ static err_t LwipRawTcpStream_ConnectedCallback(void* arg, struct tcp_pcb* pcb, return ERR_OK; } -/* RX queue + tcp_recved drain (and pbuf_free of the head pbuf when drained) - * land in the Send/Read slice. For now the callback is a no-op stub — lwIP - * requires the slot wired before tcp_connect so the recv path is set up by - * the time the peer sends data, even though the wrapper's Stream_Read does - * not yet drain. */ +/* tcp_recv fires when lwIP has bytes for us — non-NULL p means a pbuf + * arrived; NULL p means peer half-closed (FIN). Backpressure on a full + * queue by returning non-ERR_OK; lwIP holds the pbuf and replays the + * callback when the queue drains. */ static err_t LwipRawTcpStream_RecvCallback(void* arg, struct tcp_pcb* tpcb, struct pbuf* p, err_t err) { - (void) arg; (void) tpcb; - (void) p; (void) err; - return ERR_OK; + struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + err_t result = ERR_OK; + if (p == NULL) + { + self->Errored = true; + } + else if (LwipRawTcpStream_RxQueueIsFull(self)) + { + result = ERR_MEM; + } + else + { + LwipRawTcpStream_EnqueueRxPbuf(self, p); + } + return result; } -/* Real tcp_sent handling is unused under TCP_WRITE_FLAG_COPY (Commit 3 - * decision 1) — caller buffers are released at Send return, not at - * peer-ACK time. The callback exists because lwIP requires the slot set. */ +/* Real tcp_sent handling is unused under TCP_WRITE_FLAG_COPY — caller + * buffers are released at Send return, not at peer-ACK time. The slot + * exists because lwIP requires the callback set when the pcb is wired. */ static err_t LwipRawTcpStream_SentCallback(void* arg, struct tcp_pcb* tpcb, u16_t len) { (void) arg; diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index 07e5fce8..fda4909d 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -145,6 +145,7 @@ add_test(NAME SolidSyslogLwipRawDatagramTest COMMAND SolidSyslogLwipRawDatagramT add_executable(SolidSyslogLwipRawTcpStreamTest SolidSyslogLwipRawTcpStreamTest.cpp main.cpp + ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipPbufFake.c ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipTcpFake.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddress.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddressMessages.c diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index 7d87076e..60a135ea 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -6,8 +6,11 @@ using namespace CososoTesting; +#include + #include "ConfigLockFake.h" #include "ErrorHandlerFake.h" +#include "LwipPbufFake.h" #include "LwipTcpFake.h" #include "SolidSyslogLwipRawAddress.h" #include "SolidSyslogLwipRawAddressPrivate.h" @@ -21,7 +24,9 @@ using namespace CososoTesting; #include "lwip/err.h" #include "lwip/ip4_addr.h" #include "lwip/ip_addr.h" +#include "lwip/pbuf.h" #include "lwip/tcp.h" +#include "lwip/tcpbase.h" static const uint16_t TEST_PORT = 514; @@ -93,10 +98,13 @@ TEST_BASE(LwipRawTcpStreamTestBase) struct SolidSyslogLwipRawTcpStreamConfig config{}; struct SolidSyslogStream* stream = nullptr; struct SolidSyslogAddress* address = nullptr; + char sendBuffer[16] = {}; + char readBuffer[16] = {}; void createFakesAndHandles() { LwipTcpFake_Reset(); + LwipPbufFake_Reset(); FakeSleep_Reset(); FakeGetConnectTimeoutMs_Reset(); config = {}; @@ -113,6 +121,39 @@ TEST_BASE(LwipRawTcpStreamTestBase) SolidSyslogLwipRawAddress_Destroy(address); SolidSyslogLwipRawTcpStream_Destroy(stream); LONGS_EQUAL_TEXT(0, LwipTcpFake_OutstandingPcbCount(), "leaked tcp_pcb past teardown"); + LONGS_EQUAL_TEXT(0, LwipPbufFake_OutstandingPbufCount(), "leaked pbuf past teardown"); + } + + /* Drive the wrapper's tcp_recv callback with a fabricated incoming + * pbuf. Caller supplies stack storage for the pbuf so multi-pbuf + * tests can verify queue head advancement by pointer identity. */ + void pushIncomingPbuf(struct pbuf* p, const void* data, uint16_t len) + { + p->payload = const_cast(data); + p->len = len; + p->tot_len = len; + p->next = nullptr; + LwipPbufFake_NoteIncomingPbuf(); + tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); + (void) recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), p, ERR_OK); + } + + /* Drive the wrapper's tcp_recv callback with NULL p — peer half-close + * (FIN). lwIP retains the pcb; only the receive half is gone. */ + void pushPeerFin() const + { + tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); + (void) recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), nullptr, ERR_OK); + } + + /* Drive the wrapper's tcp_err callback — lwIP releases the pcb + * upstream before this fires, so the leak invariant needs the + * matching NotePcbReleasedByErr. */ + void pushTcpErr(int8_t err) const + { + tcp_err_fn errCb = LwipTcpFake_LastErrFn(); + errCb(LwipTcpFake_LastCallbackArg(), (err_t) err); + LwipTcpFake_NotePcbReleasedByErr(); } }; @@ -192,6 +233,17 @@ TEST(SolidSyslogLwipRawTcpStream, CloseBeforeOpenIsNoOp) CALLED_FAKE(LwipTcpFake_TcpAbort, NEVER); } +TEST(SolidSyslogLwipRawTcpStream, SendBeforeOpenReturnsFalse) +{ + CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, sizeof(sendBuffer))); + CALLED_FAKE(LwipTcpFake_TcpWrite, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStream, ReadBeforeOpenReturnsMinusOne) +{ + LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); +} + TEST(SolidSyslogLwipRawTcpStream, OpenCallsTcpNew) { SolidSyslogStream_Open(stream, address); @@ -369,28 +421,13 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, TcpErrCallbackReleasesPcbWithoutCalli /* Drive the err callback the wrapper registered. lwIP releases the * pcb upstream before invoking err — the wrapper must null its Pcb * field and NOT call tcp_close (use-after-free). */ - tcp_err_fn errCb = LwipTcpFake_LastErrFn(); - void* arg = LwipTcpFake_LastCallbackArg(); - errCb(arg, ERR_RST); - LwipTcpFake_NotePcbReleasedByErr(); + pushTcpErr(ERR_RST); SolidSyslogStream_Close(stream); CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); } -TEST(SolidSyslogLwipRawTcpStreamConnected, RecvCallbackReturnsErrOkAsNoOpStub) -{ - /* Real RX queue handling lands in the Send/Read slice — this slot's - * stub returns ERR_OK so lwIP sees the bytes as accepted. */ - tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); - - LONGS_EQUAL( - ERR_OK, - recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), nullptr, ERR_OK) - ); -} - TEST(SolidSyslogLwipRawTcpStreamConnected, SentCallbackReturnsErrOkAsNoOpStub) { /* TCP_WRITE_FLAG_COPY means caller buffers are released at Send return — @@ -403,10 +440,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SentCallbackReturnsErrOkAsNoOpStub) TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyAfterTcpErrDoesNotCallTcpClose) { - tcp_err_fn errCb = LwipTcpFake_LastErrFn(); - void* arg = LwipTcpFake_LastCallbackArg(); - errCb(arg, ERR_RST); - LwipTcpFake_NotePcbReleasedByErr(); + pushTcpErr(ERR_RST); SolidSyslogLwipRawTcpStream_Destroy(stream); stream = nullptr; @@ -414,6 +448,227 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyAfterTcpErrDoesNotCallTcpClose CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); } +/* ------------------------------------------------------------------ + * Send tests + * ----------------------------------------------------------------*/ + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpWriteWithCopyFlagAndSize) +{ + memcpy(sendBuffer, "hello", 5); + + SolidSyslogStream_Send(stream, sendBuffer, 5); + + CALLED_FAKE(LwipTcpFake_TcpWrite, ONCE); + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastWritePcb()); + POINTERS_EQUAL(sendBuffer, LwipTcpFake_LastWriteDataptr()); + LONGS_EQUAL(5, LwipTcpFake_LastWriteLength()); + LONGS_EQUAL(TCP_WRITE_FLAG_COPY, LwipTcpFake_LastWriteApiFlags()); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpOutputAfterTcpWrite) +{ + SolidSyslogStream_Send(stream, sendBuffer, 1); + + CALLED_FAKE(LwipTcpFake_TcpOutput, ONCE); + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastOutputPcb()); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsTrueOnTcpWriteAndOutputOk) +{ + CHECK_TRUE(SolidSyslogStream_Send(stream, sendBuffer, 1)); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsFalseAndClosesOnTcpWriteFails) +{ + LwipTcpFake_SetTcpWriteError(ERR_MEM); + + CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CALLED_FAKE(LwipTcpFake_TcpOutput, NEVER); + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsTrueWhenTcpOutputDefersWithErrMem) +{ + /* tcp_write succeeded → data is in pcb->snd_buf; ERR_MEM from + * tcp_output just means lwIP will retry on the next tcp_tmr tick. + * lwIP owns the bytes, so the wrapper reports success (mirrors POSIX's + * "kernel accepted into send buffer" semantics). */ + LwipTcpFake_SetTcpOutputError(ERR_MEM); + + CHECK_TRUE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsFalseAndClosesOnTcpOutputOtherError) +{ + LwipTcpFake_SetTcpOutputError(ERR_CONN); + + CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, SendAfterTcpErrReturnsFalse) +{ + pushTcpErr(ERR_RST); + + CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CALLED_FAKE(LwipTcpFake_TcpWrite, NEVER); +} + +/* ------------------------------------------------------------------ + * Read tests + RX queue behaviour + * ----------------------------------------------------------------*/ + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsZeroWhenQueueEmpty) +{ + /* Would-block semantic — keeps the connection alive. */ + LONGS_EQUAL(0, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadDrainsHeadPbufAndAcksWithTcpRecved) +{ + struct pbuf p = {}; + const char data[] = "ack"; + pushIncomingPbuf(&p, data, sizeof(data) - 1); + + SolidSyslogSsize n = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + + LONGS_EQUAL(3, n); + MEMCMP_EQUAL(data, readBuffer, 3); + CALLED_FAKE(LwipTcpFake_TcpRecved, ONCE); + LONGS_EQUAL(3, LwipTcpFake_LastRecvedLen()); + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastRecvedPcb()); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadFreesPbufWhenFullyDrained) +{ + struct pbuf p = {}; + const char data[] = "x"; + pushIncomingPbuf(&p, data, 1); + + (void) SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + + CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsPartialWhenBufferSmallerThanHeadPbuf) +{ + struct pbuf p = {}; + const char data[] = "abcde"; + pushIncomingPbuf(&p, data, 5); + + SolidSyslogSsize first = SolidSyslogStream_Read(stream, readBuffer, 2); + + LONGS_EQUAL(2, first); + MEMCMP_EQUAL("ab", readBuffer, 2); + /* Head not yet drained — pbuf still queued. */ + CALLED_FAKE(LwipPbufFake_PbufFree, NEVER); + + SolidSyslogSsize second = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + + LONGS_EQUAL(3, second); + MEMCMP_EQUAL("cde", readBuffer, 3); + CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadAdvancesPastDrainedPbufToNextInQueue) +{ + struct pbuf p1 = {}; + struct pbuf p2 = {}; + const char d1[] = "AA"; + const char d2[] = "BB"; + pushIncomingPbuf(&p1, d1, 2); + pushIncomingPbuf(&p2, d2, 2); + + SolidSyslogSsize n1 = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + SolidSyslogSsize n2 = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + + LONGS_EQUAL(2, n1); + LONGS_EQUAL(2, n2); + /* Both pbufs were freed as each was drained. */ + LONGS_EQUAL(2, LwipPbufFake_PbufFreeCallCount()); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsMinusOneAfterPeerFinAndDrainsBeforeEof) +{ + struct pbuf p = {}; + const char data[] = "z"; + pushIncomingPbuf(&p, data, 1); + pushPeerFin(); + + /* Queued bytes drain first. */ + LONGS_EQUAL(1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + /* Then the next Read sees empty queue + Errored and returns -1, closing + * the pcb internally per the Stream contract. */ + LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsMinusOneAfterTcpErrWithoutQueuedData) +{ + pushTcpErr(ERR_RST); + + LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + /* Pcb already nulled by tcp_err — no tcp_close. */ + CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, RecvCallbackBackpressuresWhenQueueFull) +{ + struct pbuf pbufs[SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE] = {}; + const char data[] = "1"; + for (auto& p : pbufs) + { + pushIncomingPbuf(&p, data, 1); + } + + /* Next pbuf — queue is full, callback should return non-ERR_OK so + * lwIP holds onto the pbuf and replays the callback later. */ + struct pbuf overflow = {}; + overflow.payload = const_cast(static_cast(data)); + overflow.len = 1; + overflow.tot_len = 1; + overflow.next = nullptr; + tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); + err_t result = recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), &overflow, ERR_OK); + + CHECK(result != ERR_OK); + /* Drain so teardown's leak invariant stays honest. */ + for (size_t i = 0; i < SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; ++i) + { + (void) SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + } +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyDrainsRxQueueOnCleanup) +{ + struct pbuf p1 = {}; + struct pbuf p2 = {}; + const char d[] = "xy"; + pushIncomingPbuf(&p1, d, 2); + pushIncomingPbuf(&p2, d, 2); + + SolidSyslogLwipRawTcpStream_Destroy(stream); + stream = nullptr; + + LONGS_EQUAL(2, LwipPbufFake_PbufFreeCallCount()); + /* Leak invariant (pinned in shared teardown) confirms both pbufs + * were freed. */ +} + +TEST(SolidSyslogLwipRawTcpStreamConnected, CloseDrainsRxQueueBeforeTcpClose) +{ + struct pbuf p = {}; + const char d[] = "q"; + pushIncomingPbuf(&p, d, 1); + + SolidSyslogStream_Close(stream); + + CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); + CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); +} + /* ------------------------------------------------------------------ * Pool tests — handed-out handles never call lwIP, so they don't need * the fake state. Same TEST_GROUP shape as Commit 1. diff --git a/Tests/Support/LwipFakes/Interface/LwipPbufFake.h b/Tests/Support/LwipFakes/Interface/LwipPbufFake.h index 2d2c6aab..e69d7d5a 100644 --- a/Tests/Support/LwipFakes/Interface/LwipPbufFake.h +++ b/Tests/Support/LwipFakes/Interface/LwipPbufFake.h @@ -30,6 +30,13 @@ EXTERN_C_BEGIN * leaked a pbuf — pin this in teardown to catch leaks across the suite. */ int LwipPbufFake_OutstandingPbufCount(void); + /* RX-side helper: pbufs handed to the wrapper via the tcp_recv callback + * come from lwIP's machinery, not our pbuf_alloc — but the wrapper still + * pbuf_free's them when fully drained. Tests fabricate stack pbufs and + * call this to balance the outstanding count so the leak invariant + * stays honest. */ + void LwipPbufFake_NoteIncomingPbuf(void); + EXTERN_C_END #endif /* LWIPPBUFFAKE_H */ diff --git a/Tests/Support/LwipFakes/Interface/LwipTcpFake.h b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h index 5f38cd16..cd17bd71 100644 --- a/Tests/Support/LwipFakes/Interface/LwipTcpFake.h +++ b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h @@ -65,6 +65,28 @@ EXTERN_C_BEGIN unsigned LwipTcpFake_TcpAbortCallCount(void); struct tcp_pcb* LwipTcpFake_LastAbortPcb(void); + /* tcp_write configuration */ + void LwipTcpFake_SetTcpWriteError(int8_t err); + + /* tcp_write spy */ + unsigned LwipTcpFake_TcpWriteCallCount(void); + struct tcp_pcb* LwipTcpFake_LastWritePcb(void); + const void* LwipTcpFake_LastWriteDataptr(void); + uint16_t LwipTcpFake_LastWriteLength(void); + uint8_t LwipTcpFake_LastWriteApiFlags(void); + + /* tcp_output configuration */ + void LwipTcpFake_SetTcpOutputError(int8_t err); + + /* tcp_output spy */ + unsigned LwipTcpFake_TcpOutputCallCount(void); + struct tcp_pcb* LwipTcpFake_LastOutputPcb(void); + + /* tcp_recved spy — window-update ACK after the wrapper drains bytes */ + unsigned LwipTcpFake_TcpRecvedCallCount(void); + struct tcp_pcb* LwipTcpFake_LastRecvedPcb(void); + uint16_t LwipTcpFake_LastRecvedLen(void); + /* Allocated-but-not-yet-freed PCB count. Successful tcp_new bumps it; * tcp_close / tcp_abort decrement. The tcp_err callback releases the * pcb upstream — tests that fire it via LwipTcpFake_LastErrFn must call diff --git a/Tests/Support/LwipFakes/Source/LwipPbufFake.c b/Tests/Support/LwipFakes/Source/LwipPbufFake.c index bfdb54d7..6f0258a1 100644 --- a/Tests/Support/LwipFakes/Source/LwipPbufFake.c +++ b/Tests/Support/LwipFakes/Source/LwipPbufFake.c @@ -69,6 +69,11 @@ int LwipPbufFake_OutstandingPbufCount(void) return outstandingPbufCount; } +void LwipPbufFake_NoteIncomingPbuf(void) +{ + ++outstandingPbufCount; +} + struct pbuf* pbuf_alloc(pbuf_layer layer, u16_t length, pbuf_type type) { ++pbufAllocCallCount; diff --git a/Tests/Support/LwipFakes/Source/LwipTcpFake.c b/Tests/Support/LwipFakes/Source/LwipTcpFake.c index bddaaee5..204210fa 100644 --- a/Tests/Support/LwipFakes/Source/LwipTcpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipTcpFake.c @@ -38,6 +38,21 @@ static err_t tcpCloseError = ERR_OK; static unsigned tcpAbortCallCount = 0; static struct tcp_pcb* lastAbortPcb = NULL; +static unsigned tcpWriteCallCount = 0; +static struct tcp_pcb* lastWritePcb = NULL; +static const void* lastWriteDataptr = NULL; +static u16_t lastWriteLength = 0; +static u8_t lastWriteApiFlags = 0; +static err_t tcpWriteError = ERR_OK; + +static unsigned tcpOutputCallCount = 0; +static struct tcp_pcb* lastOutputPcb = NULL; +static err_t tcpOutputError = ERR_OK; + +static unsigned tcpRecvedCallCount = 0; +static struct tcp_pcb* lastRecvedPcb = NULL; +static u16_t lastRecvedLen = 0; + static int outstandingPcbCount = 0; void LwipTcpFake_Reset(void) @@ -75,6 +90,21 @@ void LwipTcpFake_Reset(void) tcpAbortCallCount = 0; lastAbortPcb = NULL; + tcpWriteCallCount = 0; + lastWritePcb = NULL; + lastWriteDataptr = NULL; + lastWriteLength = 0; + lastWriteApiFlags = 0; + tcpWriteError = ERR_OK; + + tcpOutputCallCount = 0; + lastOutputPcb = NULL; + tcpOutputError = ERR_OK; + + tcpRecvedCallCount = 0; + lastRecvedPcb = NULL; + lastRecvedLen = 0; + outstandingPcbCount = 0; } @@ -198,6 +228,66 @@ struct tcp_pcb* LwipTcpFake_LastAbortPcb(void) return lastAbortPcb; } +void LwipTcpFake_SetTcpWriteError(int8_t err) +{ + tcpWriteError = (err_t) err; +} + +unsigned LwipTcpFake_TcpWriteCallCount(void) +{ + return tcpWriteCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastWritePcb(void) +{ + return lastWritePcb; +} + +const void* LwipTcpFake_LastWriteDataptr(void) +{ + return lastWriteDataptr; +} + +uint16_t LwipTcpFake_LastWriteLength(void) +{ + return lastWriteLength; +} + +uint8_t LwipTcpFake_LastWriteApiFlags(void) +{ + return lastWriteApiFlags; +} + +void LwipTcpFake_SetTcpOutputError(int8_t err) +{ + tcpOutputError = (err_t) err; +} + +unsigned LwipTcpFake_TcpOutputCallCount(void) +{ + return tcpOutputCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastOutputPcb(void) +{ + return lastOutputPcb; +} + +unsigned LwipTcpFake_TcpRecvedCallCount(void) +{ + return tcpRecvedCallCount; +} + +struct tcp_pcb* LwipTcpFake_LastRecvedPcb(void) +{ + return lastRecvedPcb; +} + +uint16_t LwipTcpFake_LastRecvedLen(void) +{ + return lastRecvedLen; +} + int LwipTcpFake_OutstandingPcbCount(void) { return outstandingPcbCount; @@ -286,3 +376,27 @@ void tcp_abort(struct tcp_pcb* pcb) lastAbortPcb = pcb; --outstandingPcbCount; } + +err_t tcp_write(struct tcp_pcb* pcb, const void* dataptr, u16_t len, u8_t apiflags) +{ + ++tcpWriteCallCount; + lastWritePcb = pcb; + lastWriteDataptr = dataptr; + lastWriteLength = len; + lastWriteApiFlags = apiflags; + return tcpWriteError; +} + +err_t tcp_output(struct tcp_pcb* pcb) +{ + ++tcpOutputCallCount; + lastOutputPcb = pcb; + return tcpOutputError; +} + +void tcp_recved(struct tcp_pcb* pcb, u16_t len) +{ + ++tcpRecvedCallCount; + lastRecvedPcb = pcb; + lastRecvedLen = len; +} From 619fd03262caf24b54ab28e2321c37fa7cb509e1 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 28 May 2026 07:20:45 +0000 Subject: [PATCH 4/8] =?UTF-8?q?refactor:=20S28.05=20LwipRawTcpStream=20tes?= =?UTF-8?q?ts=20=E2=80=94=20sendBytes/readBytes=20helpers,=20auto-balanced?= =?UTF-8?q?=20pushIncomingPbuf,=20CHECK=5FFORWARDED=5FPCB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 104 ++++++++++-------- 1 file changed, 61 insertions(+), 43 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index 60a135ea..304d201a 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -52,6 +52,11 @@ static const uint16_t TEST_PORT = 514; UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastCode()); \ } while (0) +// Asserts the lwIP API call recorded the pcb the wrapper got back from +// tcp_new — proves the wrapper forwarded the right handle. `getter` is +// the LwipTcpFake_LastXxxPcb accessor function (zero-arg). +#define CHECK_FORWARDED_PCB(getter) POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) + namespace { unsigned FakeSleep_CallCount = 0; @@ -124,18 +129,41 @@ TEST_BASE(LwipRawTcpStreamTestBase) LONGS_EQUAL_TEXT(0, LwipPbufFake_OutstandingPbufCount(), "leaked pbuf past teardown"); } + /* Send through the abstract base against the shared sendBuffer. Default + * length 1 covers the lifecycle / pcb-forwarding tests that don't care + * about content; size-specific tests pass it explicitly. */ + bool sendBytes(size_t length = 1U) const + { + return SolidSyslogStream_Send(stream, sendBuffer, length); + } + + /* Read through the abstract base into the shared readBuffer. Default + * capacity is the full buffer; partial-drain tests pass it explicitly. */ + SolidSyslogSsize readBytes(size_t capacity = sizeof(readBuffer)) + { + return SolidSyslogStream_Read(stream, readBuffer, capacity); + } + /* Drive the wrapper's tcp_recv callback with a fabricated incoming * pbuf. Caller supplies stack storage for the pbuf so multi-pbuf - * tests can verify queue head advancement by pointer identity. */ - void pushIncomingPbuf(struct pbuf* p, const void* data, uint16_t len) + * tests can verify queue head advancement by pointer identity. + * Returns the err_t the wrapper's callback returned — ERR_OK means + * the wrapper took ownership of the pbuf (leak counter bumped here); + * non-ERR_OK means lwIP retains the pbuf and the counter stays put, + * so backpressure tests can pin the contract without imbalance. */ + err_t pushIncomingPbuf(struct pbuf* p, const void* data, uint16_t len) { p->payload = const_cast(data); p->len = len; p->tot_len = len; p->next = nullptr; - LwipPbufFake_NoteIncomingPbuf(); tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); - (void) recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), p, ERR_OK); + err_t result = recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), p, ERR_OK); + if (result == ERR_OK) + { + LwipPbufFake_NoteIncomingPbuf(); + } + return result; } /* Drive the wrapper's tcp_recv callback with NULL p — peer half-close @@ -235,13 +263,13 @@ TEST(SolidSyslogLwipRawTcpStream, CloseBeforeOpenIsNoOp) TEST(SolidSyslogLwipRawTcpStream, SendBeforeOpenReturnsFalse) { - CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, sizeof(sendBuffer))); + CHECK_FALSE(sendBytes()); CALLED_FAKE(LwipTcpFake_TcpWrite, NEVER); } TEST(SolidSyslogLwipRawTcpStream, ReadBeforeOpenReturnsMinusOne) { - LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + LONGS_EQUAL(-1, readBytes()); } TEST(SolidSyslogLwipRawTcpStream, OpenCallsTcpNew) @@ -290,7 +318,7 @@ TEST(SolidSyslogLwipRawTcpStream, OpenCallsTcpConnectWithAddressIpAndPort) SolidSyslogStream_Open(stream, address); CALLED_FAKE(LwipTcpFake_TcpConnect, ONCE); - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastConnectPcb()); + CHECK_FORWARDED_PCB(LwipTcpFake_LastConnectPcb); POINTERS_EQUAL(&SolidSyslogLwipRawAddress_AsConst(address)->Ip, LwipTcpFake_LastConnectIpaddr()); LONGS_EQUAL(TEST_PORT, LwipTcpFake_LastConnectPort()); CHECK(LwipTcpFake_LastConnectedFn() != nullptr); @@ -377,7 +405,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, CloseCallsTcpCloseOnOpenPcb) SolidSyslogStream_Close(stream); CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastClosePcb()); + CHECK_FORWARDED_PCB(LwipTcpFake_LastClosePcb); } TEST(SolidSyslogLwipRawTcpStreamConnected, SecondCloseDoesNotCloseAgain) @@ -456,10 +484,10 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpWriteWithCopyFlagAndSize) { memcpy(sendBuffer, "hello", 5); - SolidSyslogStream_Send(stream, sendBuffer, 5); + sendBytes(5); CALLED_FAKE(LwipTcpFake_TcpWrite, ONCE); - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastWritePcb()); + CHECK_FORWARDED_PCB(LwipTcpFake_LastWritePcb); POINTERS_EQUAL(sendBuffer, LwipTcpFake_LastWriteDataptr()); LONGS_EQUAL(5, LwipTcpFake_LastWriteLength()); LONGS_EQUAL(TCP_WRITE_FLAG_COPY, LwipTcpFake_LastWriteApiFlags()); @@ -467,22 +495,22 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpWriteWithCopyFlagAndSize) TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpOutputAfterTcpWrite) { - SolidSyslogStream_Send(stream, sendBuffer, 1); + sendBytes(); CALLED_FAKE(LwipTcpFake_TcpOutput, ONCE); - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastOutputPcb()); + CHECK_FORWARDED_PCB(LwipTcpFake_LastOutputPcb); } TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsTrueOnTcpWriteAndOutputOk) { - CHECK_TRUE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CHECK_TRUE(sendBytes()); } TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsFalseAndClosesOnTcpWriteFails) { LwipTcpFake_SetTcpWriteError(ERR_MEM); - CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CHECK_FALSE(sendBytes()); CALLED_FAKE(LwipTcpFake_TcpOutput, NEVER); CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); } @@ -495,7 +523,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsTrueWhenTcpOutputDefersWit * "kernel accepted into send buffer" semantics). */ LwipTcpFake_SetTcpOutputError(ERR_MEM); - CHECK_TRUE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CHECK_TRUE(sendBytes()); CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); } @@ -503,7 +531,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendReturnsFalseAndClosesOnTcpOutputO { LwipTcpFake_SetTcpOutputError(ERR_CONN); - CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CHECK_FALSE(sendBytes()); CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); } @@ -511,7 +539,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendAfterTcpErrReturnsFalse) { pushTcpErr(ERR_RST); - CHECK_FALSE(SolidSyslogStream_Send(stream, sendBuffer, 1)); + CHECK_FALSE(sendBytes()); CALLED_FAKE(LwipTcpFake_TcpWrite, NEVER); } @@ -522,7 +550,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendAfterTcpErrReturnsFalse) TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsZeroWhenQueueEmpty) { /* Would-block semantic — keeps the connection alive. */ - LONGS_EQUAL(0, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + LONGS_EQUAL(0, readBytes()); CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); } @@ -532,13 +560,13 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadDrainsHeadPbufAndAcksWithTcpRecve const char data[] = "ack"; pushIncomingPbuf(&p, data, sizeof(data) - 1); - SolidSyslogSsize n = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + SolidSyslogSsize n = readBytes(); LONGS_EQUAL(3, n); MEMCMP_EQUAL(data, readBuffer, 3); CALLED_FAKE(LwipTcpFake_TcpRecved, ONCE); LONGS_EQUAL(3, LwipTcpFake_LastRecvedLen()); - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), LwipTcpFake_LastRecvedPcb()); + CHECK_FORWARDED_PCB(LwipTcpFake_LastRecvedPcb); } TEST(SolidSyslogLwipRawTcpStreamConnected, ReadFreesPbufWhenFullyDrained) @@ -547,7 +575,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadFreesPbufWhenFullyDrained) const char data[] = "x"; pushIncomingPbuf(&p, data, 1); - (void) SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + (void) readBytes(); CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); } @@ -558,14 +586,14 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsPartialWhenBufferSmallerTh const char data[] = "abcde"; pushIncomingPbuf(&p, data, 5); - SolidSyslogSsize first = SolidSyslogStream_Read(stream, readBuffer, 2); + SolidSyslogSsize first = readBytes(2); LONGS_EQUAL(2, first); MEMCMP_EQUAL("ab", readBuffer, 2); /* Head not yet drained — pbuf still queued. */ CALLED_FAKE(LwipPbufFake_PbufFree, NEVER); - SolidSyslogSsize second = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + SolidSyslogSsize second = readBytes(); LONGS_EQUAL(3, second); MEMCMP_EQUAL("cde", readBuffer, 3); @@ -581,8 +609,8 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadAdvancesPastDrainedPbufToNextInQu pushIncomingPbuf(&p1, d1, 2); pushIncomingPbuf(&p2, d2, 2); - SolidSyslogSsize n1 = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); - SolidSyslogSsize n2 = SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); + SolidSyslogSsize n1 = readBytes(); + SolidSyslogSsize n2 = readBytes(); LONGS_EQUAL(2, n1); LONGS_EQUAL(2, n2); @@ -598,10 +626,10 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsMinusOneAfterPeerFinAndDra pushPeerFin(); /* Queued bytes drain first. */ - LONGS_EQUAL(1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + LONGS_EQUAL(1, readBytes()); /* Then the next Read sees empty queue + Errored and returns -1, closing * the pcb internally per the Stream contract. */ - LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + LONGS_EQUAL(-1, readBytes()); CALLED_FAKE(LwipTcpFake_TcpClose, ONCE); } @@ -609,7 +637,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsMinusOneAfterTcpErrWithout { pushTcpErr(ERR_RST); - LONGS_EQUAL(-1, SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer))); + LONGS_EQUAL(-1, readBytes()); /* Pcb already nulled by tcp_err — no tcp_close. */ CALLED_FAKE(LwipTcpFake_TcpClose, NEVER); } @@ -617,28 +645,18 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, ReadReturnsMinusOneAfterTcpErrWithout TEST(SolidSyslogLwipRawTcpStreamConnected, RecvCallbackBackpressuresWhenQueueFull) { struct pbuf pbufs[SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE] = {}; - const char data[] = "1"; for (auto& p : pbufs) { - pushIncomingPbuf(&p, data, 1); + pushIncomingPbuf(&p, "1", 1); } - /* Next pbuf — queue is full, callback should return non-ERR_OK so - * lwIP holds onto the pbuf and replays the callback later. */ + /* Next pbuf — queue is full, callback returns non-ERR_OK and + * pushIncomingPbuf's auto-balancing leaves the outstanding count + * untouched so teardown's leak invariant still passes. */ struct pbuf overflow = {}; - overflow.payload = const_cast(static_cast(data)); - overflow.len = 1; - overflow.tot_len = 1; - overflow.next = nullptr; - tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); - err_t result = recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), &overflow, ERR_OK); + err_t result = pushIncomingPbuf(&overflow, "1", 1); CHECK(result != ERR_OK); - /* Drain so teardown's leak invariant stays honest. */ - for (size_t i = 0; i < SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE; ++i) - { - (void) SolidSyslogStream_Read(stream, readBuffer, sizeof(readBuffer)); - } } TEST(SolidSyslogLwipRawTcpStreamConnected, DestroyDrainsRxQueueOnCleanup) From 6fc5dd7985aa936a1929880f39eafeec77c7ddf5 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 28 May 2026 09:14:03 +0000 Subject: [PATCH 5/8] feat: S28.05 SolidSyslogLwipRawTcpStream + integrating-lwip.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CLAUDE.md | 2 + CMakeLists.txt | 16 +- DEVLOG.md | 148 ++++++++ .../Interface/SolidSyslogLwipRawTcpStream.h | 3 +- .../Source/SolidSyslogLwipRawTcpStream.c | 59 +-- .../SolidSyslogLwipRawTcpStreamPrivate.h | 16 +- .../SolidSyslogLwipRawTcpStreamStatic.c | 9 +- .../Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 3 +- .../Support/LwipFakes/Interface/LwipTcpFake.h | 2 - Tests/Support/LwipFakes/Source/LwipTcpFake.c | 2 +- docs/integrating-lwip.md | 338 ++++++++++++++++++ docs/misra-deviations.md | 33 +- misra_suppressions.txt | 2 + scripts/misra_renumber.py | 2 + 14 files changed, 579 insertions(+), 56 deletions(-) create mode 100644 docs/integrating-lwip.md diff --git a/CLAUDE.md b/CLAUDE.md index ef472970..3e9eec66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -361,6 +361,8 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullStream.h` | Any code installing a no-op stream slot (Open/Close are no-ops, Send returns `true` to drop on the floor so the Store does not fill with undeliverables, Read returns `0` for would-block so the caller does not tear the connection down) | `SolidSyslogNullStream_Get` | | `SolidSyslogPlusTcpTcpStream.h` | System setup code on FreeRTOS targets using FreeRTOS-Plus-TCP for TCP | `SolidSyslogPlusTcpTcpStream_Create(void)`, `_Destroy(base)` (wraps `FreeRTOS_socket` / `FreeRTOS_connect` / `FreeRTOS_send` / `FreeRTOS_recv` / `FreeRTOS_closesocket`; bounded blocking connect via `SO_SNDTIMEO=200ms`, cleared post-connect so Send/Read are non-blocking). Instance struct lives in a library-internal static pool (E11); default pool size 2 to support the future TLS-via-mbedTLS + plain-TCP pair (S08.07). Pool-exhaustion fallback is the shared `SolidSyslogNullStream`. | | `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. | | `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. | diff --git a/CMakeLists.txt b/CMakeLists.txt index 0afce111..dc41c5f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -150,12 +150,12 @@ endif() # time. CI builds PLUSTCP and LWIP in isolation; BOTH is a dev-container # convenience. # -# S28.03 lands the first slice of the lwIP backend (Address + Resolver, -# literal IPv4 only). LWIP and BOTH configure but the backend is still -# partial — Datagram/TcpStream/BDD arrive in S28.04-S28.06. Platform/LwipRaw -# is OS-agnostic by construction (wraps lwIP only, no FreeRTOS coupling) -# and is gated separately on LWIP_PATH below; this option only controls -# which adapter pack the FreeRTOS BDD target wires up. +# S28.03-S28.05 land the OS-agnostic slices (Address, Resolver, Datagram, +# TcpStream). LWIP and BOTH configure but the backend is still partial — +# BDD arrives in S28.06. Platform/LwipRaw is OS-agnostic by construction +# (wraps lwIP only, no FreeRTOS coupling) and is gated separately on +# LWIP_PATH below; this option only controls which adapter pack the +# FreeRTOS BDD target wires up. set(SOLIDSYSLOG_FREERTOS_NET "PLUSTCP" CACHE STRING "FreeRTOS networking backend: PLUSTCP (default), LWIP, or BOTH") set_property(CACHE SOLIDSYSLOG_FREERTOS_NET PROPERTY STRINGS PLUSTCP LWIP BOTH) @@ -169,12 +169,12 @@ endif() if(SOLIDSYSLOG_FREERTOS_NET STREQUAL "LWIP") message(STATUS "SOLIDSYSLOG_FREERTOS_NET=LWIP: lwIP backend partial — " - "Address+Resolver+Datagram only; TcpStream/BDD arrive in S28.05-S28.06.") + "Address+Resolver+Datagram+TcpStream only; BDD arrives in S28.06.") endif() if(SOLIDSYSLOG_FREERTOS_NET STREQUAL "BOTH") message(STATUS "SOLIDSYSLOG_FREERTOS_NET=BOTH: lwIP backend partial — " - "Address+Resolver+Datagram only; TcpStream/BDD arrive in S28.05-S28.06.") + "Address+Resolver+Datagram+TcpStream only; BDD arrives in S28.06.") endif() # Build-time tunables (E21: Port-Time Configurability). Integrators override diff --git a/DEVLOG.md b/DEVLOG.md index 1ec59a34..eeb372a2 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,153 @@ # Dev Log +## 2026-05-28 — S28.05 SolidSyslogLwipRawTcpStream + +Fifth story in E28 (#465, parent #439). TCP stream adapter for the +lwIP Raw API, plus the `docs/integrating-lwip.md` guide that's been +deferred since S28.03 landed the first OS-agnostic slice. The new +ground here is bridging lwIP's callback-driven Raw TCP API to +SolidSyslog's synchronous `Stream_Open`/`Send`/`Read`/`Close` +contract — a richer problem than the Datagram side because TCP's +`tcp_connect` is asynchronous and the receive path delivers +arbitrary-sized pbufs that the wrapper has to buffer until +`Stream_Read` drains them. + +### Design decisions + +The full set landed in the issue body up front (#465); five key calls. + +1. **`tcp_write` uses `TCP_WRITE_FLAG_COPY`.** Deliberate divergence + from S28.04's Datagram-side `PBUF_REF`. UDP is safe with REF + because `udp_sendto` is either synchronous or `etharp_query` + clones the payload on queue. TCP `tcp_write` defers transmission + indefinitely and depends on retransmits + peer ACKs — the + caller's buffer would have to live well past `Stream_Send` + return, which the synchronous contract doesn't allow. COPY costs + one `memcpy` per send; syslog records are small, well within + budget. `tcp_output` is called after every successful `tcp_write` + to nudge transmission; `ERR_MEM` from `tcp_output` is + queued-not-failed (lwIP owns the bytes, retries on next `tcp_tmr`). + +2. **Synchronous Open via spin-with-injected-sleep.** `tcp_connect` + returns immediately; the registered `connected_cb` fires when the + SYN/SYN-ACK exchange completes. The wrapper spins on a + `Connected` flag bounded by the existing per-instance + `GetConnectTimeoutMs` getter (the S12.17 pattern), sleeping + `SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS` (default 10 ms) per + iteration via the integrator-supplied `SolidSyslogSleepFunction`. + Semaphore-wakeup ruled out — under `NO_SYS=1` lwIP's callback + runs in the same thread as `Stream_Open`, so there's no other + thread to post a semaphore from; the spin+sleep pattern is the + only shape that covers both `NO_SYS=1` and `NO_SYS=0` with one + code path. Integrator's Sleep ticks `sys_check_timeouts` + RX + under `NO_SYS=1`; under `NO_SYS=0` it just yields. + +3. **`tcp_close`-after-`tcp_err` gotcha encapsulated.** lwIP's + `tcp_err` callback fires *after* lwIP has already released the + pcb — calling `tcp_close` on it would be a use-after-free. The + wrapper's `tcp_err` handler nulls `self->Pcb`; `Close` checks + `Pcb != NULL` before calling `tcp_close`. Integrators never see + the rule. Documented as background only in the integrator + guide ("don't poke at lwIP pcbs through the abstraction"). + +4. **Bounded RX queue draining `tcp_recv`.** Fixed ring of + `struct pbuf*` pointers sized by `SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE` + (default 8 — sized for the typical mTLS handshake flight). + `tcp_recv` callback enqueues; returns `ERR_MEM` when full so + lwIP retains the pbuf and replays (flow-control). `Stream_Read` + drains bytes from the head pbuf, calls `tcp_recved(pcb, n)` to + ACK back to lwIP's window, `pbuf_free`s the head when fully + drained. Peer FIN (`tcp_recv` with `p == NULL`) sets `Errored` + but lets queued bytes drain first — Read returns `-1` only once + the queue is empty AND Errored, and closes internally per the + Stream contract at that point. + +5. **`SOF_KEEPALIVE` set in `Open`.** Matches `SolidSyslogPosixTcpStream`. + The idle/intvl/cnt values come from the integrator's lwipopts.h + (`TCP_KEEPIDLE_DEFAULT` etc.); the wrapper just opts the pcb in. + No-op if `LWIP_TCP_KEEPALIVE=0` — documented as recommended-on + in the integrator guide. + +### Commit shape + +Four commits + one mid-story refactor: + +- **Commit 1 — Pool plumbing + tunables + 13 tests.** Three-TU split + (Static.c / Messages.c / .c with empty vtable / Private.h struct). + Three new tunables (`POOL_SIZE` = 2 to match the other TCP-stream + backends, `CONNECT_POLL_MS` = 10, `RX_QUEUE_SIZE` = 8). NULL + config or NULL Sleep → silent NullStream fallback (sibling pattern; + only POOL_EXHAUSTED + UNKNOWN_DESTROY reported). +- **Commit 2 — Open/Close lifecycle + connect callback bridge.** + `LwipTcpFake` covering `tcp_new` / `tcp_arg` / `tcp_recv` / + `tcp_err` / `tcp_sent` / `tcp_connect` / `tcp_close` / `tcp_abort` + with captured callbacks tests can drive. 22 new tests. +- **Commit 3 — Send / Read / RX queue + tcp_err handling.** The + COPY-flag Send path, ERR_MEM-after-write success branch, RX-ring + drain with `tcp_recved` ACK, peer-FIN-after-drain → -1, RX-queue + drain on Close/Destroy as a leak invariant. 20 new tests. +- **Refactor — test cleanups.** David spotted three repetitions + worth lifting. `sendBytes()` / `readBytes()` helpers on the test + base (matches the Datagram precedent). `pushIncomingPbuf` returns + the `err_t` and only bumps the leak counter on `ERR_OK`, encoding + the ownership transfer in one place. `CHECK_FORWARDED_PCB` macro + for the recurring "this lwIP call received our pcb" intent. ~20 + lines lighter; one inline test setup collapses to 3 lines. +- **Commit 4 — Housekeeping.** Integrator guide, CLAUDE.md rows, + CMake STATUS strings + the `:155` comment (deferred PR #464 nit), + this DEVLOG entry, pre-PR three-step. + +### Test-fixture pattern that carried over + +S28.04's `TEST_BASE` + two `TEST_GROUP_BASE` split (Created-only vs +Open-already) transferred 1:1. New convenience: shared `sendBuffer` ++ `readBuffer` fields with `sendBytes(len=1)` / `readBytes(cap=N)` +helpers — same shape as the Datagram tests' `sendBytes` from S28.04. +Leak invariants in shared teardown: `LwipTcpFake_OutstandingPcbCount()` +and `LwipPbufFake_OutstandingPbufCount()` both pinned to zero, +exactly mirroring the Datagram precedent. + +The novel piece is the `LwipTcpFake`'s default behaviour for +`tcp_connect`: synchronously invokes the registered `connected_cb` +with `ERR_OK` before returning. That gives the happy-path Open tests +a single line of work (`stream = …Create + Open` in the Connected +group's setup) and surfaces the spin loop's Sleep-call-count as 0 on +the happy path — pinned by `OpenHappyPathDoesNotSleep`. Tests that +need to exercise the timeout / errored-cb paths call +`LwipTcpFake_SetConnectCallbackFires(false)` / +`SetConnectCallbackResult(ERR_RST)`. + +### New tunables, new error source + +- `SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE` default `2U` — matches + every other TCP-stream backend; covers the TLS-over-plain-TCP + pair. +- `SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS` default `10U` — 20 + polls inside the default 200 ms connect deadline. +- `SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE` default `8U` — sized for + the typical mTLS handshake flight (ServerHello + Certificate + + ServerKeyExchange + ServerHelloDone is 2–4 segments; 8 leaves + margin). +- `LwipRawTcpStreamErrorSource` with `POOL_EXHAUSTED` / + `UNKNOWN_DESTROY`, matching the other TCP-stream backends. + +### `docs/integrating-lwip.md` + +The deferred integrator guide picks up the lwipopts.h expectations +both S28.04 (ARP_QUEUEING) and S28.05 (LWIP_TCP_KEEPALIVE) surfaced, +plus the `NO_SYS=1` vs `NO_SYS=0` threading rule and the +`tcpip_callback()` marshalling convention. Worked example for the +bare-metal main loop shape; per-adapter design notes documenting the +pbuf-strategy split (UDP REF, TCP COPY) so future readers don't +re-litigate the inconsistency. + +### Next up + +S28.06 — `Bdd/Targets/FreeRtosLwip/` + three new CI jobs +(`build-freertos-host-tdd-lwip`, `build-freertos-target-lwip`, +`bdd-freertos-qemu-lwip`). Mirrors the existing FreeRTOS BDD target +but with the lwIP adapters in place of PlusTcp. + ## 2026-05-27 — S28.04 SolidSyslogLwipRawDatagram Fourth story in E28 (#463, parent #439). UDP datagram adapter for the diff --git a/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h index 08d4e272..7820f54c 100644 --- a/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h +++ b/Platform/LwipRaw/Interface/SolidSyslogLwipRawTcpStream.h @@ -18,7 +18,8 @@ EXTERN_C_BEGIN Sleep; /* required — drives the bounded-connect spin; NULL config falls back to NullStream */ }; - struct SolidSyslogStream* SolidSyslogLwipRawTcpStream_Create(const struct SolidSyslogLwipRawTcpStreamConfig* config); + struct SolidSyslogStream* SolidSyslogLwipRawTcpStream_Create(const struct SolidSyslogLwipRawTcpStreamConfig* config + ); void SolidSyslogLwipRawTcpStream_Destroy(struct SolidSyslogStream * base); EXTERN_C_END diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c index cbf13962..c85e0cbb 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c @@ -1,3 +1,4 @@ +#include "SolidSyslogLwipRawTcpStream.h" #include "SolidSyslogLwipRawTcpStreamPrivate.h" #include @@ -8,21 +9,15 @@ #include "lwip/arch.h" #include "lwip/err.h" #include "lwip/ip.h" -#include "lwip/ip_addr.h" #include "lwip/pbuf.h" #include "lwip/tcp.h" #include "lwip/tcpbase.h" #include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogNullStream.h" #include "SolidSyslogStream.h" -#include "SolidSyslogStreamDefinition.h" #include "SolidSyslogTunables.h" -struct SolidSyslogAddress; - -/* SolidSyslogStream_Read returns < 0 to signal EOF/error (socket closed - * internally); -1 is the in-tree convention shared with Posix/Winsock/PlusTcp. */ -static const SolidSyslogSsize READ_FAILED = -1; +struct SolidSyslogStream; static uint32_t LwipRawTcpStream_NullConnectTimeoutGetter(void* context); @@ -32,6 +27,7 @@ static SolidSyslogSsize LwipRawTcpStream_Read(struct SolidSyslogStream* base, vo static void LwipRawTcpStream_Close(struct SolidSyslogStream* base); static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase(struct SolidSyslogStream* base); +static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromArg(void* arg); static inline bool LwipRawTcpStream_ConfigProvidesGetter(const struct SolidSyslogLwipRawTcpStreamConfig* config); static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStream* self); static inline bool LwipRawTcpStream_HasQueuedRx(const struct SolidSyslogLwipRawTcpStream* self); @@ -64,19 +60,18 @@ static err_t LwipRawTcpStream_RecvCallback(void* arg, struct tcp_pcb* tpcb, stru static err_t LwipRawTcpStream_SentCallback(void* arg, struct tcp_pcb* tpcb, u16_t len); static void LwipRawTcpStream_ErrCallback(void* arg, err_t err); -void LwipRawTcpStream_Initialise( - struct SolidSyslogStream* base, - const struct SolidSyslogLwipRawTcpStreamConfig* config -) +void LwipRawTcpStream_Initialise(struct SolidSyslogStream* base, const struct SolidSyslogLwipRawTcpStreamConfig* config) { static const struct SolidSyslogLwipRawTcpStream DefaultLwipRawTcpStream = { - .Base = {.Open = LwipRawTcpStream_Open, - .Send = LwipRawTcpStream_Send, - .Read = LwipRawTcpStream_Read, - .Close = LwipRawTcpStream_Close}, - .Config = {.GetConnectTimeoutMs = LwipRawTcpStream_NullConnectTimeoutGetter, - .ConnectTimeoutContext = NULL, - .Sleep = NULL}, + .Base = + {.Open = LwipRawTcpStream_Open, + .Send = LwipRawTcpStream_Send, + .Read = LwipRawTcpStream_Read, + .Close = LwipRawTcpStream_Close}, + .Config = + {.GetConnectTimeoutMs = LwipRawTcpStream_NullConnectTimeoutGetter, + .ConnectTimeoutContext = NULL, + .Sleep = NULL}, .Pcb = NULL, .Connected = false, .Errored = false, @@ -115,6 +110,15 @@ static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromBase( return (struct SolidSyslogLwipRawTcpStream*) base; } +/* Recovers our self pointer from the void* argument lwIP passes back into + * every callback we registered via tcp_arg(pcb, self). Single named helper + * so the void→struct cast lives in one place — and MISRA 11.5 has one + * suppression site, not one per callback. */ +static inline struct SolidSyslogLwipRawTcpStream* LwipRawTcpStream_SelfFromArg(void* arg) +{ + return (struct SolidSyslogLwipRawTcpStream*) arg; +} + void LwipRawTcpStream_Cleanup(struct SolidSyslogStream* base) { LwipRawTcpStream_Close(base); @@ -144,7 +148,7 @@ static inline bool LwipRawTcpStream_IsOpen(const struct SolidSyslogLwipRawTcpStr static inline bool LwipRawTcpStream_HasQueuedRx(const struct SolidSyslogLwipRawTcpStream* self) { - return self->RxQueueCount > 0; + return self->RxQueueCount > 0U; } static inline bool LwipRawTcpStream_RxQueueIsFull(const struct SolidSyslogLwipRawTcpStream* self) @@ -179,10 +183,7 @@ static bool LwipRawTcpStream_ConnectOrAbortOnFailure( return connected; } -static bool LwipRawTcpStream_TryConnect( - struct SolidSyslogLwipRawTcpStream* self, - const struct SolidSyslogAddress* addr -) +static bool LwipRawTcpStream_TryConnect(struct SolidSyslogLwipRawTcpStream* self, const struct SolidSyslogAddress* addr) { const struct SolidSyslogLwipRawAddress* dst = SolidSyslogLwipRawAddress_AsConst(addr); self->Connected = false; @@ -270,6 +271,10 @@ static bool LwipRawTcpStream_OutputResultIsAcceptable(err_t outputErr) static SolidSyslogSsize LwipRawTcpStream_Read(struct SolidSyslogStream* base, void* buffer, size_t size) { + /* SolidSyslogStream_Read returns < 0 to signal EOF/error (socket closed + * internally); -1 is the in-tree convention shared with Posix/Winsock/PlusTcp. */ + static const SolidSyslogSsize READ_FAILED = -1; + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromBase(base); SolidSyslogSsize result = READ_FAILED; if (LwipRawTcpStream_IsOpen(self)) @@ -303,7 +308,7 @@ static size_t LwipRawTcpStream_DrainHeadBytes(struct SolidSyslogLwipRawTcpStream struct pbuf* head = self->RxQueue[self->RxQueueHead]; size_t available = (size_t) head->len - self->RxHeadOffset; size_t toCopy = (size < available) ? size : available; - (void) memcpy(buffer, ((const uint8_t*) head->payload) + self->RxHeadOffset, toCopy); + (void) memcpy(buffer, &((const uint8_t*) head->payload)[self->RxHeadOffset], toCopy); self->RxHeadOffset += toCopy; if (self->RxHeadOffset >= (size_t) head->len) { @@ -361,7 +366,7 @@ static void LwipRawTcpStream_ClosePcb(struct SolidSyslogLwipRawTcpStream* self) static err_t LwipRawTcpStream_ConnectedCallback(void* arg, struct tcp_pcb* pcb, err_t err) { (void) pcb; - struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromArg(arg); if (err == ERR_OK) { self->Connected = true; @@ -381,7 +386,7 @@ static err_t LwipRawTcpStream_RecvCallback(void* arg, struct tcp_pcb* tpcb, stru { (void) tpcb; (void) err; - struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromArg(arg); err_t result = ERR_OK; if (p == NULL) { @@ -415,7 +420,7 @@ static err_t LwipRawTcpStream_SentCallback(void* arg, struct tcp_pcb* tpcb, u16_ static void LwipRawTcpStream_ErrCallback(void* arg, err_t err) { (void) err; - struct SolidSyslogLwipRawTcpStream* self = (struct SolidSyslogLwipRawTcpStream*) arg; + struct SolidSyslogLwipRawTcpStream* self = LwipRawTcpStream_SelfFromArg(arg); self->Pcb = NULL; self->Errored = true; } diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h index 6e539669..d5d81ba4 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamPrivate.h @@ -13,15 +13,15 @@ struct pbuf; struct SolidSyslogLwipRawTcpStream { - struct SolidSyslogStream Base; + struct SolidSyslogStream Base; struct SolidSyslogLwipRawTcpStreamConfig Config; - struct tcp_pcb* Pcb; - bool Connected; - bool Errored; - struct pbuf* RxQueue[SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE]; - size_t RxQueueHead; - size_t RxQueueCount; - size_t RxHeadOffset; + struct tcp_pcb* Pcb; + bool Connected; + bool Errored; + struct pbuf* RxQueue[SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE]; + size_t RxQueueHead; + size_t RxQueueCount; + size_t RxHeadOffset; }; void LwipRawTcpStream_Initialise( diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c index 69b4a7b0..6a9106c4 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c @@ -31,7 +31,7 @@ struct SolidSyslogStream* SolidSyslogLwipRawTcpStream_Create(const struct SolidS if (LwipRawTcpStream_IsValidConfig(config)) { size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&LwipRawTcpStream_Allocator); - if (SolidSyslogPoolAllocator_IndexIsValid(&LwipRawTcpStream_Allocator, index)) + if (SolidSyslogPoolAllocator_IndexIsValid(&LwipRawTcpStream_Allocator, index) == true) { LwipRawTcpStream_Initialise(&LwipRawTcpStream_Pool[index].Base, config); handle = &LwipRawTcpStream_Pool[index].Base; @@ -53,12 +53,7 @@ void SolidSyslogLwipRawTcpStream_Destroy(struct SolidSyslogStream* base) size_t index = LwipRawTcpStream_IndexFromHandle(base); bool released = SolidSyslogPoolAllocator_IndexIsValid(&LwipRawTcpStream_Allocator, index) && - SolidSyslogPoolAllocator_FreeIfInUse( - &LwipRawTcpStream_Allocator, - index, - LwipRawTcpStream_CleanupAtIndex, - NULL - ); + SolidSyslogPoolAllocator_FreeIfInUse(&LwipRawTcpStream_Allocator, index, LwipRawTcpStream_CleanupAtIndex, NULL); if (!released) { SolidSyslog_Error( diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index 304d201a..078d245c 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -22,8 +22,8 @@ using namespace CososoTesting; #include "SolidSyslogStreamDefinition.h" #include "SolidSyslogTunables.h" #include "lwip/err.h" +#include "lwip/ip.h" #include "lwip/ip4_addr.h" -#include "lwip/ip_addr.h" #include "lwip/pbuf.h" #include "lwip/tcp.h" #include "lwip/tcpbase.h" @@ -731,6 +731,7 @@ TEST_GROUP(SolidSyslogLwipRawTcpStreamPool) } } }; + // clang-format on TEST(SolidSyslogLwipRawTcpStreamPool, FillingPoolThenOverflowReturnsDistinctFallback) diff --git a/Tests/Support/LwipFakes/Interface/LwipTcpFake.h b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h index cd17bd71..ee1614b2 100644 --- a/Tests/Support/LwipFakes/Interface/LwipTcpFake.h +++ b/Tests/Support/LwipFakes/Interface/LwipTcpFake.h @@ -6,8 +6,6 @@ #include #include -#include "lwip/arch.h" -#include "lwip/err.h" #include "lwip/ip_addr.h" #include "lwip/tcp.h" diff --git a/Tests/Support/LwipFakes/Source/LwipTcpFake.c b/Tests/Support/LwipFakes/Source/LwipTcpFake.c index 204210fa..00d75b3e 100644 --- a/Tests/Support/LwipFakes/Source/LwipTcpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipTcpFake.c @@ -60,7 +60,7 @@ void LwipTcpFake_Reset(void) tcpNewCallCount = 0; /* Zero the fake pcb so so_options / state writes from prior tests don't * leak into the current one. */ - fakePcb = (struct tcp_pcb){0}; + fakePcb = (struct tcp_pcb) {0}; lastTcpNewReturned = NULL; tcpNewFails = false; diff --git a/docs/integrating-lwip.md b/docs/integrating-lwip.md new file mode 100644 index 00000000..ff822e6c --- /dev/null +++ b/docs/integrating-lwip.md @@ -0,0 +1,338 @@ +# Integrating SolidSyslog with lwIP (Raw API) + +`Platform/LwipRaw/` wraps lwIP's Raw API to provide the same +`SolidSyslogDatagram` / `SolidSyslogStream` / `SolidSyslogAddress` / +`SolidSyslogResolver` vtables the rest of the library composes against. +It is the right choice when: + +- Your target runs lwIP — bare-metal, FreeRTOS, Zephyr, ThreadX, NuttX + — including `NO_SYS=1` deployments where sockets/NETCONN aren't + available. +- You want to share TCP/IP between SolidSyslog and other lwIP-using + subsystems (HTTP server, MQTT client, OTA updater) without + duplicating the stack. +- You're on FreeRTOS but prefer lwIP to FreeRTOS-Plus-TCP for licence + or sizing reasons. Both backends ship in the same library; pick at + CMake time with `SOLIDSYSLOG_FREERTOS_NET=LWIP`. + +This document covers what *you*, the integrator, plug in. It does not +re-teach lwIP — for that, see the +[upstream lwIP documentation](https://www.nongnu.org/lwip/2_1_x/index.html). + +--- + +## What ships in `Platform/LwipRaw/` + +| Class | Wraps | Purpose | +|---|---|---| +| `SolidSyslogLwipRawAddress` | `ip_addr_t` + `u16_t` port | Destination handle the Resolver writes into and the Datagram/TcpStream read from. | +| `SolidSyslogLwipRawResolver` | `ipaddr_aton` | Synchronous numeric IPv4 parsing. Rejects DNS names — they need the future `SolidSyslogLwipRawDnsResolver` (S28.07). | +| `SolidSyslogLwipRawDatagram` | `udp_new` / `udp_sendto` / `udp_remove` | UDP sender. Zero-copy `PBUF_REF` send. | +| `SolidSyslogLwipRawTcpStream` | `tcp_new` / `tcp_connect` / `tcp_write` / `tcp_output` / `tcp_recv` / `tcp_recved` / `tcp_close` / `tcp_abort` | TCP byte transport. Bounded synchronous Open. Bounded RX pbuf queue. | + +`Platform/LwipRaw/Source/` is **OS-agnostic** — it wraps lwIP only and +contains zero direct calls to FreeRTOS, POSIX, Win32, Zephyr, or any +other host primitive. The one host primitive the TcpStream needs (a +bounded sleep for the synchronous-Open spin loop) is abstracted behind +the `SolidSyslogSleepFunction` typedef and supplied by you at +configure time. + +mbedTLS layering is unchanged — `SolidSyslogMbedTlsStream` consumes +`SolidSyslogLwipRawTcpStream` as its byte transport without +modification. See [`docs/integrating-mbedtls.md`](integrating-mbedtls.md) +for the TLS side. + +--- + +## `NO_SYS=1` vs `NO_SYS=0` + +lwIP supports both threading models. SolidSyslog supports both — the +adapter code is the same; the difference is entirely in how *you* +drive lwIP forward. + +### `NO_SYS=1` (bare-metal main-loop) + +Your `main()` is a forever-loop that, on each pass, calls +`sys_check_timeouts()` and drives the RX path +(`netif->input()` / `ethernetif_input()` / whichever your BSP wires). +Every lwIP Raw API call must happen on that same thread. + +SolidSyslog's `Service` loop fits this naturally — call it from your +main loop alongside `sys_check_timeouts()`. The TcpStream's bounded +synchronous-Open spin loop calls your injected `Sleep` callback +between polls; under `NO_SYS=1` your Sleep implementation should +**tick the lwIP machinery** while it waits: + +```c +void MyLwipSleep(int milliseconds) +{ + uint32_t deadline = MyTimebase_NowMs() + (uint32_t) milliseconds; + while (MyTimebase_NowMs() < deadline) + { + sys_check_timeouts(); + MyNetif_DrivePolledRx(); /* your BSP's RX pump */ + } +} +``` + +Without this, `tcp_connect`'s `connected_cb` never fires (lwIP can't +advance its state machine while you sleep), and Open times out. + +### `NO_SYS=0` (tcpip thread) + +lwIP runs a dedicated `tcpip` thread that owns its state machine; you +post work to it via `tcpip_callback()` or use the BSD Sockets / +NETCONN APIs. + +**Threading rule for the Raw API**: every Raw API call from outside +the tcpip thread must be marshalled via `tcpip_callback()` (or via +the `LWIP_TCPIP_CORE_LOCKING_INPUT` lock if you've compiled with core +locking). This applies to every call SolidSyslog's adapters make into +lwIP — Datagram's `udp_sendto`, TcpStream's `tcp_write`, etc. + +**SolidSyslog does not marshal internally.** That would force every +integrator to compile `tcpip.c` even when they're on `NO_SYS=1`. The +expectation is that *you* call SolidSyslog APIs on the tcpip thread, +either by running your Service loop on it, or by marshalling each +call at the SolidSyslog API boundary: + +```c +static void DoService(void* ctx) +{ + SolidSyslog_Service((struct SolidSyslog*) ctx); +} + +void MyServiceTick(struct SolidSyslog* handle) +{ + (void) tcpip_callback(DoService, handle); +} +``` + +Your injected `Sleep` callback under `NO_SYS=0` is just a yield — +typically `vTaskDelay(pdMS_TO_TICKS(milliseconds))` on FreeRTOS: + +```c +void MyLwipSleep(int milliseconds) +{ + vTaskDelay(pdMS_TO_TICKS((uint32_t) milliseconds)); +} +``` + +The tcpip thread runs concurrently and processes the SYN/SYN-ACK +exchange while you yield. + +--- + +## `lwipopts.h` expectations + +The adapter wraps a specific subset of lwIP — your `lwipopts.h` needs +those features compiled in. Defaults that already cover us are noted; +features you must enable are flagged. + +| Setting | Required | Notes | +|---|---|---| +| `LWIP_RAW=1` | **Yes** | The whole point — Raw API. | +| `LWIP_UDP=1` | **Yes (Datagram)** | Wraps `udp_*`. | +| `LWIP_TCP=1` | **Yes (TcpStream)** | Wraps `tcp_*`. | +| `LWIP_DNS` | No | The current Resolver only parses numeric IPv4 via `ipaddr_aton`. DNS lands in S28.07. | +| `ARP_QUEUEING=1` | **Recommended** | lwIP default. With it, the first datagram to an unresolved peer is `pbuf_clone`d into PBUF_RAM and queued behind the ARP request — when the reply lands, the packet ships. With `ARP_QUEUEING=0` the first datagram is silently dropped at the IP layer; cold-start logging loses messages. | +| `LWIP_TCP_KEEPALIVE=1` | **Recommended** | Without this, the `SOF_KEEPALIVE` bit the adapter sets on every pcb is a no-op. Tune `TCP_KEEPIDLE_DEFAULT` / `TCP_KEEPINTVL_DEFAULT` / `TCP_KEEPCNT_DEFAULT` for your deadline budget. | +| `TCP_MSS` | Per-platform | Default `536` (RFC-conservative). Bump to `1460` on Ethernet links if your MTU is 1500 and you want fewer segments per syslog record. | +| `PBUF_POOL_SIZE` | Per-traffic | Size the pool generously. The Datagram path borrows one `MEMP_PBUF` header per send; the TcpStream RX path borrows one pbuf per segment until `Stream_Read` drains it. | +| `MEMP_NUM_TCP_PCB` | Per-deployment | At least the number of concurrent `SolidSyslogLwipRawTcpStream` instances (default pool = 2 for the TLS-over-plain-TCP pair). | +| `MEMP_NUM_UDP_PCB` | Per-deployment | At least the number of concurrent `SolidSyslogLwipRawDatagram` instances (default pool = 1). | + +--- + +## Wiring example — bare-metal `NO_SYS=1` + +```c +#include "SolidSyslog.h" +#include "SolidSyslogConfig.h" +#include "SolidSyslogLwipRawAddress.h" +#include "SolidSyslogLwipRawDatagram.h" +#include "SolidSyslogLwipRawResolver.h" +#include "SolidSyslogLwipRawTcpStream.h" +#include "SolidSyslogPassthroughBuffer.h" +#include "SolidSyslogStreamSender.h" +#include "SolidSyslogUdpSender.h" + +extern void MyLwipSleep(int milliseconds); /* sys_check_timeouts + RX pump */ + +static struct SolidSyslog* g_syslog; +static struct SolidSyslogBuffer* g_buffer; + +void LogPipelineInit(void) +{ + struct SolidSyslogResolver* resolver = SolidSyslogLwipRawResolver_Create(); + struct SolidSyslogAddress* udpAddr = SolidSyslogLwipRawAddress_Create(); + struct SolidSyslogDatagram* datagram = SolidSyslogLwipRawDatagram_Create(); + + struct SolidSyslogUdpSenderConfig udpCfg = { + .Resolver = resolver, + .Datagram = datagram, + .Address = udpAddr, + .Endpoint = MyEndpoint, /* your SolidSyslogEndpointFunction */ + .EndpointVersion = MyEndpointVersion, + }; + struct SolidSyslogSender* sender = SolidSyslogUdpSender_Create(&udpCfg); + + g_buffer = SolidSyslogPassthroughBuffer_Create(sender); + + struct SolidSyslogConfig syslogCfg = { + .Hostname = MyHostname, + .AppName = MyAppName, + .ProcessId = MyProcessId, + .Clock = MyClock, + .Buffer = g_buffer, + }; + g_syslog = SolidSyslog_Create(&syslogCfg); +} + +void MainLoop(void) +{ + for (;;) + { + sys_check_timeouts(); + MyNetif_DrivePolledRx(); + SolidSyslog_Service(g_syslog); + /* … rest of your application … */ + } +} +``` + +For TCP, swap the UDP sender for a `SolidSyslogStreamSender` whose +`Stream` is a `SolidSyslogLwipRawTcpStream` built with your `Sleep`: + +```c +struct SolidSyslogLwipRawTcpStreamConfig streamCfg = { + .GetConnectTimeoutMs = NULL, /* falls back to SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS */ + .ConnectTimeoutContext = NULL, + .Sleep = MyLwipSleep, /* required */ +}; +struct SolidSyslogStream* tcpStream = SolidSyslogLwipRawTcpStream_Create(&streamCfg); +``` + +--- + +## Adapter-specific notes + +### Datagram — pbuf strategy + +`SolidSyslogLwipRawDatagram` uses `PBUF_REF`: a single pbuf header is +allocated per `SendTo`, its `payload` is pointed at the caller's +buffer, `udp_sendto` is called, and the header is `pbuf_free`d before +return. Zero copy on the hot path. + +This is safe across ARP queueing because lwIP's `etharp_query` does +`pbuf_clone(…, PBUF_RAM, q)` — it copies the referenced payload into +a private RAM pbuf before queueing, so the caller's buffer only needs +to live for the `udp_sendto` call itself (which is the synchronous +guarantee `SolidSyslogDatagram_SendTo` already provides). + +### TcpStream — `tcp_write` strategy + +`SolidSyslogLwipRawTcpStream` uses `TCP_WRITE_FLAG_COPY`: lwIP copies +your bytes into its own pbufs before `tcp_write` returns. This costs +one `memcpy` per send but honours the synchronous `Stream_Send(buf, +len)` lifetime contract — caller buffers are free at return, +regardless of when the peer ACKs. + +`tcp_output` is called after every successful `tcp_write` to nudge +transmission. If `tcp_output` returns `ERR_MEM`, the data is already +in `pcb->snd_buf` — lwIP will retry on the next `tcp_tmr` tick and +the wrapper reports Send-success (lwIP owns the bytes, exactly +matching POSIX's "kernel accepted the data into the send buffer" +semantics). + +### TcpStream — synchronous Open via spin-with-sleep + +`tcp_connect` is asynchronous: it returns immediately and lwIP fires +the registered `connected_cb` when the SYN/SYN-ACK exchange +completes. `SolidSyslogStream_Open` is synchronous. The wrapper +bridges by spinning on a `Connected` flag set by its `connected_cb`, +sleeping `SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS` (default 10 ms) +between checks via your injected `Sleep`, bounded by the +`GetConnectTimeoutMs` getter (default `SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS` += 200 ms — install a runtime getter per the S12.17 pattern if you +need to vary it). + +Timeout → `tcp_abort` on the pcb, Open returns `false`. Errored +callback → `tcp_abort`, Open returns `false`. Immediate non-`ERR_OK` +from `tcp_connect` → `tcp_abort`, Open returns `false`. + +### TcpStream — RX queue + +lwIP's `tcp_recv` callback fires when bytes arrive. The wrapper owns +a bounded ring of pbuf pointers sized by `SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE` +(default 8). Each `Stream_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. Queue full → the +callback returns non-`ERR_OK` so lwIP retains the pbuf and replays +the callback later (lwIP's flow-control hook). + +The default-8 queue size is sized for the typical mTLS handshake +flight (ServerHello + Certificate + ServerKeyExchange + +ServerHelloDone is 2–4 segments). Bump it for streaming server +responses; lower it if your `MEMP_NUM_PBUF` is constrained and you +need lwIP to backpressure sooner. + +### TcpStream — lifecycle ownership + +The wrapper owns its `tcp_pcb` end-to-end. Three things to know: + +1. **`tcp_err` releases the pcb upstream.** When lwIP fires + `tcp_err` for a fatal event (RST, OOM, ABRT), the pcb is gone + from lwIP's side *before* the callback runs. The wrapper's + `tcp_err` handler nulls its internal `Pcb` field and sets an + `Errored` flag. The next `Stream_Send` returns `false`; the next + `Stream_Read` returns `-1`. Crucially, calling `tcp_close` on a + pcb that was already released by `tcp_err` is a **use-after-free + in lwIP** — the wrapper guards against this with a `Pcb != NULL` + check before `tcp_close`. **You never see this rule** unless you + bypass the abstraction and poke at lwIP pcbs directly through + your own code — don't. + +2. **Peer FIN (`tcp_recv` with `p == NULL`) drains before EOF.** + The half-close sets `Errored`; the next `Stream_Read` that finds + the queue empty returns `-1` and internally `tcp_close`s the + pcb. Already-queued bytes drain first. + +3. **`Close` is idempotent.** Second `Close` is a no-op. `Destroy` + internally calls `Close` (which drains the RX queue's pbufs and + then `tcp_close`s if the pcb is still around), then overwrites + the abstract base with `SolidSyslogNullStream` so use-after- + destroy is a safe no-op rather than a NULL-fn-pointer crash. + +--- + +## Tunables + +All tunables live in `Core/Interface/SolidSyslogTunablesDefaults.h`. +Override by `#define`ing them in a user-tunables header passed via +the `SOLIDSYSLOG_USER_TUNABLES_FILE` CMake variable. + +| Tunable | Default | Adjust when | +|---|---|---| +| `SOLIDSYSLOG_LWIP_RAW_RESOLVER_POOL_SIZE` | `1U` | You need multiple concurrent resolver instances (rare). | +| `SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE` | `1U` | You wire more than one UDP sender. | +| `SOLIDSYSLOG_LWIP_RAW_TCP_STREAM_POOL_SIZE` | `2U` | You wire more than the canonical plain-TCP + TLS-underlying-TCP pair. | +| `SOLIDSYSLOG_ADDRESS_POOL_SIZE` | `3U` | Shared with PlusTcp / Posix / Winsock — bump if you need >3 concurrent destinations. | +| `SOLIDSYSLOG_TCP_CONNECT_TIMEOUT_MS` | `200U` | Default suits loopback / LAN. Raise for WAN deployments behind a high-RTT link; or install a runtime `GetConnectTimeoutMs` getter for per-instance tuning. | +| `SOLIDSYSLOG_LWIP_RAW_TCP_CONNECT_POLL_MS` | `10U` | Default gives 20 polls inside the 200 ms connect deadline. Lower it to notice a fast connect sooner; raise it to reduce spin overhead on a constrained MCU. | +| `SOLIDSYSLOG_LWIP_RAW_TCP_RX_QUEUE_SIZE` | `8U` | Sized for the typical mTLS handshake flight. Bump for streaming server responses; lower if `MEMP_NUM_PBUF` is tight. | + +--- + +## What this guide does not cover + +- **DNS** — currently out of scope. `SolidSyslogLwipRawResolver` only + parses numeric IPv4. `SolidSyslogLwipRawDnsResolver` lands in + S28.07. +- **IPv6** — the current Address / Resolver are IPv4-only. +- **Multi-`netif` routing** — neither Datagram nor TcpStream selects + an output interface; lwIP's routing table decides. +- **Jumbo-frame MTU discovery** — `Datagram_MaxPayload` returns + `SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD` (1232 bytes) unconditionally. +- **BDD coverage** — the FreeRTOS-on-lwIP BDD target arrives in + S28.06. diff --git a/docs/misra-deviations.md b/docs/misra-deviations.md index b6492a2b..5cbf3aeb 100644 --- a/docs/misra-deviations.md +++ b/docs/misra-deviations.md @@ -172,6 +172,33 @@ lifecycle is fundamentally per-call, not per-class. Rules 11.2 / 11.3 fire on the cast between `SolidSyslogFormatterStorage*` and `struct SolidSyslogFormatter*`. +#### (c) Third-party callback `void*` arg — `SelfFromArg` and byte-buffer reinterprets + +Several wrapped libraries expose callback-style APIs where we register a +function pointer plus an opaque `void*` context that the library passes +back to us when it invokes the callback. mbedTLS's `mbedtls_ssl_set_bio` +hands us back `void* ctx` in `BioSend` / `BioRecv`; lwIP Raw's +`tcp_arg(pcb, self)` hands us back `void* arg` in every `tcp_recv` / +`tcp_err` / `tcp_connected` / `tcp_sent` callback we registered. The +implementation has to cast that `void*` back to the concrete +implementation struct to do any work — Rule 11.5 (advisory) fires on +every such cast. + +This is structurally the same OO-in-C downcast as (a) — the library API +is the "base" type (`void*`), our struct is the "derived" type — just +happening at the callback boundary rather than the vtable-method +boundary. Each affected wrapper concentrates the cast in a single +`SelfFromArg`-style helper so the suppression has one site per class, +not one per callback (see `LwipRawTcpStream_SelfFromArg` for the +canonical shape). + +A second 11.5 sub-case the library leans on is the +`void* ↔ const uint8_t*` cast in `SolidSyslogUdpSender` when trimming +codepoint-boundary bytes from the caller's `const void*` buffer — a +byte-buffer reinterpretation that crosses the same advisory rule. The +third-party API contract (the public `Send` / `SendTo` interface) is +`void*` for opacity; the byte-level work needs a concrete unit type. + ### Rationale The pool migration under E11 / E24 retired the caller-supplied-storage @@ -197,7 +224,11 @@ opaque-type design). the per-platform Address downcast is similarly locked down because the pool slot is statically-typed `struct SolidSyslogAddress`, so the cast back from the opaque `struct SolidSyslogAddress*` cannot - lie. + lie. For (c) callback `void*` args, the pointer that goes out via + the registration call (e.g. `tcp_arg(pcb, self)`, + `mbedtls_ssl_set_bio(..., self, ...)`) is the same pointer that + comes back — the library is a pass-through; the cast can only + succeed against the type the wrapper passed in. - **Alignment** — Storage types are declared as `intptr_t storage[N]` (or a struct of the same shape), giving alignment at least as strict as any pointer or scalar the impl contains. The cast is therefore diff --git a/misra_suppressions.txt b/misra_suppressions.txt index debf789b..76728b36 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -42,6 +42,7 @@ misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:31 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c:34 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c:54 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:43 +misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:110 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddress.c:10 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:19 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:26 @@ -74,6 +75,7 @@ misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWindowsMutex.c:34 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:94 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:181 misra-c2012-11.5:Core/Source/SolidSyslogUdpSender.c:205 +misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:119 misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:271 misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:283 misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:138 diff --git a/scripts/misra_renumber.py b/scripts/misra_renumber.py index 9f72bb22..ca3bb23b 100755 --- a/scripts/misra_renumber.py +++ b/scripts/misra_renumber.py @@ -76,6 +76,7 @@ "-IPlatform/MbedTls/Interface", "-IPlatform/FreeRtos/Interface", "-IPlatform/PlusTcp/Interface", + "-IPlatform/LwipRaw/Interface", "-IPlatform/FatFs/Interface", "--xml", "--xml-version=2", @@ -87,6 +88,7 @@ "Platform/MbedTls/Source/", "Platform/FreeRtos/Source/", "Platform/PlusTcp/Source/", + "Platform/LwipRaw/Source/", "Platform/FatFs/Source/", ] From 5d579c2715c499ef42580f6f9c2537c013c9f3a0 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 28 May 2026 10:54:44 +0000 Subject: [PATCH 6/8] =?UTF-8?q?fix:=20S28.05=20PR=20#466=20review=20?= =?UTF-8?q?=E2=80=94=20IWYU=20fwd-decl,=20tidy,=20CodeRabbit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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(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) --- .../Source/SolidSyslogLwipRawTcpStream.c | 1 + Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 17 ++++++++++------- Tests/Support/LwipFakes/Source/LwipTcpFake.c | 10 +++++++++- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c index c85e0cbb..40d9cb0c 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c @@ -17,6 +17,7 @@ #include "SolidSyslogStream.h" #include "SolidSyslogTunables.h" +struct SolidSyslogAddress; struct SolidSyslogStream; static uint32_t LwipRawTcpStream_NullConnectTimeoutGetter(void* context); diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index 078d245c..f7d2aeb1 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -30,6 +30,7 @@ using namespace CososoTesting; static const uint16_t TEST_PORT = 514; +// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) // Asserts handle is non-null and not one of the slots in pool. #define CHECK_IS_FALLBACK(handle, pool) \ do \ @@ -56,6 +57,7 @@ static const uint16_t TEST_PORT = 514; // tcp_new — proves the wrapper forwarded the right handle. `getter` is // the LwipTcpFake_LastXxxPcb accessor function (zero-arg). #define CHECK_FORWARDED_PCB(getter) POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) +// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) namespace { @@ -132,14 +134,14 @@ TEST_BASE(LwipRawTcpStreamTestBase) /* Send through the abstract base against the shared sendBuffer. Default * length 1 covers the lifecycle / pcb-forwarding tests that don't care * about content; size-specific tests pass it explicitly. */ - bool sendBytes(size_t length = 1U) const + [[nodiscard]] bool sendBytes(size_t length = 1U) const { return SolidSyslogStream_Send(stream, sendBuffer, length); } /* Read through the abstract base into the shared readBuffer. Default * capacity is the full buffer; partial-drain tests pass it explicitly. */ - SolidSyslogSsize readBytes(size_t capacity = sizeof(readBuffer)) + [[nodiscard]] SolidSyslogSsize readBytes(size_t capacity = sizeof(readBuffer)) { return SolidSyslogStream_Read(stream, readBuffer, capacity); } @@ -151,8 +153,9 @@ TEST_BASE(LwipRawTcpStreamTestBase) * the wrapper took ownership of the pbuf (leak counter bumped here); * non-ERR_OK means lwIP retains the pbuf and the counter stays put, * so backpressure tests can pin the contract without imbalance. */ - err_t pushIncomingPbuf(struct pbuf* p, const void* data, uint16_t len) + static err_t pushIncomingPbuf(struct pbuf* p, const void* data, uint16_t len) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) -- pbuf->payload is void*; tests pass string literals p->payload = const_cast(data); p->len = len; p->tot_len = len; @@ -168,7 +171,7 @@ TEST_BASE(LwipRawTcpStreamTestBase) /* Drive the wrapper's tcp_recv callback with NULL p — peer half-close * (FIN). lwIP retains the pcb; only the receive half is gone. */ - void pushPeerFin() const + static void pushPeerFin() { tcp_recv_fn recvCb = LwipTcpFake_LastRecvFn(); (void) recvCb(LwipTcpFake_LastCallbackArg(), LwipTcpFake_LastTcpNewReturned(), nullptr, ERR_OK); @@ -177,7 +180,7 @@ TEST_BASE(LwipRawTcpStreamTestBase) /* Drive the wrapper's tcp_err callback — lwIP releases the pcb * upstream before this fires, so the leak invariant needs the * matching NotePcbReleasedByErr. */ - void pushTcpErr(int8_t err) const + static void pushTcpErr(int8_t err) { tcp_err_fn errCb = LwipTcpFake_LastErrFn(); errCb(LwipTcpFake_LastCallbackArg(), (err_t) err); @@ -484,7 +487,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpWriteWithCopyFlagAndSize) { memcpy(sendBuffer, "hello", 5); - sendBytes(5); + (void) sendBytes(5); CALLED_FAKE(LwipTcpFake_TcpWrite, ONCE); CHECK_FORWARDED_PCB(LwipTcpFake_LastWritePcb); @@ -495,7 +498,7 @@ TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpWriteWithCopyFlagAndSize) TEST(SolidSyslogLwipRawTcpStreamConnected, SendCallsTcpOutputAfterTcpWrite) { - sendBytes(); + (void) sendBytes(); CALLED_FAKE(LwipTcpFake_TcpOutput, ONCE); CHECK_FORWARDED_PCB(LwipTcpFake_LastOutputPcb); diff --git a/Tests/Support/LwipFakes/Source/LwipTcpFake.c b/Tests/Support/LwipFakes/Source/LwipTcpFake.c index 00d75b3e..7f503b06 100644 --- a/Tests/Support/LwipFakes/Source/LwipTcpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipTcpFake.c @@ -345,6 +345,14 @@ void tcp_sent(struct tcp_pcb* pcb, tcp_sent_fn sent) lastSentFn = sent; } +/* Default tcp_connect semantics: when configured to succeed (no immediate + * error injected, the test wants the cb to fire, and a cb was registered), + * synchronously invoke the connected callback with the configured result. */ +static inline bool ShouldFireConnectCallback(tcp_connected_fn connected) +{ + return (tcpConnectError == ERR_OK) && connectCallbackFires && (connected != NULL); +} + err_t tcp_connect(struct tcp_pcb* pcb, const ip_addr_t* ipaddr, u16_t port, tcp_connected_fn connected) { ++tcpConnectCallCount; @@ -352,7 +360,7 @@ err_t tcp_connect(struct tcp_pcb* pcb, const ip_addr_t* ipaddr, u16_t port, tcp_ lastConnectIpaddr = ipaddr; lastConnectPort = port; lastConnectedFn = connected; - if ((tcpConnectError == ERR_OK) && connectCallbackFires && (connected != NULL)) + if (ShouldFireConnectCallback(connected)) { (void) connected(lastCallbackArg, pcb, connectCallbackResult); } From b7e9b8b720fd394b8259b924ccd4fcb269dc54ca Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 28 May 2026 11:57:12 +0000 Subject: [PATCH 7/8] =?UTF-8?q?fix:=20S28.05=20PR=20#466=20review=20?= =?UTF-8?q?=E2=80=94=20clang-format=20+=20MISRA=20line=20drift?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 4 +++- misra_suppressions.txt | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index f7d2aeb1..42a07ea9 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -56,7 +56,9 @@ static const uint16_t TEST_PORT = 514; // Asserts the lwIP API call recorded the pcb the wrapper got back from // tcp_new — proves the wrapper forwarded the right handle. `getter` is // the LwipTcpFake_LastXxxPcb accessor function (zero-arg). -#define CHECK_FORWARDED_PCB(getter) POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) +#define CHECK_FORWARDED_PCB(getter) \ + POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) + // NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) namespace diff --git a/misra_suppressions.txt b/misra_suppressions.txt index 76728b36..ca264641 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -42,7 +42,7 @@ misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:31 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c:34 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c:54 misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:43 -misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:110 +misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:111 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddress.c:10 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:19 misra-c2012-11.3:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:26 @@ -75,7 +75,7 @@ misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWindowsMutex.c:34 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:94 misra-c2012-11.3:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:181 misra-c2012-11.5:Core/Source/SolidSyslogUdpSender.c:205 -misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:119 +misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:120 misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:271 misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:283 misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:138 From ae77418b15ff2c9fe8088515d588a34af0a85890 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Thu, 28 May 2026 12:43:46 +0000 Subject: [PATCH 8/8] =?UTF-8?q?fix:=20S28.05=20PR=20#466=20=E2=80=94=20cla?= =?UTF-8?q?ng-format=20off/on=20around=20CHECK=5FFORWARDED=5FPCB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp index 42a07ea9..90f41c5d 100644 --- a/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp @@ -56,8 +56,9 @@ static const uint16_t TEST_PORT = 514; // Asserts the lwIP API call recorded the pcb the wrapper got back from // tcp_new — proves the wrapper forwarded the right handle. `getter` is // the LwipTcpFake_LastXxxPcb accessor function (zero-arg). -#define CHECK_FORWARDED_PCB(getter) \ - POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) +// clang-format off +#define CHECK_FORWARDED_PCB(getter) POINTERS_EQUAL(LwipTcpFake_LastTcpNewReturned(), getter()) +// clang-format on // NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)