From e6d76b7afefaba9d03811b6e051f80bb1365eb42 Mon Sep 17 00:00:00 2001 From: David Cozens Date: Sat, 9 May 2026 15:36:38 +0000 Subject: [PATCH 1/2] feat: S08.03 slice 4 `set` command for in-RAM configuration injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 3b.2 hardcoded the FreeRTOS example's identity, message body, and endpoint into Example/FreeRtos/SingleTask/main.c because the FreeRTOS port deliberately skips Example/Common/ExampleCommandLine.c (no getopt). Slice 4 makes those configurable in-RAM via a `set NAME VALUE` command parallel to `send` and `quit` on the existing UART grammar. Library-side dispatcher (host-TDD'd, 9 ZOMBIES cycles in Tests/Example/ExampleInteractiveTest.cpp): ExampleInteractive_Run gains an ExampleInteractiveSetHandler arg with shape `bool (*)(const char* name, const char* value)`. The runner splits args at first whitespace, calls the handler, echoes `set name=value` on true or prints `set: invalid` on false. NULL handler silently ignores `set`, mirroring `switch`. POSIX/Threaded/Windows callers pass NULL. FreeRTOS-side OnSet (integrator glue): mutable static arrays sized to RFC 5424 maxima, a file-static g_message holding facility/severity in place plus messageId/msg pointers into the storage, and g_port + g_endpointVersion behind GetEndpoint/GetEndpointVersion. Validation: identity / msgid / msg / host non-empty within storage cap; facility 0-23; severity 0-7; port 1-65535. Helpers TryUpdateString and TryParseUInt do the bounds-check + memcpy + null-terminate and the full-string strtol parse respectively. `host` is plumbed through GetEndpoint and accepted into g_host but SolidSyslogFreeRtosStaticResolver still routes via the hardcoded IPv4 octets — `set host 1.2.3.4` echoes confirmation but doesn't change the destination IP yet. Follow-up slice teaches the resolver to parse dotted-quads. QEMU smoke verifies: default hostname on port 5514, `set hostname Foo` mutates frames on next send, `set port 5515` routes to the new port (endpoint-version bump triggers UdpSender re-pull), and four `set: invalid` rejections (out-of-range, empty, unknown field). Closes #302 Co-Authored-By: Claude Opus 4.7 (1M context) --- DEVLOG.md | 140 ++++++++++++++++ Example/Common/ExampleInteractive.c | 26 ++- Example/Common/ExampleInteractive.h | 5 +- Example/FreeRtos/SingleTask/main.c | 175 ++++++++++++++++---- Example/SingleTask/SolidSyslogExample.c | 2 +- Example/Threaded/main.c | 2 +- Example/Windows/SolidSyslogWindowsExample.c | 2 +- Tests/Example/CMakeLists.txt | 1 + Tests/Example/ExampleInteractiveTest.cpp | 169 +++++++++++++++++++ 9 files changed, 484 insertions(+), 38 deletions(-) create mode 100644 Tests/Example/ExampleInteractiveTest.cpp diff --git a/DEVLOG.md b/DEVLOG.md index e211f115..4360fb73 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,145 @@ # Dev Log +## 2026-05-09 — S08.03 slice 4 — `set` command for in-RAM configuration injection (#302) + +Slice 3b.2 hardcoded the FreeRTOS example's identity, message body, and +endpoint into `Example/FreeRtos/SingleTask/main.c` because the FreeRTOS +port deliberately skips `Example/Common/ExampleCommandLine.c` (no +`getopt`). Slice 4 makes those configurable in-RAM via a `set NAME VALUE` +command on the existing interactive UART grammar — same channel as +`send` and `quit`. Field set: `hostname`, `appname`, `procid`, `msgid`, +`msg`, `host`, `port`, `facility`, `severity`. Successful mutations echo +`set name=value`; rejections print `set: invalid` (catchall) and leave +the prior value untouched. `quit` still cleanly tears down the example. + +### Library-side dispatcher — host-TDD'd in 9 cycles + +`Example/Common/ExampleInteractive` grew an `onSet` callback parallel to +the existing `onSwitch`. The runner splits args at first whitespace and +calls `bool onSet(name, value)`; the library doesn't know what fields +are settable. Echo / catchall live in the runner, driven by the +handler's bool return. + +Strict ZOMBIES walk in `Tests/Example/ExampleInteractiveTest.cpp`, +`fmemopen`-backed `FILE*` for input, `dup`/`freopen`/`dup2` dance for +stdout capture: + +1. `SetHandlerNotCalledWithQuitOnly` — drove the new parameter onto the + signature; all four `ExampleInteractive_Run` call sites pass `NULL` + for the new arg (POSIX, Threaded, Windows, FreeRTOS). +2. `SetCommandCallsHandlerOnce` — drove the dispatch branch. +3. `SetCommandPassesNameToHandler` — drove first-token extraction + (`memcpy` into a local name buffer + null-terminate). +4. `SetCommandPassesValueToHandler` — drove the rest-of-line as value. +5. `SetCommandWithoutValueGivesEmptyValue` — boundary, locked in by + passing without production change (the `space != NULL` ternary + already handled it). +6. `SetCommandWithEmbeddedSpacesPreservesValueAfterFirst` — boundary, + locked in: `strchr(args, ' ')` is by definition first-occurrence. +7. `NullSetHandlerSilentlyIgnoresSetLine` — exception, locked in by + the `onSet != NULL` guard already in the dispatcher. +8. `SetCommandPrintsEchoOnHandlerSuccess` — drove the + `if (onSet(...)) printf("set %s=%s\n", name, value);` echo. +9. `SetCommandPrintsInvalidOnHandlerFailure` — drove the + `else printf("set: invalid\n");` catchall. + +Refactor pass extracted `RunWithInput` and `RunCapturingStdout` test +helpers so each test body reads as one or two lines. The +`SOLIDSYSLOG_POSIX`-gated `Tests/Example/CMakeLists.txt` block picks +up the new file alongside the existing `ExampleInteractive.c` +production source — same compile target, no separate test binary. + +### FreeRTOS-side `OnSet` handler — integrator glue + +`Example/FreeRtos/SingleTask/main.c` carries the field-name dispatch. +The walking-skeleton `TEST_*` strings became mutable static arrays +(`g_hostname[256]`, `g_appName[49]`, `g_processId[129]`, +`g_messageId[33]`, `g_msg[256]`, `g_host[16]`) sized to RFC 5424 +maxima where applicable. A file-static `g_message` holds +`facility`/`severity` (mutated in place) plus `messageId`/`msg` +pointers targeting the mutable storage so each `SolidSyslog_Log` +sees current contents. `g_port` and `g_endpointVersion` back the +endpoint callbacks. + +Validation per field: +- Identity / `msgid` / `msg` / `host`: non-empty, length within storage cap. +- `facility`: integer 0–23 (RFC 5424 facility range). +- `severity`: integer 0–7 (RFC 5424 severity range). +- `port`: integer 1–65535. + +Two small file-local helpers do the heavy lifting: `TryUpdateString` +(bounds-check + `memcpy` + null-terminate) and `TryParseUInt` +(full-string `strtol` with end-pointer + non-negative check). The +`OnSet` handler is a chain of `strcmp` lookups; unknown field names +fall through to `return false` and the runner prints `set: invalid`. + +### `host` is plumbed but currently a no-op on the wire + +`SolidSyslogFreeRtosStaticResolver` ignores the host string and routes +via the `Create`-time IPv4 octets. Slice 4 still wires `g_host` through +`GetEndpoint` (and accepts `set host …` into the storage) so the +follow-up slice that teaches the resolver to parse dotted-quads is a +contained, mechanical change in one file. For now `set host 1.2.3.4` +echoes confirmation but doesn't change the destination IP — the issue +body documents this caveat. + +### Endpoint version bump — integrator-owned + +`GetEndpointVersion` reads from `g_endpointVersion`; `OnSet` increments +it when `port` mutates successfully. The library has zero coupling to +endpoint-mutation semantics — other integrators can choose not to bump +at all if their resolver doesn't care. `SolidSyslogUdpSender` re-pulls +`endpoint()` on the next Send when the version differs from its cached +value, which is what makes `set port 5515; send 1` actually hit 5515 +instead of the cached resolver result. + +### QEMU smoke + +``` +PORT=5514 BYTES=<134>… FreeRtosExample … (default) +set hostname QemuFoo → set hostname=QemuFoo +PORT=5514 BYTES=<134>… QemuFoo … (mutated) +set port 5515 → set port=5515 +PORT=5515 BYTES=<134>… QemuFoo … (new port) +set facility 99 → set: invalid +set port 99999 → set: invalid +set hostname → set: invalid (empty value) +set bogus value → set: invalid (unknown field) +quit (clean exit) +``` + +Both ports listened simultaneously via a Python `select()` listener. +Slice 3b.1.5's transparent ARP priming means the very first `send 1` +after cold start lands without a warm-up message; the port mutation +takes effect on the first send after the version bump. + +### Pre-PR checks + +- `clang-format --dry-run --Werror` on every changed file — clean. +- `ctest --preset debug` — 5/5 (`SolidSyslogTests`, `ExampleTests` 63/63 + with 9 new `ExampleInteractive` tests, `SolidSyslogFreeRtosDatagramTest`, + `SolidSyslogFreeRtosStaticResolverTest`, `CmsdkUartTest`, + `OpenSslIntegrationTests`). +- `cmake --build --preset freertos-cross` — both ELFs link clean. +- QEMU smoke — see scenario above. +- Not run locally (CI's responsibility): `analyze-tidy`, + `analyze-cppcheck`, `analyze-iwyu`, `analyze-format`, + `build-windows-msvc`, BDD, OpenSSL integration. + +### What this leaves for later slices + +- `SolidSyslogFreeRtosStaticResolver` dotted-quad parsing so `set host` + takes effect on the wire. +- BDD harness pointed at the FreeRTOS target — slice 4 produces + deterministic confirmation lines (`set name=value`, `set: invalid`) + that a future Behave step can grep for. +- Service-thread + non-NullBuffer FreeRTOS-side wiring (CircularBuffer + + FreeRTOS mutex). +- Real RTC-backed clock callback (still hardcoded `TEST_TIMESTAMP`). +- Flash-persistent config so settings survive reboot. +- CMake-driven stack-budget scaling so the `*32` magic number can come + back down to a measured value. + ## 2026-05-09 — S08.03 slice 3b.2 — SingleTask example with SolidSyslogUdpSender + interactive command channel (#296) Slice 3b.1's "send a hardcoded ping on link-up" smoke is replaced with a diff --git a/Example/Common/ExampleInteractive.c b/Example/Common/ExampleInteractive.c index 1ba26f30..8121c93f 100644 --- a/Example/Common/ExampleInteractive.c +++ b/Example/Common/ExampleInteractive.c @@ -67,7 +67,27 @@ static void HandleSend(const char* args, const struct SolidSyslogMessage* messag printf("Sent %d message%s\n", count, (count == 1) ? "" : "s"); } -void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch) +static void HandleSet(const char* args, ExampleInteractiveSetHandler onSet) +{ + char name[MAX_LINE_LENGTH]; + const char* space = strchr(args, ' '); + size_t nameLen = (space != NULL) ? (size_t) (space - args) : strlen(args); + const char* value = (space != NULL) ? (space + 1) : ""; + + memcpy(name, args, nameLen); + name[nameLen] = '\0'; + + if (onSet(name, value)) + { + printf("set %s=%s\n", name, value); + } + else + { + printf("set: invalid\n"); + } +} + +void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch, ExampleInteractiveSetHandler onSet) { char line[MAX_LINE_LENGTH]; @@ -95,6 +115,10 @@ void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* inpu { onSwitch(args); } + else if (onSet != NULL && MatchCommand(line, "set", &args)) + { + HandleSet(args, onSet); + } PrintPrompt(); } diff --git a/Example/Common/ExampleInteractive.h b/Example/Common/ExampleInteractive.h index e8c6a947..7c7fa741 100644 --- a/Example/Common/ExampleInteractive.h +++ b/Example/Common/ExampleInteractive.h @@ -1,6 +1,7 @@ #ifndef EXAMPLEINTERACTIVE_H #define EXAMPLEINTERACTIVE_H +#include #include #include "ExternC.h" @@ -10,8 +11,10 @@ struct SolidSyslogMessage; EXTERN_C_BEGIN typedef void (*ExampleInteractiveSwitchHandler)(const char* name); + typedef bool (*ExampleInteractiveSetHandler)(const char* name, const char* value); - void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch); + void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch, + ExampleInteractiveSetHandler onSet); EXTERN_C_END diff --git a/Example/FreeRtos/SingleTask/main.c b/Example/FreeRtos/SingleTask/main.c index 273d8bfa..75af5870 100644 --- a/Example/FreeRtos/SingleTask/main.c +++ b/Example/FreeRtos/SingleTask/main.c @@ -1,15 +1,18 @@ /* FreeRTOS-Plus-TCP single-task SolidSyslog example for QEMU mps2-an385. * - * Slice 3b.2 of S08.03 — replaces 3b.1's hardcoded "ping" smoke task with - * a real SolidSyslog wiring. Static IPv4 (10.0.2.15) on the QEMU slirp - * network with the host reachable at the slirp gateway 10.0.2.2; a single - * FreeRTOS task runs Example/Common/ExampleInteractive over qemu -serial - * stdio (CmsdkUart RX wired into newlib's _read in Example/FreeRtos/ - * Common/Syscalls.c). On link-up the IP-task event hook spawns the - * interactive task once; SolidSyslog is configured with a NullBuffer + - * UdpSender driving the slice-1 SolidSyslogFreeRtosDatagram via the - * slice-3a static resolver, so each `send N` line over the UART emits N - * RFC 5424 datagrams to {10.0.2.2, 5514}. */ + * Slice 3b.2 of S08.03 wired SolidSyslog over a hardcoded TEST_* + * configuration; slice 4 makes the configurable fields mutable via the + * interactive `set ` command — hostname, appname, procid, + * msgid, msg, host (stored only — see g_host below), port, facility, + * severity. Static IPv4 (10.0.2.15) on the QEMU slirp network with the + * host reachable at the slirp gateway 10.0.2.2; a single FreeRTOS task + * runs Example/Common/ExampleInteractive over qemu -serial stdio + * (CmsdkUart RX wired into newlib's _read in Example/FreeRtos/Common/ + * Syscalls.c). On link-up the IP-task event hook spawns the interactive + * task once; SolidSyslog is configured with a NullBuffer + UdpSender + * driving the slice-1 SolidSyslogFreeRtosDatagram via the slice-3a + * static resolver, so each `send N` line over the UART emits N RFC 5424 + * datagrams to {10.0.2.2, port=g_port}. */ #include "CmsdkUart.h" #include "ExampleInteractive.h" @@ -31,8 +34,11 @@ #include #include +#include #include #include +#include +#include #define CMSDK_UART0_BASE_ADDRESS UINT32_C(0x40004000) @@ -79,13 +85,29 @@ static const uint8_t TEST_DNS[ipIP_ADDRESS_LENGTH_BYTES] = {10U, 0U static const uint8_t TEST_MAC[ipMAC_ADDRESS_LENGTH_BYTES] = {0x02U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U}; static const uint8_t TEST_DESTINATION_IPV4[ipIP_ADDRESS_LENGTH_BYTES] = {10U, 0U, 2U, 2U}; -/* Walking-skeleton TEST_* values — slice-4 will replace these with config - * injected over the interactive command grammar. */ -static const char TEST_HOSTNAME[] = "FreeRtosExample"; -static const char TEST_APP_NAME[] = "SolidSyslogExample"; -static const char TEST_PROCESS_ID[] = "1"; -static const char TEST_MESSAGE_ID[] = "example"; -static const char TEST_MESSAGE[] = "Hello from FreeRTOS"; +/* Mutable walking-skeleton state. Defaults populated at boot; the + * interactive `set ` command rewrites these in-place via + * OnSet below. Storage sizes match RFC 5424 maxima where applicable + * (HOSTNAME 255, APP-NAME 48, PROCID 128, MSGID 32) plus null + * terminator; MSG is bounded by storage rather than the protocol; + * g_host fits an IPv4 dotted-quad. g_message holds facility/severity + * (mutated in place) and the messageId/msg pointers (which target the + * mutable storage so contents are seen on each Log). */ +static char g_hostname[256] = "FreeRtosExample"; +static char g_appName[49] = "SolidSyslogExample"; +static char g_processId[129] = "1"; +static char g_messageId[33] = "example"; +static char g_msg[256] = "Hello from FreeRTOS"; +static char g_host[16] = "10.0.2.2"; +static uint16_t g_port = (uint16_t) EXAMPLE_UDP_PORT; +static uint32_t g_endpointVersion = 0U; + +static struct SolidSyslogMessage g_message = { + .facility = SOLIDSYSLOG_FACILITY_LOCAL0, + .severity = SOLIDSYSLOG_SEVERITY_INFO, + .messageId = g_messageId, + .msg = g_msg, +}; /* RFC 5424 publication date — placeholder until S08.03 slice 4+ injects a * real RTC-backed clock callback. */ @@ -114,6 +136,9 @@ static BaseType_t interactiveTaskCreated = pdFALSE; extern NetworkInterface_t* pxMPS2_FillInterfaceDescriptor(BaseType_t xEMACIndex, NetworkInterface_t* pxInterface); +static bool TryUpdateString(char* storage, size_t storageSize, const char* value); +static bool TryParseUInt(const char* value, unsigned long* out); + static uint32_t MmioRead32(uintptr_t address) { // NOLINTNEXTLINE(performance-no-int-to-ptr) -- mapping the CMSDK UART MMIO address into a 32-bit volatile pointer. @@ -151,17 +176,17 @@ static void SetEthernetIrqPriority(void) static void GetHostname(struct SolidSyslogFormatter* formatter) { - SolidSyslogFormatter_BoundedString(formatter, TEST_HOSTNAME, sizeof(TEST_HOSTNAME) - 1U); + SolidSyslogFormatter_BoundedString(formatter, g_hostname, strlen(g_hostname)); } static void GetAppName(struct SolidSyslogFormatter* formatter) { - SolidSyslogFormatter_BoundedString(formatter, TEST_APP_NAME, sizeof(TEST_APP_NAME) - 1U); + SolidSyslogFormatter_BoundedString(formatter, g_appName, strlen(g_appName)); } static void GetProcessId(struct SolidSyslogFormatter* formatter) { - SolidSyslogFormatter_BoundedString(formatter, TEST_PROCESS_ID, sizeof(TEST_PROCESS_ID) - 1U); + SolidSyslogFormatter_BoundedString(formatter, g_processId, strlen(g_processId)); } static void GetTimestamp(struct SolidSyslogTimestamp* timestamp) @@ -171,15 +196,106 @@ static void GetTimestamp(struct SolidSyslogTimestamp* timestamp) static void GetEndpoint(struct SolidSyslogEndpoint* endpoint) { - /* SolidSyslogFreeRtosStaticResolver ignores the host string, so we - * leave it empty; the port still needs to be populated for sendto. */ - SolidSyslogFormatter_BoundedString(endpoint->host, "", 0U); - endpoint->port = (uint16_t) EXAMPLE_UDP_PORT; + /* SolidSyslogFreeRtosStaticResolver currently ignores the host + * string and routes via TEST_DESTINATION_IPV4, so g_host is plumbed + * here for forward-compatibility with the follow-up slice that will + * teach the resolver to parse dotted-quads. The port reaches the + * wire via sendto unchanged. */ + SolidSyslogFormatter_BoundedString(endpoint->host, g_host, strlen(g_host)); + endpoint->port = g_port; } static uint32_t GetEndpointVersion(void) { - return 0U; + return g_endpointVersion; +} + +static bool OnSet(const char* name, const char* value) +{ + if (strcmp(name, "hostname") == 0) + { + return TryUpdateString(g_hostname, sizeof(g_hostname), value); + } + if (strcmp(name, "appname") == 0) + { + return TryUpdateString(g_appName, sizeof(g_appName), value); + } + if (strcmp(name, "procid") == 0) + { + return TryUpdateString(g_processId, sizeof(g_processId), value); + } + if (strcmp(name, "msgid") == 0) + { + return TryUpdateString(g_messageId, sizeof(g_messageId), value); + } + if (strcmp(name, "msg") == 0) + { + return TryUpdateString(g_msg, sizeof(g_msg), value); + } + if (strcmp(name, "host") == 0) + { + return TryUpdateString(g_host, sizeof(g_host), value); + } + if (strcmp(name, "port") == 0) + { + unsigned long parsed = 0U; + if (!TryParseUInt(value, &parsed) || parsed == 0U || parsed > UINT16_MAX) + { + return false; + } + g_port = (uint16_t) parsed; + g_endpointVersion++; + return true; + } + if (strcmp(name, "facility") == 0) + { + unsigned long parsed = 0U; + if (!TryParseUInt(value, &parsed) || parsed > 23U) + { + return false; + } + g_message.facility = (enum SolidSyslog_Facility) parsed; + return true; + } + if (strcmp(name, "severity") == 0) + { + unsigned long parsed = 0U; + if (!TryParseUInt(value, &parsed) || parsed > 7U) + { + return false; + } + g_message.severity = (enum SolidSyslog_Severity) parsed; + return true; + } + return false; +} + +static bool TryUpdateString(char* storage, size_t storageSize, const char* value) +{ + size_t length = strlen(value); + if ((length == 0U) || (length >= storageSize)) + { + return false; + } + memcpy(storage, value, length); + storage[length] = '\0'; + return true; +} + +static bool TryParseUInt(const char* value, unsigned long* out) +{ + if (*value == '\0') + { + return false; + } + char* end = NULL; + long parsed = strtol(value, &end, 10); + if ((*end != '\0') || (parsed < 0)) + { + return false; + } + *out = (unsigned long) parsed; + return true; } static void InteractiveTask(void* argument) @@ -212,14 +328,7 @@ static void InteractiveTask(void* argument) }; SolidSyslog_Create(&config); - struct SolidSyslogMessage message = { - .facility = SOLIDSYSLOG_FACILITY_LOCAL0, - .severity = SOLIDSYSLOG_SEVERITY_INFO, - .messageId = TEST_MESSAGE_ID, - .msg = TEST_MESSAGE, - }; - - ExampleInteractive_Run(&message, stdin, NULL); + ExampleInteractive_Run(&g_message, stdin, NULL, OnSet); SolidSyslog_Destroy(); SolidSyslogNullStore_Destroy(); diff --git a/Example/SingleTask/SolidSyslogExample.c b/Example/SingleTask/SolidSyslogExample.c index ea0b8d8a..cc2d5eaf 100644 --- a/Example/SingleTask/SolidSyslogExample.c +++ b/Example/SingleTask/SolidSyslogExample.c @@ -121,7 +121,7 @@ int SolidSyslogExample_Run(int argc, char* argv[]) .msg = options.msg, }; - ExampleInteractive_Run(&message, stdin, NULL); + ExampleInteractive_Run(&message, stdin, NULL, NULL); SolidSyslog_Destroy(); SolidSyslogOriginSd_Destroy(); diff --git a/Example/Threaded/main.c b/Example/Threaded/main.c index f027ab01..8369a733 100644 --- a/Example/Threaded/main.c +++ b/Example/Threaded/main.c @@ -281,7 +281,7 @@ int main(int argc, char* argv[]) .msg = options.msg, }; - ExampleInteractive_Run(&message, stdin, ExampleSwitchConfig_SetByName); + ExampleInteractive_Run(&message, stdin, ExampleSwitchConfig_SetByName, NULL); shutdown_flag = true; pthread_join(serviceThread, NULL); diff --git a/Example/Windows/SolidSyslogWindowsExample.c b/Example/Windows/SolidSyslogWindowsExample.c index b5d1d491..20ddb9a1 100644 --- a/Example/Windows/SolidSyslogWindowsExample.c +++ b/Example/Windows/SolidSyslogWindowsExample.c @@ -352,7 +352,7 @@ int SolidSyslogWindowsExample_Run(int argc, char* argv[]) .msg = options.msg, }; - ExampleInteractive_Run(&message, stdin, ExampleSwitchConfig_SetByName); + ExampleInteractive_Run(&message, stdin, ExampleSwitchConfig_SetByName, NULL); shutdownFlag = true; WaitForSingleObject(serviceThread, INFINITE); diff --git a/Tests/Example/CMakeLists.txt b/Tests/Example/CMakeLists.txt index d1823fb0..353df679 100644 --- a/Tests/Example/CMakeLists.txt +++ b/Tests/Example/CMakeLists.txt @@ -19,6 +19,7 @@ set(EXAMPLE_TEST_INCLUDE_DIRS if(SOLIDSYSLOG_POSIX) list(APPEND EXAMPLE_TEST_SOURCES ExampleCommandLineTest.cpp + ExampleInteractiveTest.cpp SolidSyslogExampleTest.cpp ExampleServiceThreadTest.cpp ExampleSwitchConfigTest.cpp diff --git a/Tests/Example/ExampleInteractiveTest.cpp b/Tests/Example/ExampleInteractiveTest.cpp new file mode 100644 index 00000000..7604fab6 --- /dev/null +++ b/Tests/Example/ExampleInteractiveTest.cpp @@ -0,0 +1,169 @@ +#include "ExampleInteractive.h" +#include "SolidSyslog.h" +#include "CppUTest/TestHarness.h" + +#include +#include +#include +#include + +namespace +{ +struct SetHandlerSpy +{ + int callCount; + std::string lastName; + std::string lastValue; + bool returnValue; +}; + +SetHandlerSpy spy; + +bool RecordSet(const char* name, const char* value) +{ + spy.callCount++; + spy.lastName = name; + spy.lastValue = value; + return spy.returnValue; +} + +const char* const STDOUT_CAPTURE_PATH = "/tmp/solidsyslog_example_interactive_stdout.txt"; +int saved_stdout_fd = -1; + +void StartStdoutCapture() +{ + fflush(stdout); + saved_stdout_fd = dup(fileno(stdout)); + (void) freopen(STDOUT_CAPTURE_PATH, "w", stdout); + setvbuf(stdout, nullptr, _IONBF, 0); +} + +std::string EndStdoutCapture() +{ + fflush(stdout); + (void) dup2(saved_stdout_fd, fileno(stdout)); + close(saved_stdout_fd); + saved_stdout_fd = -1; + + std::string content; + FILE* f = fopen(STDOUT_CAPTURE_PATH, "r"); + if (f != nullptr) + { + char buf[2048]; + size_t n = 0; + while ((n = fread(buf, 1, sizeof(buf), f)) > 0) + { + content.append(buf, n); + } + fclose(f); + } + return content; +} + +void RunWithInput(const char* input, ExampleInteractiveSetHandler onSet) +{ + FILE* in = fmemopen((void*) input, strlen(input), "r"); + struct SolidSyslogMessage message = {}; + ExampleInteractive_Run(&message, in, nullptr, onSet); + fclose(in); +} + +std::string RunCapturingStdout(const char* input, ExampleInteractiveSetHandler onSet) +{ + StartStdoutCapture(); + RunWithInput(input, onSet); + return EndStdoutCapture(); +} +} // namespace + +// clang-format off +TEST_GROUP(ExampleInteractive) +{ + void setup() override + { + spy = SetHandlerSpy{}; + spy.returnValue = true; + } +}; +// clang-format on + +TEST(ExampleInteractive, SetHandlerNotCalledWithQuitOnly) +{ + RunWithInput("quit\n", RecordSet); + + LONGS_EQUAL(0, spy.callCount); +} + +TEST(ExampleInteractive, SetCommandCallsHandlerOnce) +{ + RunWithInput("set hostname Foo\nquit\n", RecordSet); + + LONGS_EQUAL(1, spy.callCount); +} + +TEST(ExampleInteractive, SetCommandPassesNameToHandler) +{ + RunWithInput("set hostname Foo\nquit\n", RecordSet); + + STRCMP_EQUAL("hostname", spy.lastName.c_str()); +} + +TEST(ExampleInteractive, SetCommandPassesValueToHandler) +{ + RunWithInput("set hostname Foo\nquit\n", RecordSet); + + STRCMP_EQUAL("Foo", spy.lastValue.c_str()); +} + +TEST(ExampleInteractive, SetCommandWithoutValueGivesEmptyValue) +{ + RunWithInput("set hostname\nquit\n", RecordSet); + + STRCMP_EQUAL("hostname", spy.lastName.c_str()); + STRCMP_EQUAL("", spy.lastValue.c_str()); +} + +TEST(ExampleInteractive, SetCommandWithEmbeddedSpacesPreservesValueAfterFirst) +{ + RunWithInput("set msg some text\nquit\n", RecordSet); + + STRCMP_EQUAL("msg", spy.lastName.c_str()); + STRCMP_EQUAL("some text", spy.lastValue.c_str()); +} + +TEST(ExampleInteractive, NullSetHandlerSilentlyIgnoresSetLine) +{ + RunWithInput("set hostname Foo\nquit\n", nullptr); + + LONGS_EQUAL(0, spy.callCount); +} + +TEST(ExampleInteractive, SetCommandPrintsEchoOnHandlerSuccess) +{ + spy.returnValue = true; + + std::string captured = RunCapturingStdout("set hostname Foo\nquit\n", RecordSet); + + CHECK(captured.find("set hostname=Foo") != std::string::npos); +} + +TEST(ExampleInteractive, SetCommandPrintsInvalidOnHandlerFailure) +{ + spy.returnValue = false; + + std::string captured = RunCapturingStdout("set hostname Foo\nquit\n", RecordSet); + + CHECK(captured.find("set: invalid") != std::string::npos); +} + +/* Test list (ZOMBIES order): + * Z [x] SetHandlerNotCalledWithQuitOnly + * O [x] SetCommandCallsHandlerOnce + * O.2[x] SetCommandPassesNameToHandler + * O.3[x] SetCommandPassesValueToHandler + * B [x] SetCommandWithoutValueGivesEmptyValue (locked in) + * B [x] SetCommandWithEmbeddedSpacesPreservesValueAfterFirst (locked in) + * E [x] NullSetHandlerSilentlyIgnoresSetLine (locked in) + * I [x] SetCommandPrintsEchoOnHandlerSuccess + * I [x] SetCommandPrintsInvalidOnHandlerFailure + */ From d229ef6935f7e0ae383cc3038138d1059f27339d Mon Sep 17 00:00:00 2001 From: David Cozens Date: Sat, 9 May 2026 15:57:20 +0000 Subject: [PATCH 2/2] fix: address CI + CodeRabbit review feedback on slice 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cppcheck (`[cstyleCast]`): replace `(void*) input` in ExampleInteractiveTest's RunWithInput with `const_cast(input)` + rationale comment (fmemopen mode "r" never writes to the buffer). clang-tidy (analyze-tidy): - SetHandlerSpy: in-class default initializers for callCount and returnValue (cppcoreguidelines-pro-type-member-init). - Stdout-capture helpers + RunWithInput: NOLINT comments for the owning-memory checks on dup / freopen / fopen / fclose ×2 and the clang-analyzer-unix.Stream check on the fread loop, matching the pattern already established in SolidSyslogExampleTest.cpp for the same POSIX-C test-helper idioms. - const_cast in RunWithInput: NOLINT for cppcoreguidelines-pro-type-const-cast cross-referencing the existing rationale comment. CodeRabbit nitpick (TryParseUInt): switch from `strtol` + explicit `< 0` rejection to `strtoul` for semantic correctness on unsigned parsing. Behaviour preserved because each call site already range-checks (port <= UINT16_MAX, facility <= 23, severity <= 7) and strtoul's wrap-on-negative is caught by those upper bounds; comment documents the contract. CodeRabbit MD040: add `text` language tag to the QEMU smoke fenced block in the DEVLOG slice-4 entry. Declined (replies on threads, no code change): - ExampleInteractiveSetHandler context parameter — Tier 3 example code; would also pull Switch migration into scope per CLAUDE.md "the class that owns them is next touched". - g_endpointVersion bump on host mutation — wasted work today (resolver still ignores endpoint->host); lands in the follow-up slice that wires the resolver to dotted-quad parsing. Local verification: cppcheck preset clean, tidy preset clean, freertos-cross builds clean, ExampleTests 63/63 (77 checks), clang-format --dry-run --Werror clean on all touched files. Co-Authored-By: Claude Opus 4.7 (1M context) --- DEVLOG.md | 2 +- Example/FreeRtos/SingleTask/main.c | 11 +++++++---- Tests/Example/ExampleInteractiveTest.cpp | 17 +++++++++++++---- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/DEVLOG.md b/DEVLOG.md index 4360fb73..9c52c51e 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -95,7 +95,7 @@ instead of the cached resolver result. ### QEMU smoke -``` +```text PORT=5514 BYTES=<134>… FreeRtosExample … (default) set hostname QemuFoo → set hostname=QemuFoo PORT=5514 BYTES=<134>… QemuFoo … (mutated) diff --git a/Example/FreeRtos/SingleTask/main.c b/Example/FreeRtos/SingleTask/main.c index 75af5870..77f2453f 100644 --- a/Example/FreeRtos/SingleTask/main.c +++ b/Example/FreeRtos/SingleTask/main.c @@ -288,13 +288,16 @@ static bool TryParseUInt(const char* value, unsigned long* out) { return false; } - char* end = NULL; - long parsed = strtol(value, &end, 10); - if ((*end != '\0') || (parsed < 0)) + char* end = NULL; + unsigned long parsed = strtoul(value, &end, 10); + /* strtoul accepts a leading '-' and wraps to a huge unsigned, but each + * call site range-checks (port <= UINT16_MAX, facility <= 23, + * severity <= 7) so wrapped values are still rejected upstream. */ + if (*end != '\0') { return false; } - *out = (unsigned long) parsed; + *out = parsed; return true; } diff --git a/Tests/Example/ExampleInteractiveTest.cpp b/Tests/Example/ExampleInteractiveTest.cpp index 7604fab6..9d712598 100644 --- a/Tests/Example/ExampleInteractiveTest.cpp +++ b/Tests/Example/ExampleInteractiveTest.cpp @@ -11,10 +11,10 @@ namespace { struct SetHandlerSpy { - int callCount; + int callCount = 0; std::string lastName; std::string lastValue; - bool returnValue; + bool returnValue = false; }; SetHandlerSpy spy; @@ -33,7 +33,9 @@ int saved_stdout_fd = -1; void StartStdoutCapture() { fflush(stdout); + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) -- test helper; FD is hand-managed and restored in EndStdoutCapture saved_stdout_fd = dup(fileno(stdout)); + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) -- freopen redirects stdout for test; no ownership transfer (void) freopen(STDOUT_CAPTURE_PATH, "w", stdout); setvbuf(stdout, nullptr, _IONBF, 0); } @@ -46,15 +48,18 @@ std::string EndStdoutCapture() saved_stdout_fd = -1; std::string content; - FILE* f = fopen(STDOUT_CAPTURE_PATH, "r"); + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) -- fopen/fclose is POSIX C; no owning memory concern + FILE* f = fopen(STDOUT_CAPTURE_PATH, "r"); if (f != nullptr) { char buf[2048]; size_t n = 0; + // NOLINTNEXTLINE(clang-analyzer-unix.Stream) -- test helper; fread on EOF/error returns 0 and exits the loop cleanly while ((n = fread(buf, 1, sizeof(buf), f)) > 0) { content.append(buf, n); } + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) -- fclose is POSIX C; no owning memory concern fclose(f); } return content; @@ -62,9 +67,13 @@ std::string EndStdoutCapture() void RunWithInput(const char* input, ExampleInteractiveSetHandler onSet) { - FILE* in = fmemopen((void*) input, strlen(input), "r"); + /* fmemopen takes a non-const void*; with mode "r" it never writes to + * the buffer, so dropping const here is safe in practice. */ + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) -- see comment above; fmemopen mode "r" never writes + FILE* in = fmemopen(const_cast(input), strlen(input), "r"); struct SolidSyslogMessage message = {}; ExampleInteractive_Run(&message, in, nullptr, onSet); + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) -- fclose is POSIX C; no owning memory concern fclose(in); }