From 784014e8a7e05be26076dfa97609c1adecbca4c3 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 8 Sep 2026 23:52:54 +0000 Subject: [PATCH] fix(ci): unbreak the ESP32 static analysis gate after the cppcheck 2.20 jump (#11777) Every PR targeting develop has been red since 2026-09-07 on the seven ESP32 check jobs, while the nRF52, RP2040 and STM32 jobs pass on identical source. gh-action-firmware#61 moved the ESP32 container images onto the pioarduino core. Its esp32 platform ships its own tool-cppcheck 2.20.1 and reinstalls it over anything the repo pins, so ESP32 now analyses with cppcheck 2.20 while every other platform still resolves platformio/tool-cppcheck 1.21100.230717, i.e. 2.11. 2.20 parses far more of this tree than 2.11 ever managed, so checks that were always enabled fired for the first time: 388 defects, ESP32 only. #11776 cleared the two unknownMacro errors. Of the 386 left, two are worth acting on and are fixed rather than suppressed: * SerialModule dereferenced a null Position in NMEA/CALTOPO mode. `decoded` stays NULL when pb_decode_from_bytes() fails, but printWPL() was called with *decoded regardless, so a malformed position payload on our portnum crashed the node. Emit the waypoint only on a successful decode. * InkHUD's 12-hour clock passed a signed 12 to a %u conversion. The rest are style and performance suggestions - functionStatic and the const-correctness family account for 351 of them. Suppress those check ids so the gate means the same thing on every platform again, scoping the one-off ones to their file so a new occurrence elsewhere still fails. Burning them down is worth doing deliberately, not under a CI outage. Verified in the CI container images: all seven previously failing ESP32 environments pass, and rak4631, tracker-t1000-e and t-echo-plus still pass under cppcheck 2.11. --- src/graphics/niche/InkHUD/Applet.cpp | 2 +- src/modules/SerialModule.cpp | 11 +++---- suppressions.txt | 44 ++++++++++++++++++++++++---- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/graphics/niche/InkHUD/Applet.cpp b/src/graphics/niche/InkHUD/Applet.cpp index 50efff8ffe..03fdd0ea40 100644 --- a/src/graphics/niche/InkHUD/Applet.cpp +++ b/src/graphics/niche/InkHUD/Applet.cpp @@ -635,7 +635,7 @@ std::string InkHUD::Applet::getTimeString(uint32_t epochSeconds) // Format the clock string, either 12 hour or 24 hour char clockStr[11]; if (config.display.use_12h_clock) - sprintf(clockStr, "%u:%02u %s", (hour % 12 == 0 ? 12 : hour % 12), min, hour > 11 ? "PM" : "AM"); + sprintf(clockStr, "%u:%02u %s", (hour % 12 == 0 ? 12u : hour % 12), min, hour > 11 ? "PM" : "AM"); else sprintf(clockStr, "%02u:%02u", hour, min); diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index ef26bc360e..c1b2d9d6b7 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -408,17 +408,14 @@ ProcessMessage SerialModuleRadio::handleReceived(const meshtastic_MeshPacket &mp HAS_GPS) { // Decode the Payload some more meshtastic_Position scratch; - meshtastic_Position *decoded = NULL; if (mp.which_payload_variant == meshtastic_MeshPacket_decoded_tag && mp.decoded.portnum == ourPortNum) { memset(&scratch, 0, sizeof(scratch)); + // A payload that fails to decode leaves nothing to report, so say nothing. if (pb_decode_from_bytes(p.payload.bytes, p.payload.size, &meshtastic_Position_msg, &scratch)) { - decoded = &scratch; - } - // send position packet as WPL to the serial port - { - meshtastic_NodeInfoLite *senderNode = nodeDB->getMeshNode(getFrom(&mp)); + // send position packet as WPL to the serial port + const meshtastic_NodeInfoLite *senderNode = nodeDB->getMeshNode(getFrom(&mp)); const char *senderName = senderNode ? senderNode->long_name : ""; - printWPL(outbuf, sizeof(outbuf), *decoded, senderName, + printWPL(outbuf, sizeof(outbuf), scratch, senderName, moduleConfig.serial.mode == meshtastic_ModuleConfig_SerialConfig_Serial_Mode_CALTOPO); serialPrint->printf("%s", outbuf); } diff --git a/suppressions.txt b/suppressions.txt index ca94c269d9..63c9b9208e 100644 --- a/suppressions.txt +++ b/suppressions.txt @@ -52,13 +52,8 @@ postfixOperator:*/mqtt/* missingOverride virtualCallInConstructor -passedByValue:*/RedirectablePrint.h - internalAstError:*/CrossPlatformCryptoEngine.cpp uninitMemberVar:*/AudioThread.h -// False positive -constVariableReference:*/Channels.cpp -constParameterPointer:*/unishox2.c // False positive: make_zeroizing_array() returns unique_ptr, so // .get() is uint8_t*, not void*. cppcheck can't resolve the custom-deleter alias @@ -67,4 +62,41 @@ arithOperationsOnVoidPointer:*/EncryptedStorage.cpp useStlAlgorithm -variableScope \ No newline at end of file +variableScope + +// cppcheck 2.20 (ESP32 only) +// +// The ESP32 CI images build on the pioarduino core, whose esp32 platform installs its own +// tool-cppcheck 2.20.1 and reinstalls it over any pinned version. Every other platform still +// resolves platformio/tool-cppcheck 1.21100.230717, i.e. cppcheck 2.11. 2.20 parses far more of +// this tree than 2.11 managed to, so checks that have always been enabled started firing for the +// first time - 388 defects on ESP32, none anywhere else, for identical source. +// +// Silence the checks that appeared with that jump so the gate means the same thing on every +// platform again. These are style and performance suggestions, not defects; burning them down is +// worth doing deliberately, not under a CI outage. +functionStatic +staticFunction +constParameterPointer +iterateByValue +returnByReference +passedByValue +uselessOverride + +constVariable +constVariablePointer +constVariableReference +constParameterReference + +// Single deliberate sites, scoped so a new one elsewhere still fails the gate. Router folds the +// bitfield's want_response bit into the decoded bool; Power interpolates the OCV curve in float. +bitwiseOnBoolean:*/Router.cpp +suspiciousFloatingPointCast:*/Power.cpp + +// The 2.20 successor to cstyleCast, which is already suppressed above for the same reason. +dangerousTypeCast + +// Sensor drivers hold a driver object they new in begin() and are constructed once, at global +// scope. cppcheck wants the rule of three on them; nothing ever copies one. +noCopyConstructor:*/Telemetry/Sensor/* +noOperatorEq:*/Telemetry/Sensor/*