From 46ede5a4912141f55355443976967b7366a1e0e3 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 05:59:30 +0000 Subject: [PATCH 1/7] chore: S28.04 LwipRawDatagram pool plumbing + tests + tunable Co-Authored-By: Claude Opus 4.7 (1M context) --- Core/Interface/SolidSyslogTunablesDefaults.h | 19 ++ .../Interface/SolidSyslogLwipRawDatagram.h | 15 ++ .../SolidSyslogLwipRawDatagramErrors.h | 21 ++ .../Source/SolidSyslogLwipRawDatagram.c | 13 ++ .../SolidSyslogLwipRawDatagramMessages.c | 23 ++ .../SolidSyslogLwipRawDatagramPrivate.h | 14 ++ .../Source/SolidSyslogLwipRawDatagramStatic.c | 81 +++++++ Tests/CMakeLists.txt | 1 + Tests/Lwip/CMakeLists.txt | 29 +++ Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 209 ++++++++++++++++++ 10 files changed, 425 insertions(+) create mode 100644 Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagram.h create mode 100644 Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagramErrors.h create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h create mode 100644 Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c create mode 100644 Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp diff --git a/Core/Interface/SolidSyslogTunablesDefaults.h b/Core/Interface/SolidSyslogTunablesDefaults.h index 92056deb..de472072 100644 --- a/Core/Interface/SolidSyslogTunablesDefaults.h +++ b/Core/Interface/SolidSyslogTunablesDefaults.h @@ -569,6 +569,25 @@ #error "SOLIDSYSLOG_LWIP_RAW_RESOLVER_POOL_SIZE must be >= 1" #endif +/* + * Number of SolidSyslogLwipRawDatagram instances the library's internal + * static pool can simultaneously hold. Each instance carries a single + * struct udp_pcb pointer the wrapper holds across Open/SendTo/Close. + * + * Default 1 — almost all lwIP Raw integrators wire a single datagram into + * a UdpSender. Bump via SOLIDSYSLOG_USER_TUNABLES_FILE if more than one + * is genuinely needed. + * + * Floor: 1. Sub-floor values rejected at compile time. + */ +#ifndef SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE +#define SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE 1U +#endif + +#if SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE < 1 +#error "SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_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/SolidSyslogLwipRawDatagram.h b/Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagram.h new file mode 100644 index 00000000..507f0f1d --- /dev/null +++ b/Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagram.h @@ -0,0 +1,15 @@ +#ifndef SOLIDSYSLOGLWIPRAWDATAGRAM_H +#define SOLIDSYSLOGLWIPRAWDATAGRAM_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogDatagram; + + struct SolidSyslogDatagram* SolidSyslogLwipRawDatagram_Create(void); + void SolidSyslogLwipRawDatagram_Destroy(struct SolidSyslogDatagram * base); + +EXTERN_C_END + +#endif /* SOLIDSYSLOGLWIPRAWDATAGRAM_H */ diff --git a/Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagramErrors.h b/Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagramErrors.h new file mode 100644 index 00000000..0c922cd3 --- /dev/null +++ b/Platform/LwipRaw/Interface/SolidSyslogLwipRawDatagramErrors.h @@ -0,0 +1,21 @@ +#ifndef SOLIDSYSLOGLWIPRAWDATAGRAMERRORS_H +#define SOLIDSYSLOGLWIPRAWDATAGRAMERRORS_H + +#include "ExternC.h" + +EXTERN_C_BEGIN + + struct SolidSyslogErrorSource; + + enum SolidSyslogLwipRawDatagramErrors + { + LWIPRAWDATAGRAM_ERROR_POOL_EXHAUSTED, + LWIPRAWDATAGRAM_ERROR_UNKNOWN_DESTROY, + LWIPRAWDATAGRAM_ERROR_MAX + }; + + extern const struct SolidSyslogErrorSource LwipRawDatagramErrorSource; + +EXTERN_C_END + +#endif /* SOLIDSYSLOGLWIPRAWDATAGRAMERRORS_H */ diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c new file mode 100644 index 00000000..039ec912 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c @@ -0,0 +1,13 @@ +#include "SolidSyslogLwipRawDatagramPrivate.h" + +#include "SolidSyslogDatagramDefinition.h" + +void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base) +{ + (void) base; +} + +void LwipRawDatagram_Cleanup(struct SolidSyslogDatagram* base) +{ + (void) base; +} diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c new file mode 100644 index 00000000..de56f0c5 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c @@ -0,0 +1,23 @@ +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogLwipRawDatagramErrors.h" + +static const char* LwipRawDatagramError_AsString(uint8_t code) +{ + static const char* const messages[LWIPRAWDATAGRAM_ERROR_MAX] = { + [LWIPRAWDATAGRAM_ERROR_POOL_EXHAUSTED] = + "SolidSyslogLwipRawDatagram_Create pool exhausted; returning fallback NullDatagram", + [LWIPRAWDATAGRAM_ERROR_UNKNOWN_DESTROY] = + "SolidSyslogLwipRawDatagram_Destroy called with a handle not issued by this pool", + }; + const char* result = "unknown"; + if (code < (uint8_t) LWIPRAWDATAGRAM_ERROR_MAX) + { + enum SolidSyslogLwipRawDatagramErrors typed = (enum SolidSyslogLwipRawDatagramErrors) code; + result = messages[typed]; + } + return result; +} + +const struct SolidSyslogErrorSource LwipRawDatagramErrorSource = {"LwipRawDatagram", LwipRawDatagramError_AsString}; diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h new file mode 100644 index 00000000..df5a6a47 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h @@ -0,0 +1,14 @@ +#ifndef SOLIDSYSLOGLWIPRAWDATAGRAMPRIVATE_H +#define SOLIDSYSLOGLWIPRAWDATAGRAMPRIVATE_H + +#include "SolidSyslogDatagramDefinition.h" + +struct SolidSyslogLwipRawDatagram +{ + struct SolidSyslogDatagram Base; +}; + +void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base); +void LwipRawDatagram_Cleanup(struct SolidSyslogDatagram* base); + +#endif /* SOLIDSYSLOGLWIPRAWDATAGRAMPRIVATE_H */ diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c new file mode 100644 index 00000000..2031ab43 --- /dev/null +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c @@ -0,0 +1,81 @@ +#include "SolidSyslogLwipRawDatagram.h" + +#include +#include +#include + +#include "SolidSyslogError.h" +#include "SolidSyslogLwipRawDatagramErrors.h" +#include "SolidSyslogLwipRawDatagramPrivate.h" +#include "SolidSyslogNullDatagram.h" +#include "SolidSyslogPoolAllocator.h" +#include "SolidSyslogPrival.h" +#include "SolidSyslogTunables.h" + +struct SolidSyslogDatagram; + +static inline size_t LwipRawDatagram_IndexFromHandle(const struct SolidSyslogDatagram* base); +static inline void LwipRawDatagram_CleanupAtIndex(size_t index, void* context); + +static bool LwipRawDatagram_InUse[SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE]; +static struct SolidSyslogLwipRawDatagram LwipRawDatagram_Pool[SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE]; +static struct SolidSyslogPoolAllocator LwipRawDatagram_Allocator = { + LwipRawDatagram_InUse, + SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE +}; + +struct SolidSyslogDatagram* SolidSyslogLwipRawDatagram_Create(void) +{ + size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&LwipRawDatagram_Allocator); + struct SolidSyslogDatagram* handle = SolidSyslogNullDatagram_Get(); + if (SolidSyslogPoolAllocator_IndexIsValid(&LwipRawDatagram_Allocator, index) == true) + { + LwipRawDatagram_Initialise(&LwipRawDatagram_Pool[index].Base); + handle = &LwipRawDatagram_Pool[index].Base; + } + else + { + SolidSyslog_Error( + SOLIDSYSLOG_SEVERITY_ERROR, + &LwipRawDatagramErrorSource, + (uint8_t) LWIPRAWDATAGRAM_ERROR_POOL_EXHAUSTED + ); + } + return handle; +} + +void SolidSyslogLwipRawDatagram_Destroy(struct SolidSyslogDatagram* base) +{ + size_t index = LwipRawDatagram_IndexFromHandle(base); + bool released = + SolidSyslogPoolAllocator_IndexIsValid(&LwipRawDatagram_Allocator, index) && + SolidSyslogPoolAllocator_FreeIfInUse(&LwipRawDatagram_Allocator, index, LwipRawDatagram_CleanupAtIndex, NULL); + if (!released) + { + SolidSyslog_Error( + SOLIDSYSLOG_SEVERITY_WARNING, + &LwipRawDatagramErrorSource, + (uint8_t) LWIPRAWDATAGRAM_ERROR_UNKNOWN_DESTROY + ); + } +} + +static inline size_t LwipRawDatagram_IndexFromHandle(const struct SolidSyslogDatagram* base) +{ + size_t result = SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE; + for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE; poolIndex++) + { + if (base == &LwipRawDatagram_Pool[poolIndex].Base) + { + result = poolIndex; + break; + } + } + return result; +} + +static inline void LwipRawDatagram_CleanupAtIndex(size_t index, void* context) +{ + (void) context; + LwipRawDatagram_Cleanup(&LwipRawDatagram_Pool[index].Base); +} diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 649a65ed..9a8135b3 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -271,6 +271,7 @@ endforeach() foreach(lwip_test_target IN ITEMS SolidSyslogLwipRawAddressTest SolidSyslogLwipRawResolverTest + SolidSyslogLwipRawDatagramTest ) if(TARGET ${lwip_test_target}) register_junit_test(${lwip_test_target}) diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index 87c83e88..ac81c31a 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -100,3 +100,32 @@ set_target_properties(SolidSyslogLwipRawResolverTest PROPERTIES ) add_test(NAME SolidSyslogLwipRawResolverTest COMMAND SolidSyslogLwipRawResolverTest) + +# ---- SolidSyslogLwipRawDatagramTest ---- + +add_executable(SolidSyslogLwipRawDatagramTest + SolidSyslogLwipRawDatagramTest.cpp + main.cpp + ${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/SolidSyslogLwipRawDatagram.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramMessages.c + ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c +) + +target_link_libraries(SolidSyslogLwipRawDatagramTest PRIVATE + ${PROJECT_NAME} + ConfigLockFake + ErrorHandlerFake + CppUTest + CppUTestExt +) + +target_include_directories(SolidSyslogLwipRawDatagramTest PRIVATE ${LWIP_TEST_INCLUDE_DIRS}) + +set_target_properties(SolidSyslogLwipRawDatagramTest PROPERTIES + CXX_CPPCHECK "" +) + +add_test(NAME SolidSyslogLwipRawDatagramTest COMMAND SolidSyslogLwipRawDatagramTest) diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp new file mode 100644 index 00000000..f70169ae --- /dev/null +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -0,0 +1,209 @@ +#include "TestUtils.h" +#include "CppUTest/TestHarness.h" + +using namespace CososoTesting; + +#include "ConfigLockFake.h" +#include "ErrorHandlerFake.h" +#include "SolidSyslogDatagram.h" +#include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogLwipRawAddress.h" +#include "SolidSyslogLwipRawDatagram.h" +#include "SolidSyslogLwipRawDatagramErrors.h" +#include "SolidSyslogNullDatagram.h" +#include "SolidSyslogPrival.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). +// Use after the act-phase of a test that expects exactly one SolidSyslog_Error call. +#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) + +// clang-format off +TEST_GROUP(SolidSyslogLwipRawDatagram) +{ + struct SolidSyslogDatagram* datagram = nullptr; + struct SolidSyslogAddress* address = nullptr; + + void setup() override + { + datagram = SolidSyslogLwipRawDatagram_Create(); + address = SolidSyslogLwipRawAddress_Create(); + } + + void teardown() override + { + SolidSyslogLwipRawAddress_Destroy(address); + SolidSyslogLwipRawDatagram_Destroy(datagram); + } +}; +// clang-format on + +TEST(SolidSyslogLwipRawDatagram, CreateReturnsNonNullDatagram) +{ + CHECK(datagram != nullptr); +} + +TEST(SolidSyslogLwipRawDatagram, DestroyReleasesSlotToPool) +{ + SolidSyslogLwipRawDatagram_Destroy(datagram); + + datagram = SolidSyslogLwipRawDatagram_Create(); + + CHECK(datagram != SolidSyslogNullDatagram_Get()); +} + +// clang-format off +TEST_GROUP(SolidSyslogLwipRawDatagramPool) +{ + struct SolidSyslogDatagram* pooled[SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE] = {}; + struct SolidSyslogDatagram* overflow = nullptr; + + void teardown() override + { + for (auto* handle : pooled) + { + if (handle != nullptr) + { + SolidSyslogLwipRawDatagram_Destroy(handle); + } + } + if (overflow != nullptr) + { + SolidSyslogLwipRawDatagram_Destroy(overflow); + } + ConfigLockFake_Uninstall(); + } + + void FillPool() + { + for (auto*& slot : pooled) + { + slot = SolidSyslogLwipRawDatagram_Create(); + } + } +}; +// clang-format on + +TEST(SolidSyslogLwipRawDatagramPool, FillingPoolThenOverflowReturnsDistinctFallback) +{ + FillPool(); + + overflow = SolidSyslogLwipRawDatagram_Create(); + + CHECK_IS_FALLBACK(overflow, pooled); +} + +TEST(SolidSyslogLwipRawDatagramPool, ExhaustedCreateReportsError) +{ + ErrorHandlerFake_Install(nullptr); + FillPool(); + + overflow = SolidSyslogLwipRawDatagram_Create(); + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_ERROR, LwipRawDatagramErrorSource, LWIPRAWDATAGRAM_ERROR_POOL_EXHAUSTED); +} + +TEST(SolidSyslogLwipRawDatagramPool, FallbackVtableMethodsAreNoOps) +{ + FillPool(); + overflow = SolidSyslogLwipRawDatagram_Create(); + struct SolidSyslogAddress* localAddr = SolidSyslogLwipRawAddress_Create(); + + /* NullDatagram's Open returns true so caller success paths are not + * tripped; SendTo returns SENT; no underlying lwIP API is invoked + * because the production-side vtable is never wired on the fallback + * handle. */ + CHECK_TRUE(SolidSyslogDatagram_Open(overflow)); + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, + SolidSyslogDatagram_SendTo(overflow, "x", 1, localAddr) + ); + SolidSyslogDatagram_Close(overflow); + + SolidSyslogLwipRawAddress_Destroy(localAddr); +} + +TEST(SolidSyslogLwipRawDatagramPool, CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot) +{ + ConfigLockFake_Install(); + + pooled[0] = SolidSyslogLwipRawDatagram_Create(); + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogLwipRawDatagramPool, CreateLocksOncePerSlotProbedWhenPoolIsFull) +{ + FillPool(); + ConfigLockFake_Install(); + + overflow = SolidSyslogLwipRawDatagram_Create(); + + LONGS_EQUAL(SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE, ConfigLockFake_LockCallCount()); + LONGS_EQUAL(SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE, ConfigLockFake_UnlockCallCount()); +} + +TEST(SolidSyslogLwipRawDatagramPool, DestroyOfPooledHandleLocksOnce) +{ + pooled[0] = SolidSyslogLwipRawDatagram_Create(); + ConfigLockFake_Install(); + + SolidSyslogLwipRawDatagram_Destroy(pooled[0]); + pooled[0] = nullptr; + + CALLED_FAKE(ConfigLockFake_Lock, ONCE); + CALLED_FAKE(ConfigLockFake_Unlock, ONCE); +} + +TEST(SolidSyslogLwipRawDatagramPool, DestroyOfUnknownHandleDoesNotLock) +{ + ConfigLockFake_Install(); + struct SolidSyslogDatagram stranger = {}; + + SolidSyslogLwipRawDatagram_Destroy(&stranger); + + CALLED_FAKE(ConfigLockFake_Lock, NEVER); + CALLED_FAKE(ConfigLockFake_Unlock, NEVER); +} + +TEST(SolidSyslogLwipRawDatagramPool, DestroyOfUnknownHandleReportsWarning) +{ + ErrorHandlerFake_Install(nullptr); + struct SolidSyslogDatagram stranger = {}; + + SolidSyslogLwipRawDatagram_Destroy(&stranger); + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_WARNING, LwipRawDatagramErrorSource, LWIPRAWDATAGRAM_ERROR_UNKNOWN_DESTROY); +} + +TEST(SolidSyslogLwipRawDatagramPool, DestroyOfStaleHandleReportsWarning) +{ + pooled[0] = SolidSyslogLwipRawDatagram_Create(); + SolidSyslogLwipRawDatagram_Destroy(pooled[0]); + ErrorHandlerFake_Install(nullptr); + + SolidSyslogLwipRawDatagram_Destroy(pooled[0]); + pooled[0] = nullptr; + + CHECK_REPORTED(SOLIDSYSLOG_SEVERITY_WARNING, LwipRawDatagramErrorSource, LWIPRAWDATAGRAM_ERROR_UNKNOWN_DESTROY); +} From 16a44ae2ca94b0ad4e5db2433c0544cdbaa71a50 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 06:48:24 +0000 Subject: [PATCH 2/7] feat: S28.04 LwipRawDatagram Open/Close lifecycle + PCB tracking Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Source/SolidSyslogLwipRawDatagram.c | 51 +++++++++- .../SolidSyslogLwipRawDatagramPrivate.h | 3 + Tests/Lwip/CMakeLists.txt | 8 ++ Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 92 +++++++++++++++++++ .../Support/LwipFakes/Interface/LwipUdpFake.h | 27 ++++++ Tests/Support/LwipFakes/Source/LwipUdpFake.c | 60 ++++++++++++ 6 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 Tests/Support/LwipFakes/Interface/LwipUdpFake.h create mode 100644 Tests/Support/LwipFakes/Source/LwipUdpFake.c diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c index 039ec912..af4c1dd8 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c @@ -1,13 +1,60 @@ #include "SolidSyslogLwipRawDatagramPrivate.h" +#include +#include + +#include "lwip/udp.h" #include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogNullDatagram.h" + +static bool LwipRawDatagram_Open(struct SolidSyslogDatagram* base); +static void LwipRawDatagram_Close(struct SolidSyslogDatagram* base); + +static inline struct SolidSyslogLwipRawDatagram* LwipRawDatagram_SelfFromBase(struct SolidSyslogDatagram* base); +static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagram* self); void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base) { - (void) base; + struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base); + self->Base.Open = LwipRawDatagram_Open; + self->Base.Close = LwipRawDatagram_Close; + self->Pcb = NULL; +} + +static inline struct SolidSyslogLwipRawDatagram* LwipRawDatagram_SelfFromBase(struct SolidSyslogDatagram* base) +{ + return (struct SolidSyslogLwipRawDatagram*) base; } void LwipRawDatagram_Cleanup(struct SolidSyslogDatagram* base) { - (void) base; + LwipRawDatagram_Close(base); + /* Overwrite the abstract base with the shared NullDatagram vtable so + * use-after-destroy is a safe no-op rather than a NULL-fn-pointer crash. */ + *base = *SolidSyslogNullDatagram_Get(); +} + +static void LwipRawDatagram_Close(struct SolidSyslogDatagram* base) +{ + struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base); + if (LwipRawDatagram_IsOpen(self)) + { + udp_remove(self->Pcb); + self->Pcb = NULL; + } +} + +static bool LwipRawDatagram_Open(struct SolidSyslogDatagram* base) +{ + struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base); + if (!LwipRawDatagram_IsOpen(self)) + { + self->Pcb = udp_new(); + } + return LwipRawDatagram_IsOpen(self); +} + +static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagram* self) +{ + return self->Pcb != NULL; } diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h index df5a6a47..514f87fa 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramPrivate.h @@ -3,9 +3,12 @@ #include "SolidSyslogDatagramDefinition.h" +struct udp_pcb; + struct SolidSyslogLwipRawDatagram { struct SolidSyslogDatagram Base; + struct udp_pcb* Pcb; }; void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base); diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index ac81c31a..a56e5259 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -27,6 +27,13 @@ set(LWIP_TEST_INCLUDE_DIRS $ENV{LWIP_PATH}/src/include ) +# lwIP host-side fakes — recompiled into each test exe that needs them, mirroring +# the FreeRtosFakes pattern (sources live under Tests/Support/LwipFakes/Source/, +# pulled inline rather than wrapped as a static lib). +set(LWIP_FAKE_SOURCES + ${CMAKE_SOURCE_DIR}/Tests/Support/LwipFakes/Source/LwipUdpFake.c +) + # lwIP upstream parser. Wrapped as a STATIC library with cppcheck disabled # so the `toomanyconfigs` warning from opt.h's #ifdef forest doesn't fail # our build. We don't audit upstream lwIP code — that is lwIP's concern. @@ -106,6 +113,7 @@ add_test(NAME SolidSyslogLwipRawResolverTest COMMAND SolidSyslogLwipRawResolverT add_executable(SolidSyslogLwipRawDatagramTest SolidSyslogLwipRawDatagramTest.cpp main.cpp + ${LWIP_FAKE_SOURCES} ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddress.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddressMessages.c ${CMAKE_SOURCE_DIR}/Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index f70169ae..bc1d244e 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -5,6 +5,7 @@ using namespace CososoTesting; #include "ConfigLockFake.h" #include "ErrorHandlerFake.h" +#include "LwipUdpFake.h" #include "SolidSyslogDatagram.h" #include "SolidSyslogDatagramDefinition.h" #include "SolidSyslogLwipRawAddress.h" @@ -45,6 +46,7 @@ TEST_GROUP(SolidSyslogLwipRawDatagram) void setup() override { + LwipUdpFake_Reset(); datagram = SolidSyslogLwipRawDatagram_Create(); address = SolidSyslogLwipRawAddress_Create(); } @@ -71,6 +73,96 @@ TEST(SolidSyslogLwipRawDatagram, DestroyReleasesSlotToPool) CHECK(datagram != SolidSyslogNullDatagram_Get()); } +TEST(SolidSyslogLwipRawDatagram, OpenSucceeds) +{ + CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); +} + +TEST(SolidSyslogLwipRawDatagram, CloseSucceeds) +{ + SolidSyslogDatagram_Close(datagram); +} + +TEST(SolidSyslogLwipRawDatagram, OpenCallsUdpNew) +{ + SolidSyslogDatagram_Open(datagram); + + CALLED_FAKE(LwipUdpFake_UdpNew, ONCE); +} + +TEST(SolidSyslogLwipRawDatagram, CloseCallsUdpRemoveOnOpenPcb) +{ + SolidSyslogDatagram_Open(datagram); + + SolidSyslogDatagram_Close(datagram); + + CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); + POINTERS_EQUAL(LwipUdpFake_LastUdpRemovePcb(), LwipUdpFake_LastUdpNewReturned()); +} + +TEST(SolidSyslogLwipRawDatagram, OpenIsIdempotent) +{ + SolidSyslogDatagram_Open(datagram); + + CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); + CALLED_FAKE(LwipUdpFake_UdpNew, ONCE); +} + +TEST(SolidSyslogLwipRawDatagram, CloseIsIdempotent) +{ + SolidSyslogDatagram_Open(datagram); + SolidSyslogDatagram_Close(datagram); + + SolidSyslogDatagram_Close(datagram); + + CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); +} + +TEST(SolidSyslogLwipRawDatagram, CloseWithoutOpenIsNoOp) +{ + SolidSyslogDatagram_Close(datagram); + + CALLED_FAKE(LwipUdpFake_UdpRemove, NEVER); +} + +TEST(SolidSyslogLwipRawDatagram, OpenReturnsFalseWhenUdpNewFails) +{ + LwipUdpFake_SetUdpNewFails(true); + + CHECK_FALSE(SolidSyslogDatagram_Open(datagram)); +} + +TEST(SolidSyslogLwipRawDatagram, CloseAndReopenCreatesFreshPcb) +{ + SolidSyslogDatagram_Open(datagram); + SolidSyslogDatagram_Close(datagram); + + SolidSyslogDatagram_Open(datagram); + + CALLED_FAKE(LwipUdpFake_UdpNew, TWICE); +} + +TEST(SolidSyslogLwipRawDatagram, DestroyClosesOpenPcb) +{ + SolidSyslogDatagram_Open(datagram); + + SolidSyslogLwipRawDatagram_Destroy(datagram); + datagram = nullptr; + + CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); +} + +TEST(SolidSyslogLwipRawDatagram, DestroyAfterCloseDoesNotRemoveAgain) +{ + SolidSyslogDatagram_Open(datagram); + SolidSyslogDatagram_Close(datagram); + + SolidSyslogLwipRawDatagram_Destroy(datagram); + datagram = nullptr; + + CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); +} + // clang-format off TEST_GROUP(SolidSyslogLwipRawDatagramPool) { diff --git a/Tests/Support/LwipFakes/Interface/LwipUdpFake.h b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h new file mode 100644 index 00000000..f105cdb1 --- /dev/null +++ b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h @@ -0,0 +1,27 @@ +#ifndef LWIPUDPFAKE_H +#define LWIPUDPFAKE_H + +#include "ExternC.h" + +#include + +EXTERN_C_BEGIN + + struct udp_pcb; + + void LwipUdpFake_Reset(void); + + /* udp_new configuration */ + void LwipUdpFake_SetUdpNewFails(bool fails); + + /* udp_new spy */ + unsigned LwipUdpFake_UdpNewCallCount(void); + struct udp_pcb* LwipUdpFake_LastUdpNewReturned(void); + + /* udp_remove spy */ + unsigned LwipUdpFake_UdpRemoveCallCount(void); + struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void); + +EXTERN_C_END + +#endif /* LWIPUDPFAKE_H */ diff --git a/Tests/Support/LwipFakes/Source/LwipUdpFake.c b/Tests/Support/LwipFakes/Source/LwipUdpFake.c new file mode 100644 index 00000000..0560175c --- /dev/null +++ b/Tests/Support/LwipFakes/Source/LwipUdpFake.c @@ -0,0 +1,60 @@ +#include "LwipUdpFake.h" + +#include + +#include "lwip/udp.h" + +static unsigned udpNewCallCount = 0; +static struct udp_pcb fakePcb; +static struct udp_pcb* lastUdpNewReturned = NULL; +static bool udpNewFails = false; + +static unsigned udpRemoveCallCount = 0; +static struct udp_pcb* lastUdpRemovePcb = NULL; + +void LwipUdpFake_Reset(void) +{ + udpNewCallCount = 0; + lastUdpNewReturned = NULL; + udpNewFails = false; + udpRemoveCallCount = 0; + lastUdpRemovePcb = NULL; +} + +void LwipUdpFake_SetUdpNewFails(bool fails) +{ + udpNewFails = fails; +} + +unsigned LwipUdpFake_UdpNewCallCount(void) +{ + return udpNewCallCount; +} + +struct udp_pcb* LwipUdpFake_LastUdpNewReturned(void) +{ + return lastUdpNewReturned; +} + +unsigned LwipUdpFake_UdpRemoveCallCount(void) +{ + return udpRemoveCallCount; +} + +struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void) +{ + return lastUdpRemovePcb; +} + +struct udp_pcb* udp_new(void) +{ + ++udpNewCallCount; + lastUdpNewReturned = udpNewFails ? NULL : &fakePcb; + return lastUdpNewReturned; +} + +void udp_remove(struct udp_pcb* pcb) +{ + ++udpRemoveCallCount; + lastUdpRemovePcb = pcb; +} From cf9536f01e9291687c7f37da36f6cf462fe40752 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 08:15:54 +0000 Subject: [PATCH 3/7] =?UTF-8?q?refactor:=20S28.04=20LwipRawDatagram=20test?= =?UTF-8?q?s=20=E2=80=94=20shared=20base=20+=20leak=20invariant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the per-test Open/Close boilerplate into a TEST_BASE + TEST_GROUP_BASE pair: SolidSyslogLwipRawDatagram (Created-only) and SolidSyslogLwipRawDatagramOpen (Open lifted into setup). Test bodies now focus on the single behaviour under observation. Add LwipUdpFake_OutstandingPcbCount and check it in the shared teardown: every udp_pcb handed out by udp_new must return via udp_remove by the end of the test. Catches PCB leaks across all 13 lifecycle tests at zero per-test cost. Co-Authored-By: Claude Opus 4.7 (1M context) --- Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 98 +++++++++++++------ .../Support/LwipFakes/Interface/LwipUdpFake.h | 5 + Tests/Support/LwipFakes/Source/LwipUdpFake.c | 19 +++- 3 files changed, 91 insertions(+), 31 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index bc1d244e..ddd674b5 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -38,27 +38,64 @@ using namespace CososoTesting; UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastCode()); \ } while (0) +/* Shared fixture: every Datagram lifecycle test needs the fake reset, a fresh + * datagram + address handle pair, teardown of both, and the leak invariant + * — every udp_pcb handed out by udp_new must come back via udp_remove by the + * end of the test. TEST_GROUP_BASE keeps Created-only vs Opened groups + * sharing this boilerplate. */ // clang-format off -TEST_GROUP(SolidSyslogLwipRawDatagram) +TEST_BASE(LwipRawDatagramTestBase) { struct SolidSyslogDatagram* datagram = nullptr; struct SolidSyslogAddress* address = nullptr; - void setup() override + void createFakesAndHandles() { LwipUdpFake_Reset(); datagram = SolidSyslogLwipRawDatagram_Create(); address = SolidSyslogLwipRawAddress_Create(); } - void teardown() override + void destroyHandlesAndCheckNoLeak() const { SolidSyslogLwipRawAddress_Destroy(address); SolidSyslogLwipRawDatagram_Destroy(datagram); + LONGS_EQUAL_TEXT(0, LwipUdpFake_OutstandingPcbCount(), "leaked udp_pcb past teardown"); + } +}; + +TEST_GROUP_BASE(SolidSyslogLwipRawDatagram, LwipRawDatagramTestBase) +{ + void setup() override + { + createFakesAndHandles(); + } + + void teardown() override + { + destroyHandlesAndCheckNoLeak(); + } +}; + +TEST_GROUP_BASE(SolidSyslogLwipRawDatagramOpen, LwipRawDatagramTestBase) +{ + void setup() override + { + createFakesAndHandles(); + SolidSyslogDatagram_Open(datagram); + } + + void teardown() override + { + destroyHandlesAndCheckNoLeak(); } }; // clang-format on +/* ------------------------------------------------------------------ + * Created-but-not-opened tests + * ----------------------------------------------------------------*/ + TEST(SolidSyslogLwipRawDatagram, CreateReturnsNonNullDatagram) { CHECK(datagram != nullptr); @@ -78,7 +115,7 @@ TEST(SolidSyslogLwipRawDatagram, OpenSucceeds) CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); } -TEST(SolidSyslogLwipRawDatagram, CloseSucceeds) +TEST(SolidSyslogLwipRawDatagram, CloseSucceedsWithoutOpen) { SolidSyslogDatagram_Close(datagram); } @@ -90,51 +127,50 @@ TEST(SolidSyslogLwipRawDatagram, OpenCallsUdpNew) CALLED_FAKE(LwipUdpFake_UdpNew, ONCE); } -TEST(SolidSyslogLwipRawDatagram, CloseCallsUdpRemoveOnOpenPcb) +TEST(SolidSyslogLwipRawDatagram, CloseWithoutOpenIsNoOp) { - SolidSyslogDatagram_Open(datagram); - SolidSyslogDatagram_Close(datagram); - CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); - POINTERS_EQUAL(LwipUdpFake_LastUdpRemovePcb(), LwipUdpFake_LastUdpNewReturned()); + CALLED_FAKE(LwipUdpFake_UdpRemove, NEVER); } -TEST(SolidSyslogLwipRawDatagram, OpenIsIdempotent) +TEST(SolidSyslogLwipRawDatagram, OpenReturnsFalseWhenUdpNewFails) { - SolidSyslogDatagram_Open(datagram); + LwipUdpFake_SetUdpNewFails(true); - CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); - CALLED_FAKE(LwipUdpFake_UdpNew, ONCE); + CHECK_FALSE(SolidSyslogDatagram_Open(datagram)); } -TEST(SolidSyslogLwipRawDatagram, CloseIsIdempotent) -{ - SolidSyslogDatagram_Open(datagram); - SolidSyslogDatagram_Close(datagram); +/* ------------------------------------------------------------------ + * Already-opened tests (Open lifted into the group's setup) + * ----------------------------------------------------------------*/ +TEST(SolidSyslogLwipRawDatagramOpen, CloseCallsUdpRemoveOnOpenPcb) +{ SolidSyslogDatagram_Close(datagram); CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); + POINTERS_EQUAL(LwipUdpFake_LastUdpRemovePcb(), LwipUdpFake_LastUdpNewReturned()); } -TEST(SolidSyslogLwipRawDatagram, CloseWithoutOpenIsNoOp) +TEST(SolidSyslogLwipRawDatagramOpen, ReopenDoesNotAllocateNewPcb) { - SolidSyslogDatagram_Close(datagram); + CHECK_TRUE(SolidSyslogDatagram_Open(datagram)); - CALLED_FAKE(LwipUdpFake_UdpRemove, NEVER); + CALLED_FAKE(LwipUdpFake_UdpNew, ONCE); } -TEST(SolidSyslogLwipRawDatagram, OpenReturnsFalseWhenUdpNewFails) +TEST(SolidSyslogLwipRawDatagramOpen, SecondCloseDoesNotRemoveAgain) { - LwipUdpFake_SetUdpNewFails(true); + SolidSyslogDatagram_Close(datagram); - CHECK_FALSE(SolidSyslogDatagram_Open(datagram)); + SolidSyslogDatagram_Close(datagram); + + CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); } -TEST(SolidSyslogLwipRawDatagram, CloseAndReopenCreatesFreshPcb) +TEST(SolidSyslogLwipRawDatagramOpen, CloseThenOpenAllocatesFreshPcb) { - SolidSyslogDatagram_Open(datagram); SolidSyslogDatagram_Close(datagram); SolidSyslogDatagram_Open(datagram); @@ -142,19 +178,16 @@ TEST(SolidSyslogLwipRawDatagram, CloseAndReopenCreatesFreshPcb) CALLED_FAKE(LwipUdpFake_UdpNew, TWICE); } -TEST(SolidSyslogLwipRawDatagram, DestroyClosesOpenPcb) +TEST(SolidSyslogLwipRawDatagramOpen, DestroyClosesOpenPcb) { - SolidSyslogDatagram_Open(datagram); - SolidSyslogLwipRawDatagram_Destroy(datagram); datagram = nullptr; CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); } -TEST(SolidSyslogLwipRawDatagram, DestroyAfterCloseDoesNotRemoveAgain) +TEST(SolidSyslogLwipRawDatagramOpen, DestroyAfterCloseDoesNotRemoveAgain) { - SolidSyslogDatagram_Open(datagram); SolidSyslogDatagram_Close(datagram); SolidSyslogLwipRawDatagram_Destroy(datagram); @@ -169,6 +202,11 @@ TEST_GROUP(SolidSyslogLwipRawDatagramPool) struct SolidSyslogDatagram* pooled[SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE] = {}; struct SolidSyslogDatagram* overflow = nullptr; + void setup() override + { + LwipUdpFake_Reset(); + } + void teardown() override { for (auto* handle : pooled) diff --git a/Tests/Support/LwipFakes/Interface/LwipUdpFake.h b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h index f105cdb1..2f9b9be1 100644 --- a/Tests/Support/LwipFakes/Interface/LwipUdpFake.h +++ b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h @@ -22,6 +22,11 @@ EXTERN_C_BEGIN unsigned LwipUdpFake_UdpRemoveCallCount(void); struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void); + /* Allocated-but-not-yet-freed PCB count. Successful udp_new bumps it; + * udp_remove decrements. A test that ends with a non-zero value has + * leaked a PCB — pin this in teardown to catch leaks across the suite. */ + int LwipUdpFake_OutstandingPcbCount(void); + EXTERN_C_END #endif /* LWIPUDPFAKE_H */ diff --git a/Tests/Support/LwipFakes/Source/LwipUdpFake.c b/Tests/Support/LwipFakes/Source/LwipUdpFake.c index 0560175c..e776e9a5 100644 --- a/Tests/Support/LwipFakes/Source/LwipUdpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipUdpFake.c @@ -12,6 +12,8 @@ static bool udpNewFails = false; static unsigned udpRemoveCallCount = 0; static struct udp_pcb* lastUdpRemovePcb = NULL; +static int outstandingPcbCount = 0; + void LwipUdpFake_Reset(void) { udpNewCallCount = 0; @@ -19,6 +21,7 @@ void LwipUdpFake_Reset(void) udpNewFails = false; udpRemoveCallCount = 0; lastUdpRemovePcb = NULL; + outstandingPcbCount = 0; } void LwipUdpFake_SetUdpNewFails(bool fails) @@ -46,10 +49,23 @@ struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void) return lastUdpRemovePcb; } +int LwipUdpFake_OutstandingPcbCount(void) +{ + return outstandingPcbCount; +} + struct udp_pcb* udp_new(void) { ++udpNewCallCount; - lastUdpNewReturned = udpNewFails ? NULL : &fakePcb; + if (udpNewFails) + { + lastUdpNewReturned = NULL; + } + else + { + lastUdpNewReturned = &fakePcb; + ++outstandingPcbCount; + } return lastUdpNewReturned; } @@ -57,4 +73,5 @@ void udp_remove(struct udp_pcb* pcb) { ++udpRemoveCallCount; lastUdpRemovePcb = pcb; + --outstandingPcbCount; } From 7652553a374047e12d38841e95a8b8a938b8978b Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 10:02:36 +0000 Subject: [PATCH 4/7] feat: S28.04 LwipRawDatagram SendTo via udp_sendto + PBUF_REF, plus MaxPayload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SendTo allocates a PBUF_REF pbuf pointing at the caller's buffer, calls udp_sendto, and frees the pbuf — on success and on every failure path. Guards: not-open returns FAILED; pbuf_alloc-NULL returns FAILED without sendto; udp_sendto error returns FAILED. MaxPayload returns the IPv6-safe 1232-byte default unconditionally (no netif MTU lookup). Test fixture grew an LwipPbufFake with the same shape as LwipUdpFake (spies, failure injection, OutstandingPbufCount). The shared teardown asserts both pcb and pbuf outstanding counts are zero, catching leaks across every lifecycle test. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Source/SolidSyslogLwipRawDatagram.c | 47 +++++++ Tests/Lwip/CMakeLists.txt | 1 + Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 131 ++++++++++++++++++ .../LwipFakes/Interface/LwipPbufFake.h | 35 +++++ .../Support/LwipFakes/Interface/LwipUdpFake.h | 14 ++ Tests/Support/LwipFakes/Source/LwipPbufFake.c | 98 +++++++++++++ Tests/Support/LwipFakes/Source/LwipUdpFake.c | 53 +++++++ 7 files changed, 379 insertions(+) create mode 100644 Tests/Support/LwipFakes/Interface/LwipPbufFake.h create mode 100644 Tests/Support/LwipFakes/Source/LwipPbufFake.c diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c index af4c1dd8..44468335 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c @@ -3,12 +3,24 @@ #include #include +#include "lwip/pbuf.h" #include "lwip/udp.h" #include "SolidSyslogDatagramDefinition.h" +#include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogNullDatagram.h" +#include "SolidSyslogUdpPayload.h" + +struct SolidSyslogAddress; static bool LwipRawDatagram_Open(struct SolidSyslogDatagram* base); static void LwipRawDatagram_Close(struct SolidSyslogDatagram* base); +static enum SolidSyslogDatagramSendResult LwipRawDatagram_SendTo( + struct SolidSyslogDatagram* base, + const void* buffer, + size_t size, + const struct SolidSyslogAddress* addr +); +static size_t LwipRawDatagram_MaxPayload(struct SolidSyslogDatagram* base); static inline struct SolidSyslogLwipRawDatagram* LwipRawDatagram_SelfFromBase(struct SolidSyslogDatagram* base); static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagram* self); @@ -18,6 +30,8 @@ void LwipRawDatagram_Initialise(struct SolidSyslogDatagram* base) struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base); self->Base.Open = LwipRawDatagram_Open; self->Base.Close = LwipRawDatagram_Close; + self->Base.SendTo = LwipRawDatagram_SendTo; + self->Base.MaxPayload = LwipRawDatagram_MaxPayload; self->Pcb = NULL; } @@ -58,3 +72,36 @@ static inline bool LwipRawDatagram_IsOpen(const struct SolidSyslogLwipRawDatagra { return self->Pcb != NULL; } + +static enum SolidSyslogDatagramSendResult LwipRawDatagram_SendTo( + struct SolidSyslogDatagram* base, + const void* buffer, + size_t size, + const struct SolidSyslogAddress* addr +) +{ + struct SolidSyslogLwipRawDatagram* self = LwipRawDatagram_SelfFromBase(base); + enum SolidSyslogDatagramSendResult result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED; + if (LwipRawDatagram_IsOpen(self)) + { + struct pbuf* p = pbuf_alloc(PBUF_TRANSPORT, (u16_t) size, PBUF_REF); + if (p != NULL) + { + const struct SolidSyslogLwipRawAddress* dst = SolidSyslogLwipRawAddress_AsConst(addr); + p->payload = (void*) buffer; + err_t err = udp_sendto(self->Pcb, p, &dst->Ip, dst->Port); + (void) pbuf_free(p); + if (err == ERR_OK) + { + result = SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT; + } + } + } + return result; +} + +static size_t LwipRawDatagram_MaxPayload(struct SolidSyslogDatagram* base) +{ + (void) base; + return SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD; +} diff --git a/Tests/Lwip/CMakeLists.txt b/Tests/Lwip/CMakeLists.txt index a56e5259..0eaa1af5 100644 --- a/Tests/Lwip/CMakeLists.txt +++ b/Tests/Lwip/CMakeLists.txt @@ -31,6 +31,7 @@ set(LWIP_TEST_INCLUDE_DIRS # the FreeRtosFakes pattern (sources live under Tests/Support/LwipFakes/Source/, # 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/LwipUdpFake.c ) diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index ddd674b5..41b355d5 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -5,15 +5,21 @@ using namespace CososoTesting; #include "ConfigLockFake.h" #include "ErrorHandlerFake.h" +#include "LwipPbufFake.h" #include "LwipUdpFake.h" #include "SolidSyslogDatagram.h" #include "SolidSyslogDatagramDefinition.h" #include "SolidSyslogLwipRawAddress.h" +#include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogLwipRawDatagram.h" #include "SolidSyslogLwipRawDatagramErrors.h" #include "SolidSyslogNullDatagram.h" #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" +#include "SolidSyslogUdpPayload.h" +#include "lwip/ip4_addr.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) \ @@ -52,8 +58,12 @@ TEST_BASE(LwipRawDatagramTestBase) void createFakesAndHandles() { LwipUdpFake_Reset(); + LwipPbufFake_Reset(); datagram = SolidSyslogLwipRawDatagram_Create(); address = SolidSyslogLwipRawAddress_Create(); + struct SolidSyslogLwipRawAddress* lwipAddress = SolidSyslogLwipRawAddress_As(address); + IP4_ADDR(&lwipAddress->Ip, 127, 0, 0, 1); + lwipAddress->Port = TEST_PORT; } void destroyHandlesAndCheckNoLeak() const @@ -61,6 +71,7 @@ TEST_BASE(LwipRawDatagramTestBase) SolidSyslogLwipRawAddress_Destroy(address); SolidSyslogLwipRawDatagram_Destroy(datagram); LONGS_EQUAL_TEXT(0, LwipUdpFake_OutstandingPcbCount(), "leaked udp_pcb past teardown"); + LONGS_EQUAL_TEXT(0, LwipPbufFake_OutstandingPbufCount(), "leaked pbuf past teardown"); } }; @@ -141,6 +152,21 @@ TEST(SolidSyslogLwipRawDatagram, OpenReturnsFalseWhenUdpNewFails) CHECK_FALSE(SolidSyslogDatagram_Open(datagram)); } +TEST(SolidSyslogLwipRawDatagram, MaxPayloadReturnsIpv6SafeDefault) +{ + LONGS_EQUAL(SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD, SolidSyslogDatagram_MaxPayload(datagram)); +} + +TEST(SolidSyslogLwipRawDatagram, SendToFailsBeforeOpen) +{ + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, + SolidSyslogDatagram_SendTo(datagram, "x", 1, address) + ); + CALLED_FAKE(LwipPbufFake_PbufAlloc, NEVER); + CALLED_FAKE(LwipUdpFake_UdpSendto, NEVER); +} + /* ------------------------------------------------------------------ * Already-opened tests (Open lifted into the group's setup) * ----------------------------------------------------------------*/ @@ -196,6 +222,111 @@ TEST(SolidSyslogLwipRawDatagramOpen, DestroyAfterCloseDoesNotRemoveAgain) CALLED_FAKE(LwipUdpFake_UdpRemove, ONCE); } +TEST(SolidSyslogLwipRawDatagramOpen, SendToSucceeds) +{ + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, + SolidSyslogDatagram_SendTo(datagram, "x", 1, address) + ); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToCallsUdpSendto) +{ + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + CALLED_FAKE(LwipUdpFake_UdpSendto, ONCE); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesPbuf) +{ + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + CALLED_FAKE(LwipPbufFake_PbufAlloc, ONCE); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesTransportLayerPbufRef) +{ + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + LONGS_EQUAL(PBUF_TRANSPORT, LwipPbufFake_LastAllocLayer()); + LONGS_EQUAL(PBUF_REF, LwipPbufFake_LastAllocType()); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToPbufPayloadPointsAtCallerBuffer) +{ + static const char buf[] = "hello"; + + SolidSyslogDatagram_SendTo(datagram, buf, sizeof(buf) - 1U, address); + + POINTERS_EQUAL(buf, LwipPbufFake_LastAllocReturned()->payload); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsAddressIpAndPort) +{ + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + POINTERS_EQUAL(&SolidSyslogLwipRawAddress_AsConst(address)->Ip, LwipUdpFake_LastSendtoIpaddr()); + LONGS_EQUAL(TEST_PORT, LwipUdpFake_LastSendtoPort()); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsLengthVerbatim) +{ + static const char buf[1232] = {}; + + SolidSyslogDatagram_SendTo(datagram, buf, 1, address); + LONGS_EQUAL(1, LwipPbufFake_LastAllocLength()); + + SolidSyslogDatagram_SendTo(datagram, buf, 1232, address); + LONGS_EQUAL(1232, LwipPbufFake_LastAllocLength()); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToFreesPbufAfterSendto) +{ + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsAfterClose) +{ + SolidSyslogDatagram_Close(datagram); + + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, + SolidSyslogDatagram_SendTo(datagram, "x", 1, address) + ); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsWhenPbufAllocFails) +{ + LwipPbufFake_SetPbufAllocFails(true); + + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, + SolidSyslogDatagram_SendTo(datagram, "x", 1, address) + ); + CALLED_FAKE(LwipUdpFake_UdpSendto, NEVER); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsWhenSendtoErrors) +{ + LwipUdpFake_SetUdpSendtoError(ERR_MEM); + + LONGS_EQUAL( + SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, + SolidSyslogDatagram_SendTo(datagram, "x", 1, address) + ); +} + +TEST(SolidSyslogLwipRawDatagramOpen, SendToFreesPbufEvenWhenSendtoErrors) +{ + LwipUdpFake_SetUdpSendtoError(ERR_MEM); + + SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + + CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); +} + // clang-format off TEST_GROUP(SolidSyslogLwipRawDatagramPool) { diff --git a/Tests/Support/LwipFakes/Interface/LwipPbufFake.h b/Tests/Support/LwipFakes/Interface/LwipPbufFake.h new file mode 100644 index 00000000..2d2c6aab --- /dev/null +++ b/Tests/Support/LwipFakes/Interface/LwipPbufFake.h @@ -0,0 +1,35 @@ +#ifndef LWIPPBUFFAKE_H +#define LWIPPBUFFAKE_H + +#include "ExternC.h" + +#include +#include + +#include "lwip/pbuf.h" + +EXTERN_C_BEGIN + + void LwipPbufFake_Reset(void); + + /* pbuf_alloc configuration */ + void LwipPbufFake_SetPbufAllocFails(bool fails); + + /* pbuf_alloc spy */ + unsigned LwipPbufFake_PbufAllocCallCount(void); + pbuf_layer LwipPbufFake_LastAllocLayer(void); + uint16_t LwipPbufFake_LastAllocLength(void); + pbuf_type LwipPbufFake_LastAllocType(void); + struct pbuf* LwipPbufFake_LastAllocReturned(void); + + /* pbuf_free spy */ + unsigned LwipPbufFake_PbufFreeCallCount(void); + + /* Allocated-but-not-yet-freed pbuf count. Successful pbuf_alloc bumps it; + * pbuf_free decrements. A test that ends with a non-zero value has + * leaked a pbuf — pin this in teardown to catch leaks across the suite. */ + int LwipPbufFake_OutstandingPbufCount(void); + +EXTERN_C_END + +#endif /* LWIPPBUFFAKE_H */ diff --git a/Tests/Support/LwipFakes/Interface/LwipUdpFake.h b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h index 2f9b9be1..be4ecb5e 100644 --- a/Tests/Support/LwipFakes/Interface/LwipUdpFake.h +++ b/Tests/Support/LwipFakes/Interface/LwipUdpFake.h @@ -4,10 +4,14 @@ #include "ExternC.h" #include +#include + +#include "lwip/ip_addr.h" EXTERN_C_BEGIN struct udp_pcb; + struct pbuf; void LwipUdpFake_Reset(void); @@ -22,6 +26,16 @@ EXTERN_C_BEGIN unsigned LwipUdpFake_UdpRemoveCallCount(void); struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void); + /* udp_sendto configuration */ + void LwipUdpFake_SetUdpSendtoError(int8_t err); + + /* udp_sendto spy */ + unsigned LwipUdpFake_UdpSendtoCallCount(void); + struct udp_pcb* LwipUdpFake_LastSendtoPcb(void); + struct pbuf* LwipUdpFake_LastSendtoPbuf(void); + const ip_addr_t* LwipUdpFake_LastSendtoIpaddr(void); + uint16_t LwipUdpFake_LastSendtoPort(void); + /* Allocated-but-not-yet-freed PCB count. Successful udp_new bumps it; * udp_remove decrements. A test that ends with a non-zero value has * leaked a PCB — pin this in teardown to catch leaks across the suite. */ diff --git a/Tests/Support/LwipFakes/Source/LwipPbufFake.c b/Tests/Support/LwipFakes/Source/LwipPbufFake.c new file mode 100644 index 00000000..bc9c995b --- /dev/null +++ b/Tests/Support/LwipFakes/Source/LwipPbufFake.c @@ -0,0 +1,98 @@ +#include "LwipPbufFake.h" + +#include + +#include "lwip/pbuf.h" + +static unsigned pbufAllocCallCount = 0; +static struct pbuf fakePbuf; +static struct pbuf* lastAllocReturned = NULL; +static pbuf_layer lastAllocLayer = PBUF_TRANSPORT; +static u16_t lastAllocLength = 0; +static pbuf_type lastAllocType = PBUF_REF; +static bool pbufAllocFails = false; + +static unsigned pbufFreeCallCount = 0; + +static int outstandingPbufCount = 0; + +void LwipPbufFake_Reset(void) +{ + pbufAllocCallCount = 0; + lastAllocReturned = NULL; + lastAllocLayer = PBUF_TRANSPORT; + lastAllocLength = 0; + lastAllocType = PBUF_REF; + pbufAllocFails = false; + pbufFreeCallCount = 0; + outstandingPbufCount = 0; +} + +void LwipPbufFake_SetPbufAllocFails(bool fails) +{ + pbufAllocFails = fails; +} + +unsigned LwipPbufFake_PbufAllocCallCount(void) +{ + return pbufAllocCallCount; +} + +pbuf_layer LwipPbufFake_LastAllocLayer(void) +{ + return lastAllocLayer; +} + +uint16_t LwipPbufFake_LastAllocLength(void) +{ + return lastAllocLength; +} + +pbuf_type LwipPbufFake_LastAllocType(void) +{ + return lastAllocType; +} + +struct pbuf* LwipPbufFake_LastAllocReturned(void) +{ + return lastAllocReturned; +} + +unsigned LwipPbufFake_PbufFreeCallCount(void) +{ + return pbufFreeCallCount; +} + +int LwipPbufFake_OutstandingPbufCount(void) +{ + return outstandingPbufCount; +} + +struct pbuf* pbuf_alloc(pbuf_layer layer, u16_t length, pbuf_type type) +{ + ++pbufAllocCallCount; + lastAllocLayer = layer; + lastAllocLength = length; + lastAllocType = type; + if (pbufAllocFails) + { + lastAllocReturned = NULL; + } + else + { + fakePbuf.payload = NULL; + fakePbuf.len = length; + fakePbuf.tot_len = length; + lastAllocReturned = &fakePbuf; + ++outstandingPbufCount; + } + return lastAllocReturned; +} + +u8_t pbuf_free(struct pbuf* p) +{ + (void) p; + ++pbufFreeCallCount; + --outstandingPbufCount; + return 1; +} diff --git a/Tests/Support/LwipFakes/Source/LwipUdpFake.c b/Tests/Support/LwipFakes/Source/LwipUdpFake.c index e776e9a5..d9d6e131 100644 --- a/Tests/Support/LwipFakes/Source/LwipUdpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipUdpFake.c @@ -12,6 +12,13 @@ static bool udpNewFails = false; static unsigned udpRemoveCallCount = 0; static struct udp_pcb* lastUdpRemovePcb = NULL; +static unsigned udpSendtoCallCount = 0; +static struct udp_pcb* lastSendtoPcb = NULL; +static struct pbuf* lastSendtoPbuf = NULL; +static const ip_addr_t* lastSendtoIpaddr = NULL; +static u16_t lastSendtoPort = 0; +static err_t udpSendtoError = ERR_OK; + static int outstandingPcbCount = 0; void LwipUdpFake_Reset(void) @@ -21,9 +28,20 @@ void LwipUdpFake_Reset(void) udpNewFails = false; udpRemoveCallCount = 0; lastUdpRemovePcb = NULL; + udpSendtoCallCount = 0; + lastSendtoPcb = NULL; + lastSendtoPbuf = NULL; + lastSendtoIpaddr = NULL; + lastSendtoPort = 0; + udpSendtoError = ERR_OK; outstandingPcbCount = 0; } +void LwipUdpFake_SetUdpSendtoError(int8_t err) +{ + udpSendtoError = (err_t) err; +} + void LwipUdpFake_SetUdpNewFails(bool fails) { udpNewFails = fails; @@ -49,6 +67,31 @@ struct udp_pcb* LwipUdpFake_LastUdpRemovePcb(void) return lastUdpRemovePcb; } +unsigned LwipUdpFake_UdpSendtoCallCount(void) +{ + return udpSendtoCallCount; +} + +struct udp_pcb* LwipUdpFake_LastSendtoPcb(void) +{ + return lastSendtoPcb; +} + +struct pbuf* LwipUdpFake_LastSendtoPbuf(void) +{ + return lastSendtoPbuf; +} + +const ip_addr_t* LwipUdpFake_LastSendtoIpaddr(void) +{ + return lastSendtoIpaddr; +} + +uint16_t LwipUdpFake_LastSendtoPort(void) +{ + return lastSendtoPort; +} + int LwipUdpFake_OutstandingPcbCount(void) { return outstandingPcbCount; @@ -75,3 +118,13 @@ void udp_remove(struct udp_pcb* pcb) lastUdpRemovePcb = pcb; --outstandingPcbCount; } + +err_t udp_sendto(struct udp_pcb* pcb, struct pbuf* p, const ip_addr_t* dst_ip, u16_t dst_port) +{ + ++udpSendtoCallCount; + lastSendtoPcb = pcb; + lastSendtoPbuf = p; + lastSendtoIpaddr = dst_ip; + lastSendtoPort = dst_port; + return udpSendtoError; +} From b7bfc8e5b052b91f5477d5b25ee3c2ed841136c8 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 10:38:47 +0000 Subject: [PATCH 5/7] =?UTF-8?q?refactor:=20S28.04=20LwipRawDatagram=20Send?= =?UTF-8?q?To=20tests=20=E2=80=94=20shared=20buffer=20+=20sendBytes=20help?= =?UTF-8?q?er?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a 1232-byte sendBuffer member and a sendBytes(length = 1) helper on LwipRawDatagramTestBase. Most SendTo tests reduce to a single helper call; tests asserting on length pass it explicitly. Cuts ~7 lines of boilerplate per test without losing any check. SendToPbufPayloadPointsAtCallerBuffer keeps its own distinct local buffer so the pointer-identity assertion stays meaningful — the test must demonstrate the production forwarded the exact pointer the test handed in, not a fixture address that always round-trips. Co-Authored-By: Claude Opus 4.7 (1M context) --- Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 66 +++++++++---------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index 41b355d5..e59a802b 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -54,6 +54,11 @@ TEST_BASE(LwipRawDatagramTestBase) { struct SolidSyslogDatagram* datagram = nullptr; struct SolidSyslogAddress* address = nullptr; + /* Shared scratch buffer for SendTo tests. Sized to MaxPayload so the + * largest-length test (sendBytes(SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD)) + * stays in-bounds. Content is irrelevant — payload-pointer-identity + * and length are the observable surface. */ + char sendBuffer[SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD] = {}; void createFakesAndHandles() { @@ -73,6 +78,13 @@ TEST_BASE(LwipRawDatagramTestBase) LONGS_EQUAL_TEXT(0, LwipUdpFake_OutstandingPcbCount(), "leaked udp_pcb past teardown"); LONGS_EQUAL_TEXT(0, LwipPbufFake_OutstandingPbufCount(), "leaked pbuf past teardown"); } + + /* SendTo against the shared buffer + address. Default length 1 — most + * tests don't care; tests asserting length pass it explicitly. */ + enum SolidSyslogDatagramSendResult sendBytes(size_t length = 1U) + { + return SolidSyslogDatagram_SendTo(datagram, sendBuffer, length, address); + } }; TEST_GROUP_BASE(SolidSyslogLwipRawDatagram, LwipRawDatagramTestBase) @@ -159,10 +171,7 @@ TEST(SolidSyslogLwipRawDatagram, MaxPayloadReturnsIpv6SafeDefault) TEST(SolidSyslogLwipRawDatagram, SendToFailsBeforeOpen) { - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, - SolidSyslogDatagram_SendTo(datagram, "x", 1, address) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, sendBytes()); CALLED_FAKE(LwipPbufFake_PbufAlloc, NEVER); CALLED_FAKE(LwipUdpFake_UdpSendto, NEVER); } @@ -224,29 +233,26 @@ TEST(SolidSyslogLwipRawDatagramOpen, DestroyAfterCloseDoesNotRemoveAgain) TEST(SolidSyslogLwipRawDatagramOpen, SendToSucceeds) { - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, - SolidSyslogDatagram_SendTo(datagram, "x", 1, address) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, sendBytes()); } TEST(SolidSyslogLwipRawDatagramOpen, SendToCallsUdpSendto) { - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); CALLED_FAKE(LwipUdpFake_UdpSendto, ONCE); } TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesPbuf) { - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); CALLED_FAKE(LwipPbufFake_PbufAlloc, ONCE); } TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesTransportLayerPbufRef) { - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); LONGS_EQUAL(PBUF_TRANSPORT, LwipPbufFake_LastAllocLayer()); LONGS_EQUAL(PBUF_REF, LwipPbufFake_LastAllocType()); @@ -254,16 +260,19 @@ TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesTransportLayerPbufRef) TEST(SolidSyslogLwipRawDatagramOpen, SendToPbufPayloadPointsAtCallerBuffer) { - static const char buf[] = "hello"; + /* Distinct local buffer (not the fixture's sendBuffer) so the + * assertion proves the production forwarded the exact pointer the + * test handed in, not just any address that happened to round-trip. */ + static const char callerBuffer[] = "hello"; - SolidSyslogDatagram_SendTo(datagram, buf, sizeof(buf) - 1U, address); + SolidSyslogDatagram_SendTo(datagram, callerBuffer, sizeof(callerBuffer) - 1U, address); - POINTERS_EQUAL(buf, LwipPbufFake_LastAllocReturned()->payload); + POINTERS_EQUAL(callerBuffer, LwipPbufFake_LastAllocReturned()->payload); } TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsAddressIpAndPort) { - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); POINTERS_EQUAL(&SolidSyslogLwipRawAddress_AsConst(address)->Ip, LwipUdpFake_LastSendtoIpaddr()); LONGS_EQUAL(TEST_PORT, LwipUdpFake_LastSendtoPort()); @@ -271,18 +280,16 @@ TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsAddressIpAndPort) TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsLengthVerbatim) { - static const char buf[1232] = {}; - - SolidSyslogDatagram_SendTo(datagram, buf, 1, address); + sendBytes(1); LONGS_EQUAL(1, LwipPbufFake_LastAllocLength()); - SolidSyslogDatagram_SendTo(datagram, buf, 1232, address); - LONGS_EQUAL(1232, LwipPbufFake_LastAllocLength()); + sendBytes(SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD); + LONGS_EQUAL(SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD, LwipPbufFake_LastAllocLength()); } TEST(SolidSyslogLwipRawDatagramOpen, SendToFreesPbufAfterSendto) { - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); } @@ -291,20 +298,14 @@ TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsAfterClose) { SolidSyslogDatagram_Close(datagram); - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, - SolidSyslogDatagram_SendTo(datagram, "x", 1, address) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, sendBytes()); } TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsWhenPbufAllocFails) { LwipPbufFake_SetPbufAllocFails(true); - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, - SolidSyslogDatagram_SendTo(datagram, "x", 1, address) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, sendBytes()); CALLED_FAKE(LwipUdpFake_UdpSendto, NEVER); } @@ -312,17 +313,14 @@ TEST(SolidSyslogLwipRawDatagramOpen, SendToFailsWhenSendtoErrors) { LwipUdpFake_SetUdpSendtoError(ERR_MEM); - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, - SolidSyslogDatagram_SendTo(datagram, "x", 1, address) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED, sendBytes()); } TEST(SolidSyslogLwipRawDatagramOpen, SendToFreesPbufEvenWhenSendtoErrors) { LwipUdpFake_SetUdpSendtoError(ERR_MEM); - SolidSyslogDatagram_SendTo(datagram, "x", 1, address); + sendBytes(); CALLED_FAKE(LwipPbufFake_PbufFree, ONCE); } From 0a1c385d099c1b6e1f08f63b6450740465016b31 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 10:41:08 +0000 Subject: [PATCH 6/7] refactor: S28.04 SendToPbufPayloadPointsAtCallerBuffer uses fixture sendBuffer Drop the local callerBuffer in favour of writing the known content into the fixture's sendBuffer and going through the shared sendBytes helper. Same pointer-identity proof, consistent with every other SendTo test. Co-Authored-By: Claude Opus 4.7 (1M context) --- Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index e59a802b..1d863d14 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -260,14 +260,11 @@ TEST(SolidSyslogLwipRawDatagramOpen, SendToAllocatesTransportLayerPbufRef) TEST(SolidSyslogLwipRawDatagramOpen, SendToPbufPayloadPointsAtCallerBuffer) { - /* Distinct local buffer (not the fixture's sendBuffer) so the - * assertion proves the production forwarded the exact pointer the - * test handed in, not just any address that happened to round-trip. */ - static const char callerBuffer[] = "hello"; + memcpy(sendBuffer, "hello", 5); - SolidSyslogDatagram_SendTo(datagram, callerBuffer, sizeof(callerBuffer) - 1U, address); + sendBytes(5); - POINTERS_EQUAL(callerBuffer, LwipPbufFake_LastAllocReturned()->payload); + POINTERS_EQUAL(sendBuffer, LwipPbufFake_LastAllocReturned()->payload); } TEST(SolidSyslogLwipRawDatagramOpen, SendToForwardsAddressIpAndPort) From 89c2bcd2d433b9a6d41ec6382d1e813aa733ec17 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Wed, 27 May 2026 13:01:17 +0000 Subject: [PATCH 7/7] feat: S28.04 SolidSyslogLwipRawDatagram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-of-story bundle: CLAUDE.md public-headers rows for the new LwipRawDatagram + LwipRawDatagramErrors headers; CMake STATUS message updated to "Address+Resolver+Datagram only; TcpStream/BDD arrive in S28.05-S28.06"; DEVLOG entry covering the pbuf strategy / ARP delegation / leak-invariant test pattern and the next-up pointer. IWYU pass over the four touched files (production + two fakes + the test cpp) — three header adds, zero removes. clang-format reflow on the test file. cppcheck-misra surfaced six findings in Platform/LwipRaw/ that the CI analyze-cppcheck step does not yet scan. CI is extended in this PR to add Platform/LwipRaw/{Interface,Source/} to all three cppcheck invocations; suppressions land for the four S28.03-era findings (LwipRawAddressPrivate.h: 11.2 / 11.3; LwipRawAddressStatic.c: 11.3) that have been invisible since S28.03 merged, plus the two S28.04 findings (LwipRawDatagram.c:43 SelfFromBase 11.3; LwipRawDatagram.c:94 pbuf->payload const-strip 11.8). D.006 in docs/misra-deviations.md gains a sub-section for the lwIP pbuf->payload site as the second "forced by an external interface" entry alongside the Winsock select() case. S28-04-HANDOFF.md removed. Closes #463. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 6 ++ CLAUDE.md | 2 + CMakeLists.txt | 4 +- DEVLOG.md | 97 +++++++++++++++++++ .../Source/SolidSyslogLwipRawDatagram.c | 3 + Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp | 11 ++- Tests/Support/LwipFakes/Source/LwipPbufFake.c | 1 + Tests/Support/LwipFakes/Source/LwipUdpFake.c | 2 + docs/misra-deviations.md | 50 +++++++--- misra_suppressions.txt | 10 +- 10 files changed, 168 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2cd92923..a73406e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -592,6 +592,7 @@ jobs: -IPlatform/MbedTls/Interface -IPlatform/FreeRtos/Interface -IPlatform/PlusTcp/Interface + -IPlatform/LwipRaw/Interface -IPlatform/FatFs/Interface --xml --xml-version=2 Core/Source/ @@ -602,6 +603,7 @@ jobs: Platform/MbedTls/Source/ Platform/FreeRtos/Source/ Platform/PlusTcp/Source/ + Platform/LwipRaw/Source/ Platform/FatFs/Source/ 2> build/cppcheck/cppcheck-report.xml @@ -637,6 +639,7 @@ jobs: -IPlatform/MbedTls/Interface -IPlatform/FreeRtos/Interface -IPlatform/PlusTcp/Interface + -IPlatform/LwipRaw/Interface -IPlatform/FatFs/Interface Core/Source/ Platform/Atomics/Source/ @@ -646,6 +649,7 @@ jobs: Platform/MbedTls/Source/ Platform/FreeRtos/Source/ Platform/PlusTcp/Source/ + Platform/LwipRaw/Source/ Platform/FatFs/Source/ - name: Generate cppcheck-misra XML report @@ -666,6 +670,7 @@ jobs: -IPlatform/MbedTls/Interface \ -IPlatform/FreeRtos/Interface \ -IPlatform/PlusTcp/Interface \ + -IPlatform/LwipRaw/Interface \ -IPlatform/FatFs/Interface \ --xml --xml-version=2 \ Core/Source/ \ @@ -676,6 +681,7 @@ jobs: Platform/MbedTls/Source/ \ Platform/FreeRtos/Source/ \ Platform/PlusTcp/Source/ \ + Platform/LwipRaw/Source/ \ Platform/FatFs/Source/ \ 2> build/cppcheck-misra/cppcheck-misra-report.xml diff --git a/CLAUDE.md b/CLAUDE.md index 88547d1d..ef472970 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -352,6 +352,8 @@ live under `Core/Interface/`; platform-specific helpers (the `SolidSyslogPosix*` | `SolidSyslogNullDatagram.h` | Any code installing a no-op datagram slot (Open/Close are no-ops, SendTo returns `SENT` to drop on the floor so the Store does not fill with undeliverables, MaxPayload returns the IPv6-safe default) | `SolidSyslogNullDatagram_Get` | | `SolidSyslogPlusTcpDatagram.h` | System setup code on FreeRTOS targets using FreeRTOS-Plus-TCP for UDP | `SolidSyslogPlusTcpDatagram_Create(void)`, `_Destroy(base)` (wraps `FreeRTOS_socket` / `FreeRTOS_sendto` / `FreeRTOS_closesocket`). Instance struct lives in a library-internal static pool (E11). Pool-exhaustion fallback is the shared `SolidSyslogNullDatagram`. | | `SolidSyslogPlusTcpDatagramErrors.h` | Any code installing an error handler that wants to react to PlusTcpDatagram-specific events (pointer-identity match on `PlusTcpDatagramErrorSource`, switch on `enum SolidSyslogPlusTcpDatagramErrors`) | `enum SolidSyslogPlusTcpDatagramErrors` (`PLUSTCPDATAGRAM_ERROR_*` codes + `PLUSTCPDATAGRAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource PlusTcpDatagramErrorSource`. Integrators ignore if not handling errors per source. | +| `SolidSyslogLwipRawDatagram.h` | System setup code on **any target using lwIP Raw API** wiring a UDP sender | `SolidSyslogLwipRawDatagram_Create(void)`, `_Destroy(base)` (wraps `udp_new` / `udp_sendto` / `udp_remove`; PBUF_REF zero-copy send; relies on lwIP `ARP_QUEUEING` for cache-miss recovery). Instance struct lives in a library-internal static pool (E11). Pool-exhaustion fallback is the shared `SolidSyslogNullDatagram`. | +| `SolidSyslogLwipRawDatagramErrors.h` | Any code installing an error handler that wants to react to LwipRawDatagram-specific events (pointer-identity match on `LwipRawDatagramErrorSource`, switch on `enum SolidSyslogLwipRawDatagramErrors`) | `enum SolidSyslogLwipRawDatagramErrors` (`LWIPRAWDATAGRAM_ERROR_*` codes + `LWIPRAWDATAGRAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource LwipRawDatagramErrorSource`. Integrators ignore if not handling errors per source. | | `SolidSyslogWinsockDatagramErrors.h` | Any code installing an error handler that wants to react to WinsockDatagram-specific events (pointer-identity match on `WinsockDatagramErrorSource`, switch on `enum SolidSyslogWinsockDatagramErrors`) | `enum SolidSyslogWinsockDatagramErrors` (`WINSOCKDATAGRAM_ERROR_*` codes + `WINSOCKDATAGRAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource WinsockDatagramErrorSource`. Integrators ignore if not handling errors per source. | | `SolidSyslogPosixDatagramErrors.h` | Any code installing an error handler that wants to react to PosixDatagram-specific events (pointer-identity match on `PosixDatagramErrorSource`, switch on `enum SolidSyslogPosixDatagramErrors`) | `enum SolidSyslogPosixDatagramErrors` (`POSIXDATAGRAM_ERROR_*` codes + `POSIXDATAGRAM_ERROR_MAX` bookend), `extern const struct SolidSyslogErrorSource PosixDatagramErrorSource`. Integrators ignore if not handling errors per source. | | `SolidSyslogStream.h` | Sender implementors using stream transport | `SolidSyslogStream_Open`, `_Send`, `_Read`, `_Close`, `SolidSyslogSsize` | diff --git a/CMakeLists.txt b/CMakeLists.txt index 51f0ff46..0afce111 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -169,12 +169,12 @@ endif() if(SOLIDSYSLOG_FREERTOS_NET STREQUAL "LWIP") message(STATUS "SOLIDSYSLOG_FREERTOS_NET=LWIP: lwIP backend partial — " - "Address+Resolver only; Datagram/TcpStream/BDD arrive in S28.04-S28.06.") + "Address+Resolver+Datagram only; TcpStream/BDD arrive in S28.05-S28.06.") endif() if(SOLIDSYSLOG_FREERTOS_NET STREQUAL "BOTH") message(STATUS "SOLIDSYSLOG_FREERTOS_NET=BOTH: lwIP backend partial — " - "Address+Resolver only; Datagram/TcpStream/BDD arrive in S28.04-S28.06.") + "Address+Resolver+Datagram only; TcpStream/BDD arrive in S28.05-S28.06.") endif() # Build-time tunables (E21: Port-Time Configurability). Integrators override diff --git a/DEVLOG.md b/DEVLOG.md index dbd595a7..1ec59a34 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,102 @@ # Dev Log +## 2026-05-27 — S28.04 SolidSyslogLwipRawDatagram + +Fourth story in E28 (#463, parent #439). UDP datagram adapter for the +lwIP Raw API. Same three-TU pool shape as S28.03; the new ground was +the lwIP calling convention (`pbuf` allocation discipline + ARP +behaviour) and a test-shape refactor that lifted the lwIP fakes into +their own directory and pinned down a leak invariant. + +### Design decisions + +1. **`pbuf` strategy — PBUF_REF + `pbuf_alloc` per send.** SendTo + allocates a header from lwIP's `MEMP_PBUF` pool with type + `PBUF_REF`, points `payload` at the caller's buffer, calls + `udp_sendto`, and `pbuf_free`s on every exit. Mid-commit-3 David + reopened the question with a MISRA hat (Rule 21.3, no dynamic + allocation): the alternative was embedding a `struct pbuf` in + `SolidSyslogLwipRawDatagram` and skipping alloc/free entirely. + Conclusion: keep PBUF_REF. Embedding the pbuf would save *one* + `MEMP_PBUF` slot per send, but lwIP's `udp_sendto` still internally + allocates a header pbuf to prepend the UDP header. The asymmetry + doesn't justify the manual `ref`/`type` init and the latent risk + that lwIP later changes assumptions about who owns the pbuf. The + project's no-dynamic-allocation philosophy applies at *our* code + boundary; lwIP's pool is its concern. + +2. **ARP cache miss — delegate to lwIP.** No manual ARP probe in + our SendTo (the PlusTcpDatagram pattern, where the host stack does + *not* queue on cache miss). lwIP's `etharp_query` fires the ARP + request and queues the pbuf when `ARP_QUEUEING=1` (the lwIP + compiled-in default), then drops it onto the wire when the reply + lands. First datagram survives. The `ARP_QUEUEING=0` integrator + footgun lives in S28.05's integrator guide, not here. Keeps the + wrapper netif-agnostic — no `netif*` argument, no `netif_default` + dependence. + +3. **Failure mapping.** `udp_sendto` → `ERR_OK` maps to `SENT`; + everything else maps to `FAILED`. **No `OVERSIZE`** — lwIP has no + clean oversize signal, and the upstream `Service` algorithm already + honours `MaxPayload()` before reaching SendTo. + +4. **`MaxPayload()` returns 1232 unconditionally.** `SOLIDSYSLOG_UDP_IPV6_SAFE_PAYLOAD`, + same as PlusTcp. No `netif_get_mtu()` plumbing — the IPv6-safe + default covers every interface this adapter is realistically + wired against. + +### Test-shape work + +The test file grew through three refactor passes (commits 4–6 on the +branch) responding to David's feedback. Worth recording the shape it +settled into — applicable to every future pool-allocated adapter: + +- **`TEST_BASE` + two `TEST_GROUP_BASE` derivations** — + `SolidSyslogLwipRawDatagram` (Created-only) and + `SolidSyslogLwipRawDatagramOpen` (Open in setup). Most lifecycle + tests don't need to repeat `SolidSyslogDatagram_Open(datagram)` in + the body; the assertion is what matters. The two-group split keeps + setup focused on the precondition each test class shares. + +- **Leak invariants in the shared teardown.** `LwipUdpFake_OutstandingPcbCount()` + and `LwipPbufFake_OutstandingPbufCount()` track successful + alloc-minus-free per resource, asserted to be zero in every + teardown. Earned its keep at commit 6 — the invariant caught the + missing `pbuf_free` before the explicit `SendToFreesPbufAfterSendto` + test landed. **Pattern is general** — extend to other adapter + fakes whenever the production owns alloc/free of a host resource. + +- **`sendBytes(size_t length = 1U)` helper on the base** — most + SendTo tests just need *a* send, default-1 byte. Tests that observe + length pass it explicitly. Cut ~7 lines per test of `SolidSyslogDatagram_SendTo(datagram, "x", 1, address)` + boilerplate. + +### New tunable, new error source + +- `SOLIDSYSLOG_LWIP_RAW_DATAGRAM_POOL_SIZE` default `1U` — pool of + one is the dominant integrator shape (single UDP sender). +- `LwipRawDatagramErrorSource` with `POOL_EXHAUSTED` / + `UNKNOWN_DESTROY`, matching every other pool class. + +### LwipFakes evolution + +Two new fake files: `LwipUdpFake.{c,h}` (`udp_new`, `udp_remove`, +`udp_sendto` + spies + failure injection) and `LwipPbufFake.{c,h}` +(`pbuf_alloc`, `pbuf_free` + spies + alloc-fail injection). +`LwipUpstreamParser` is not pulled in — none of these calls need +real lwIP code. Sources inlined per-test-executable via +`LWIP_FAKE_SOURCES` variable in `Tests/Lwip/CMakeLists.txt`, +matching the FreeRtosFakes / FatFsFakes precedent. + +### Next up + +S28.05 — `SolidSyslogLwipRawTcpStream`. Stream side of the same +adapter pair: `tcp_new` / `tcp_connect` / `tcp_write` / +`tcp_recv` / `tcp_close`, with the receive callback bridging into +the synchronous `Stream_Read` contract (the part of lwIP Raw that +genuinely differs from sockets — a tiny pending-data ring the +adapter owns). + ## 2026-05-26 — S24.12 inline cppcheck-suppress and NOLINT audit E24 code-hygiene story #461. The session-end of S24.11 left 213 inline diff --git a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c index 44468335..d007a3f4 100644 --- a/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c +++ b/Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c @@ -3,8 +3,11 @@ #include #include +#include "lwip/arch.h" +#include "lwip/err.h" #include "lwip/pbuf.h" #include "lwip/udp.h" +#include "SolidSyslogDatagram.h" #include "SolidSyslogDatagramDefinition.h" #include "SolidSyslogLwipRawAddressPrivate.h" #include "SolidSyslogNullDatagram.h" diff --git a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp index 1d863d14..6c5ec5aa 100644 --- a/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp +++ b/Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp @@ -1,3 +1,6 @@ +#include +#include + #include "TestUtils.h" #include "CppUTest/TestHarness.h" @@ -17,7 +20,9 @@ using namespace CososoTesting; #include "SolidSyslogPrival.h" #include "SolidSyslogTunables.h" #include "SolidSyslogUdpPayload.h" +#include "lwip/err.h" #include "lwip/ip4_addr.h" +#include "lwip/pbuf.h" static const uint16_t TEST_PORT = 514; @@ -357,6 +362,7 @@ TEST_GROUP(SolidSyslogLwipRawDatagramPool) } } }; + // clang-format on TEST(SolidSyslogLwipRawDatagramPool, FillingPoolThenOverflowReturnsDistinctFallback) @@ -389,10 +395,7 @@ TEST(SolidSyslogLwipRawDatagramPool, FallbackVtableMethodsAreNoOps) * because the production-side vtable is never wired on the fallback * handle. */ CHECK_TRUE(SolidSyslogDatagram_Open(overflow)); - LONGS_EQUAL( - SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, - SolidSyslogDatagram_SendTo(overflow, "x", 1, localAddr) - ); + LONGS_EQUAL(SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT, SolidSyslogDatagram_SendTo(overflow, "x", 1, localAddr)); SolidSyslogDatagram_Close(overflow); SolidSyslogLwipRawAddress_Destroy(localAddr); diff --git a/Tests/Support/LwipFakes/Source/LwipPbufFake.c b/Tests/Support/LwipFakes/Source/LwipPbufFake.c index bc9c995b..bfdb54d7 100644 --- a/Tests/Support/LwipFakes/Source/LwipPbufFake.c +++ b/Tests/Support/LwipFakes/Source/LwipPbufFake.c @@ -2,6 +2,7 @@ #include +#include "lwip/arch.h" #include "lwip/pbuf.h" static unsigned pbufAllocCallCount = 0; diff --git a/Tests/Support/LwipFakes/Source/LwipUdpFake.c b/Tests/Support/LwipFakes/Source/LwipUdpFake.c index d9d6e131..fac23e49 100644 --- a/Tests/Support/LwipFakes/Source/LwipUdpFake.c +++ b/Tests/Support/LwipFakes/Source/LwipUdpFake.c @@ -2,6 +2,8 @@ #include +#include "lwip/arch.h" +#include "lwip/err.h" #include "lwip/udp.h" static unsigned udpNewCallCount = 0; diff --git a/docs/misra-deviations.md b/docs/misra-deviations.md index c9b40d73..b6492a2b 100644 --- a/docs/misra-deviations.md +++ b/docs/misra-deviations.md @@ -382,8 +382,9 @@ Two distinct site categories trigger this rule: that 11.8 applies to *pointer casts*, not to member-access yielding a non-`const`-qualified pointer rvalue. -2. **Platform-API const-strip (1 site)** — - `Platform/Windows/Source/SolidSyslogWinsockTcpStream.c`: +2. **Platform-API const-strip (2 sites)** — + + **(a)** `Platform/Windows/Source/SolidSyslogWinsockTcpStream.c`: ```c return select(nfds, readfds, writefds, exceptfds, (struct timeval*) timeout); @@ -395,6 +396,28 @@ Two distinct site categories trigger this rule: const-correct and forces the const-strip down to the platform-API boundary. The code carries a comment explaining the cast. + **(b)** `Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c`: + + ```c + /* SolidSyslogDatagram_SendTo's buffer is const void*; lwIP's + * pbuf->payload is plain void*. Assignment forces the const-strip. */ + p->payload = (void*) buffer; + ``` + + `SolidSyslogDatagram_SendTo` takes the caller's buffer as + `const void*` — the contract is read-only inside the library. + lwIP's `struct pbuf::payload` is declared `void*` (no `const` + variant in the lwIP headers); `udp_sendto` only reads the + payload (the PBUF_REF zero-copy contract — see handoff design + decision #1) but the field type does not encode that. Assigning + our `const void*` parameter to lwIP's `void*` field strips the + qualifier at the platform-API boundary, same shape as the + Winsock `select()` site above. Alternatives considered and + rejected: changing the public `SendTo` signature to `void*` + (breaks const-correctness for every other backend); copying + the buffer to PBUF_RAM (defeats the zero-copy point of PBUF_REF + and doubles per-send pool pressure). + ### Scope - **Strict tier** — 10 field-access sites: 8 in `Core/Source/SolidSyslog.c` @@ -405,9 +428,11 @@ Two distinct site categories trigger this rule: 1 in `Core/Source/SolidSyslogBlockStoreStatic.c` (`BlockStore_ResolveSecurityPolicy` accepting `config->SecurityPolicy`). -- **Pragmatic tier** — 1 site in +- **Pragmatic tier** — 2 sites: 1 in `Platform/Windows/Source/SolidSyslogWinsockTcpStream.c` (the - `select()` timeout cast). + `select()` timeout cast); 1 in + `Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c` (the + lwIP `pbuf->payload` field cast). ### Rationale @@ -418,11 +443,12 @@ false-positive would either drop the outer `const` qualifier on introduce a no-op `const_cast`-style explicit cast that the tool would still flag. A site-local deviation is the honest record. -The Winsock site is the canonical example MISRA itself calls out -("forced by an external interface") in the deviation guidance for -rule 11.8. The Windows-side declaration is fixed by Microsoft; the -SolidSyslog seam keeps the const-correctness contract on the caller's -side of the boundary. +The two platform-API sites are the canonical "forced by an external +interface" pattern MISRA itself calls out in the deviation guidance for +rule 11.8. Both upstream declarations (Microsoft's `select()` timeout, +lwIP's `pbuf::payload`) are fixed by their vendors; the SolidSyslog seam +keeps the const-correctness contract on the caller's side of the +boundary. ### Risk and mitigation @@ -430,8 +456,10 @@ side of the boundary. codebase would surface as a fresh 11.8 finding, not be silently absorbed by the existing suppressions — the suppressions are line-specific. -- **Platform-API site.** The Winsock cast is the only one of its - kind in the codebase and is documented at the call site. +- **Platform-API sites.** Both the Winsock and lwIP casts are + documented at the call site and listed individually here; any new + const-strip at a platform boundary surfaces as a fresh 11.8 finding + rather than being absorbed by glob. ### Approval diff --git a/misra_suppressions.txt b/misra_suppressions.txt index fa95dbeb..debf789b 100644 --- a/misra_suppressions.txt +++ b/misra_suppressions.txt @@ -12,6 +12,8 @@ # D.002 — Rules 11.2 / 11.3 / 11.5: vtable downcasts + Formatter # See docs/misra-deviations.md#d002 misra-c2012-11.2:Core/Interface/SolidSyslogFormatter.h:28 +misra-c2012-11.2:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:24 +misra-c2012-11.2:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:31 misra-c2012-11.2:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddress.c:10 misra-c2012-11.2:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:19 misra-c2012-11.2:Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressPrivate.h:26 @@ -35,6 +37,11 @@ misra-c2012-11.3:Core/Source/SolidSyslogTimeQualitySd.c:59 misra-c2012-11.3:Core/Source/SolidSyslogUdpSender.c:101 misra-c2012-11.3:Platform/Atomics/Source/SolidSyslogStdAtomicCounter.c:25 misra-c2012-11.3:Platform/FatFs/Source/SolidSyslogFatFsFile.c:45 +misra-c2012-11.3:Platform/LwipRaw/Source/SolidSyslogLwipRawAddressPrivate.h:24 +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/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 @@ -81,7 +88,7 @@ misra-c2012-5.7:Core/Interface/SolidSyslogAddress.h:10 # See docs/misra-deviations.md#d005 misra-c2012-18.7:Core/Source/SolidSyslogFormatter.c:12 -# D.006 — Rule 11.8: const-strip false-positives + Winsock platform-API +# D.006 — Rule 11.8: const-strip false-positives + platform-API (Winsock, lwIP) # See docs/misra-deviations.md#d006 misra-c2012-11.8:Core/Source/BlockSequence.c:465 misra-c2012-11.8:Core/Source/SolidSyslog.c:94 @@ -93,6 +100,7 @@ misra-c2012-11.8:Core/Source/SolidSyslog.c:99 misra-c2012-11.8:Core/Source/SolidSyslog.c:100 misra-c2012-11.8:Core/Source/SolidSyslog.c:101 misra-c2012-11.8:Core/Source/SolidSyslogBlockStoreStatic.c:40 +misra-c2012-11.8:Platform/LwipRaw/Source/SolidSyslogLwipRawDatagram.c:94 misra-c2012-11.8:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:93 # D.007 — Rule 21.10: transitive via