refactor: S24.18 drop do/while(0) wrapper from test CHECK_* macros

CLAUDE.md

@@ -549,10 +549,18 @@ code should follow them; reviewers should call out drift.
 - **`CHECK_*` macros for repeated assertion shapes.** When the same buf+memcmp
   or several-line assertion repeats across tests, wrap it in a macro that
   *names* the intent. The macro must be a macro (not a function) so test
-  failures report the caller's `__FILE__`/`__LINE__`. Wrap with
-  `// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)`
-  and use `do { ... } while (0)` for safe single-statement use. Examples:
-  `CHECK_PRIVAL` in `SolidSyslogTest.cpp`, `CHECK_BLOCK_CONTAINS` in
+  failures report the caller's `__FILE__`/`__LINE__`. A multi-statement body
+  is a plain compound `{ ... }`; a single-statement body is just the bare
+  expression. Do **not** use the `do { ... } while (0)` wrapper: its only job
+  is to stop a multi-statement macro's tail escaping an unbraced `if`/`else`,
+  and `.clang-format`'s `InsertBraces: true` (our MISRA 15.6 enforcement)
+  already braces every conditional body before the macro expands, so the
+  wrapper is dead weight — and `cppcoreguidelines-avoid-do-while` (kept on for
+  the `Tests/` tier) now rejects it. No `NOLINT` is needed: `Tests/.clang-tidy`
+  disables `cppcoreguidelines-macro-usage` tier-wide, so a `CHECK_*` macro
+  needs no per-site suppression. Examples: `CHECK_PRIVAL` (single-statement,
+  bare expression) in `SolidSyslogMessageFormatterTest.cpp`,
+  `CHECK_BLOCK_CONTAINS` (declares a local, so keeps its `{ ... }` block) in
   `SolidSyslogFileBlockDeviceTest.cpp`.
 - **DRY the setup, DRY the asserts, keep the test body small.** Each `TEST(...)`
   body should read as a sentence: arrange → act → assert. If three lines of

DEVLOG.md

@@ -1,5 +1,46 @@
 # Dev Log
 
+## 2026-06-06 — S24.18 drop do/while(0) from test CHECK_* macros
+
+Mechanical tree-wide sweep: removed the `do { ... } while (0)` wrapper (and the
+paired `NOLINTBEGIN/END(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)`
+brackets) from every test assertion/helper macro — 42 files, ~60 macros. The
+wrapper's only purpose is to stop a multi-statement macro's tail escaping an
+unbraced `if`/`else`, and `.clang-format`'s `InsertBraces: true` (our MISRA 15.6
+enforcement) already braces every conditional body before a macro expands, so the
+wrapper was dead weight.
+
+### Decisions
+- **Conversion shape.** Multi-statement bodies → plain compound `{ ... }`
+  (preserves block scope, which matters for macros that declare a local, e.g.
+  `CHECK_BLOCK_CONTAINS`'s `checkBuf`, `CHECK_IS_FALLBACK`'s `for` loop var).
+  Single-statement macros were already bare expressions — untouched.
+- **Re-enabled `cppcoreguidelines-avoid-do-while` in `Tests/.clang-tidy`**
+  (removed it from the tier disable). The check now actively *bans* do/while in
+  test code, so the wrapper can't creep back via a CodeRabbit-style suggestion.
+  `cppcoreguidelines-macro-usage` stays disabled — CppUTest needs macros for
+  caller `__FILE__`/`__LINE__`, so no per-site NOLINT is needed.
+- **Upstream-convention macros left as-is.** `configASSERT` (FreeRTOS) and
+  `LWIP_PLATFORM_DIAG`/`LWIP_PLATFORM_ASSERT` (lwIP) in the fake config headers
+  keep their `do/while(0)` — they are consumed by un-formatted upstream code
+  where InsertBraces gives no protection. Safe under the re-enabled check:
+  `configASSERT` has zero use sites in linted code, and `LWIP_ASSERT` expands
+  only in `Bdd/Targets/FreeRtosLwip/netif/EthernetIf.c` (governed by
+  `Bdd/.clang-tidy`, not `Tests/.clang-tidy`). A definition is not a statement,
+  so the check never fires on the macro body itself.
+- Updated the CLAUDE.md "Test code" design-pattern entry to record the new rule
+  and the InsertBraces rationale; fixed a stale `CHECK_PRIVAL` path reference
+  (now `SolidSyslogMessageFormatterTest.cpp`).
+
+### Verification
+- Builds run via docker compose (`.devcontainer/docker-compose.yml`, gcc image).
+  debug: **1482 tests green**, no behaviour change. `tidy` preset green with
+  `avoid-do-while` enforced (no do/while warnings). `cppcheck` clean rebuild
+  green. clang-format dry-run clean tree-wide. freertos-host debug build green
+  (covers the FreeRtos/Lwip/MbedTls/FatFs/PlusFat test trees).
+- Windows/Winsock and the freertos/lwip clang-tidy lanes are CI's job; the
+  conversions there are identical and mechanical.
+
 ## 2026-06-06 — S14.03 migrate TimeQualitySd onto the SD writer (walking skeleton)
 
 First real consumer of the E14 SD writer. `TimeQualitySd_Format` now emits via

Tests/.clang-tidy

@@ -36,11 +36,18 @@
 #   (FreeRTOSConfig.h, lwipopts.h, …) which the test harness includes:
 #   the upstream symbols must remain macros.
 #
-# -cppcoreguidelines-avoid-do-while
-#   Paired with the macro-usage relaxation above: do { ... } while (0)
-#   is the standard idiom for wrapping a multi-statement macro body so
-#   it remains a single statement at the call site. Without the wrapper,
-#   the macros lose composability inside if/else.
+# cppcoreguidelines-avoid-do-while is intentionally NOT disabled (S24.18).
+#   Multi-statement test assertion/helper macros use a plain compound
+#   `{ ... }` body, not the `do { ... } while (0)` wrapper. The wrapper's
+#   only job — keeping a multi-statement macro a single statement so its
+#   tail cannot escape an unbraced if/else — cannot bite us: `.clang-format`
+#   sets `InsertBraces: true` (our MISRA 15.6 enforcement), so every
+#   conditional body is already brace-delimited before any macro expands.
+#   Leaving the check on keeps the wrapper from creeping back in. The
+#   upstream-convention `do/while(0)` macro *definitions* in the FreeRTOS /
+#   lwIP fake config headers (configASSERT, LWIP_PLATFORM_*) are exempt by
+#   construction: a definition is not a statement, so the check never fires
+#   on it, and those macros are not expanded in any file this config governs.
 #
 # -cppcoreguidelines-pro-type-reinterpret-cast
 #   Test fixtures legitimately reinterpret-cast for two patterns:
@@ -58,5 +65,4 @@ Checks: >
   -bugprone-easily-swappable-parameters,
   -google-build-using-namespace,
   -cppcoreguidelines-macro-usage,
-  -cppcoreguidelines-avoid-do-while,
   -cppcoreguidelines-pro-type-reinterpret-cast

Tests/Bdd/Targets/BddTargetErrorTextTest.cpp

@@ -8,14 +8,10 @@
 
 TEST_GROUP(BddTargetErrorText){};
 
-// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)
 #define CHECK_CATEGORY_TEXT(category, expectedText)                            \
-    do                                                                         \
     {                                                                          \
         STRCMP_EQUAL((expectedText), BddTargetErrorText_Category((category))); \
-    } while (0)
-
-// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)
+    }
 
 TEST(BddTargetErrorText, BadConfigCategoryMapsToText)
 {

Tests/FatFs/SolidSyslogFatFsFilePoolTest.cpp

@@ -19,15 +19,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 // clang-format off
 TEST_GROUP(SolidSyslogFatFsFilePool)

Tests/FreeRtos/SolidSyslogFreeRtosMutexTest.cpp

@@ -17,15 +17,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 // clang-format off
 TEST_GROUP(SolidSyslogFreeRtosMutex)

Tests/FreeRtos/SolidSyslogPlusTcpAddressTest.cpp

@@ -17,15 +17,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 // clang-format off
 TEST_GROUP(SolidSyslogPlusTcpAddress)

Tests/FreeRtos/SolidSyslogPlusTcpDatagramTest.cpp

@@ -27,15 +27,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 static const uint16_t TEST_PORT = 514;
 

Tests/FreeRtos/SolidSyslogPlusTcpResolverTest.cpp

@@ -25,15 +25,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 static const char* const TEST_HOST = "10.0.2.2";
 static const char* const TEST_ALTERNATE_HOST = "192.168.1.1";

Tests/FreeRtos/SolidSyslogPlusTcpTcpStreamTest.cpp

@@ -26,15 +26,14 @@ using namespace CososoTesting;
 
 // 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)
+    }
 
 static const uint16_t TEST_PORT = 514;
 static const char TEST_MESSAGE[] = "hello";
@@ -120,11 +119,10 @@ TEST_GROUP(SolidSyslogPlusTcpTcpStream)
 // clang-format on
 
 #define CHECK_SOCKET_CLOSED_ONCE()                                                                             \
-    do                                                                                                         \
     {                                                                                                          \
         CALLED_FAKE(FreeRtosSocketsFake_Closesocket, ONCE);                                                    \
         POINTERS_EQUAL(FreeRtosSocketsFake_LastSocketReturned(), FreeRtosSocketsFake_LastClosesocketSocket()); \
-    } while (0)
+    }
 
 TEST(SolidSyslogPlusTcpTcpStream, CreateReturnsNonNullStream)
 

Tests/Lwip/SolidSyslogLwipRawAddressTest.cpp

@@ -16,27 +16,25 @@ using namespace CososoTesting;
 
 // 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, expectedCategory, code)                   \
-    do                                                                             \
     {                                                                              \
         CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);                                \
         LONGS_EQUAL((severity), ErrorHandlerFake_LastSeverity());                  \
         POINTERS_EQUAL(&(source), ErrorHandlerFake_LastSource());                  \
         UNSIGNED_LONGS_EQUAL((expectedCategory), ErrorHandlerFake_LastCategory()); \
         UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastDetail());               \
-    } while (0)
+    }
 
 // clang-format off
 TEST_GROUP(SolidSyslogLwipRawAddress)

Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp

@@ -30,27 +30,25 @@ 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)                                                \
-    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, expectedCategory, code)                   \
-    do                                                                             \
     {                                                                              \
         CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);                                \
         LONGS_EQUAL((severity), ErrorHandlerFake_LastSeverity());                  \
         POINTERS_EQUAL(&(source), ErrorHandlerFake_LastSource());                  \
         UNSIGNED_LONGS_EQUAL((expectedCategory), ErrorHandlerFake_LastCategory()); \
         UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastDetail());               \
-    } 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

Tests/Lwip/SolidSyslogLwipRawDnsResolverTest.cpp

@@ -26,26 +26,24 @@ using namespace CososoTesting;
 
 // Asserts handle is non-null and not one of the slots in pool.
 #define CHECK_IS_FALLBACK(handle, pool)                                                \
-    do                                                                                 \
     {                                                                                  \
         CHECK_TEXT((handle) != nullptr, "Fallback handle was nullptr");                \
         for (auto* slot : (pool))                                                      \
         {                                                                              \
             CHECK_TEXT(slot != nullptr, "pool slot was nullptr (FillPool failed?)");   \
             CHECK_TEXT((handle) != slot, "Fallback handle collided with a pool slot"); \
         }                                                                              \
-    } while (0)
+    }
 
 // Asserts the most recent ErrorHandlerFake call matched (severity, source, code).
 #define CHECK_REPORTED(severity, source, expectedCategory, code)                   \
-    do                                                                             \
     {                                                                              \
         CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);                                \
         LONGS_EQUAL((severity), ErrorHandlerFake_LastSeverity());                  \
         POINTERS_EQUAL(&(source), ErrorHandlerFake_LastSource());                  \
         UNSIGNED_LONGS_EQUAL((expectedCategory), ErrorHandlerFake_LastCategory()); \
         UNSIGNED_LONGS_EQUAL((code), ErrorHandlerFake_LastDetail());               \
-    } while (0)
+    }
 
 static const char* const TEST_HOST = "syslog-ng";
 static const uint16_t TEST_PORT = 514;