From 6883bc7afc0c3b66a1649abf534118149d3d3594 Mon Sep 17 00:00:00 2001 From: Mike Kinney Date: Mon, 24 Jan 2022 19:58:07 +0000 Subject: [PATCH] fix more warnings; add to CI; suppress some warnings --- .github/workflows/main.yml | 7 +++++++ bin/check-all.sh | 15 +++++++++------ platformio.ini | 4 ++-- src/PowerFSM.cpp | 3 +-- src/esp32/SimpleAllocator.h | 2 +- src/mesh/MemoryPool.h | 2 +- src/mesh/PhoneAPI.h | 2 +- src/mesh/http/ContentHandler.h | 2 +- src/mesh/wifi/WiFiServerAPI.h | 2 +- src/nimble/NimbleBluetoothAPI.h | 6 +++--- src/plugins/esp32/RangeTestPlugin.cpp | 2 +- src/plugins/esp32/StoreForwardPlugin.cpp | 7 ++++--- src/plugins/esp32/StoreForwardPlugin.h | 10 +++++----- suppressions.txt | 14 ++++++++++++++ 14 files changed, 51 insertions(+), 27 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0425f13fa..78f7b6f1c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,6 +24,10 @@ jobs: with: submodules: 'recursive' + - name: Install cppcheck + run: | + apt-get install -y cppcheck + - name: Setup Python uses: actions/setup-python@v2 with: @@ -88,6 +92,9 @@ jobs: # - name: Build for wisblock RAK4631 # run: platformio run -e rak4631 + - name: Check everything + run: bin/check-all.sh + - name: Build everything run: bin/build-all.sh diff --git a/bin/check-all.sh b/bin/check-all.sh index f54ff03e4..90413bb25 100755 --- a/bin/check-all.sh +++ b/bin/check-all.sh @@ -9,11 +9,14 @@ VERSION=`bin/buildinfo.py long` # The shell vars the build tool expects to find export APP_VERSION=$VERSION -# only check high and medium in our source -# TODO: only doing tbeam (to start; add all/more later) -#pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" -e tbeam --skip-packages --severity=medium --severity=high --pattern="src/" -pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" -e tbeam --skip-packages --pattern="src/" -return_code=$? +if [[ $# -gt 0 ]]; then + # can override which environment by passing arg + BOARDS="-e $1" +else + BOARDS="-e tlora-v2 -e tlora-v1 -e tlora_v1_3 -e tlora-v2-1-1.6 -e tbeam -e heltec-v1 -e heltec-v2.0 -e heltec-v2.1 -e tbeam0.7 -e meshtastic-diy-v1 -e rak4631_5005 -e rak4631_19003 -e t-echo" +fi +#echo "BOARDS:${BOARDS}" +pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" $BOARDS --skip-packages --pattern="src/" +#return_code=$? # TODO: not sure why return_code is 0 -echo "return_code:${return_code}" diff --git a/platformio.ini b/platformio.ini index ca6c06a0d..98a9a7733 100644 --- a/platformio.ini +++ b/platformio.ini @@ -84,9 +84,9 @@ lib_deps = SPI https://github.com/geeksville/ArduinoThread.git#72921ac222eed6f526ba1682023cee290d9aa1b3 PubSubClient - + ; Used for the code analysis in PIO Home / Inspect -check_tool = cppcheck, clangtidy +check_tool = cppcheck check_skip_packages = yes ; Common settings for conventional (non Portduino) Arduino targets diff --git a/src/PowerFSM.cpp b/src/PowerFSM.cpp index 3316b208e..095f844b5 100644 --- a/src/PowerFSM.cpp +++ b/src/PowerFSM.cpp @@ -63,7 +63,6 @@ static void lsIdle() // DEBUG_MSG("lsIdle begin ls_secs=%u\n", getPref_ls_secs()); #ifndef NO_ESP32 - esp_sleep_source_t wakeCause2 = ESP_SLEEP_WAKEUP_UNDEFINED; // Do we have more sleeping to do? if (secsSlept < getPref_ls_secs()) { @@ -73,7 +72,7 @@ static void lsIdle() // If some other service would stall sleep, don't let sleep happen yet if (doPreflightSleep()) { setLed(false); // Never leave led on while in light sleep - wakeCause2 = doLightSleep(sleepTime * 1000LL); + esp_sleep_source_t wakeCause2 = doLightSleep(sleepTime * 1000LL); switch (wakeCause2) { case ESP_SLEEP_WAKEUP_TIMER: diff --git a/src/esp32/SimpleAllocator.h b/src/esp32/SimpleAllocator.h index a529c1a50..5fdb6738c 100644 --- a/src/esp32/SimpleAllocator.h +++ b/src/esp32/SimpleAllocator.h @@ -37,6 +37,6 @@ void *operator new(size_t size, SimpleAllocator &p); */ class AllocatorScope { public: - AllocatorScope(SimpleAllocator &a); + explicit AllocatorScope(SimpleAllocator &a); ~AllocatorScope(); }; diff --git a/src/mesh/MemoryPool.h b/src/mesh/MemoryPool.h index de97fe259..84cac7eff 100644 --- a/src/mesh/MemoryPool.h +++ b/src/mesh/MemoryPool.h @@ -99,7 +99,7 @@ template class MemoryPool : public Allocator ~MemoryPool() { delete[] buf; } /// Return a buffer for use by others - virtual void release(T *p) + void release(T *p) { assert(p >= buf && (size_t)(p - buf) < diff --git a/src/mesh/PhoneAPI.h b/src/mesh/PhoneAPI.h index 67b52d20a..5a4ca381d 100644 --- a/src/mesh/PhoneAPI.h +++ b/src/mesh/PhoneAPI.h @@ -58,7 +58,7 @@ class PhoneAPI // Call this when the client drops the connection, resets the state to STATE_SEND_NOTHING // Unregisters our observer. A closed connection **can** be reopened by calling init again. - virtual void close(); + void close(); /** * Handle a ToRadio protobuf diff --git a/src/mesh/http/ContentHandler.h b/src/mesh/http/ContentHandler.h index 541fcff39..c3babdd72 100644 --- a/src/mesh/http/ContentHandler.h +++ b/src/mesh/http/ContentHandler.h @@ -36,7 +36,7 @@ class HttpAPI : public PhoneAPI protected: /// Check the current underlying physical link to see if the client is currently connected - virtual bool checkIsConnected() { return true; } // FIXME, be smarter about this + virtual bool checkIsConnected() override { return true; } // FIXME, be smarter about this }; diff --git a/src/mesh/wifi/WiFiServerAPI.h b/src/mesh/wifi/WiFiServerAPI.h index 272dd29ac..d3750e8c0 100644 --- a/src/mesh/wifi/WiFiServerAPI.h +++ b/src/mesh/wifi/WiFiServerAPI.h @@ -18,7 +18,7 @@ class WiFiServerAPI : public StreamAPI virtual ~WiFiServerAPI(); /// override close to also shutdown the TCP link - virtual void close() override; + virtual void close(); protected: /// We override this method to prevent publishing EVENT_SERIAL_CONNECTED/DISCONNECTED for wifi links (we want the board to diff --git a/src/nimble/NimbleBluetoothAPI.h b/src/nimble/NimbleBluetoothAPI.h index e2260ba4d..4df2498b8 100644 --- a/src/nimble/NimbleBluetoothAPI.h +++ b/src/nimble/NimbleBluetoothAPI.h @@ -10,10 +10,10 @@ protected: /** * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies) */ - virtual void onNowHasData(uint32_t fromRadioNum); + virtual void onNowHasData(uint32_t fromRadioNum) override; /// Check the current underlying physical link to see if the client is currently connected - virtual bool checkIsConnected(); + virtual bool checkIsConnected() override; }; -extern PhoneAPI *bluetoothPhoneAPI; \ No newline at end of file +extern PhoneAPI *bluetoothPhoneAPI; diff --git a/src/plugins/esp32/RangeTestPlugin.cpp b/src/plugins/esp32/RangeTestPlugin.cpp index aa1881326..9ce009931 100644 --- a/src/plugins/esp32/RangeTestPlugin.cpp +++ b/src/plugins/esp32/RangeTestPlugin.cpp @@ -87,8 +87,8 @@ int32_t RangeTestPlugin::runOnce() DEBUG_MSG("Range Test Plugin - Disabled\n"); } - return (INT32_MAX); #endif + return (INT32_MAX); } MeshPacket *RangeTestPluginRadio::allocReply() diff --git a/src/plugins/esp32/StoreForwardPlugin.cpp b/src/plugins/esp32/StoreForwardPlugin.cpp index 24535793e..f556359f2 100644 --- a/src/plugins/esp32/StoreForwardPlugin.cpp +++ b/src/plugins/esp32/StoreForwardPlugin.cpp @@ -267,9 +267,10 @@ ProcessMessage StoreForwardPlugin::handleReceived(const MeshPacket &mp) } } else if ((p.payload.bytes[0] == 'S') && (p.payload.bytes[1] == 'F') && (p.payload.bytes[2] == 'm') && (p.payload.bytes[3] == 0x00)) { - strcpy(this->routerMessage, "01234567890123456789012345678901234567890123456789012345678901234567890123456789" - "01234567890123456789012345678901234567890123456789012345678901234567890123456789" - "01234567890123456789012345678901234567890123456789012345678901234567890123456"); + strlcpy(this->routerMessage, "01234567890123456789012345678901234567890123456789012345678901234567890123456789" + "01234567890123456789012345678901234567890123456789012345678901234567890123456789" + "01234567890123456789012345678901234567890123456789012345678901234567890123456", + sizeof(this->routerMessage)); storeForwardPlugin->sendMessage(getFrom(&mp), this->routerMessage); } else { diff --git a/src/plugins/esp32/StoreForwardPlugin.h b/src/plugins/esp32/StoreForwardPlugin.h index 110b0a75a..2514b89bf 100644 --- a/src/plugins/esp32/StoreForwardPlugin.h +++ b/src/plugins/esp32/StoreForwardPlugin.h @@ -22,16 +22,16 @@ class StoreForwardPlugin : public SinglePortPlugin, private concurrency::OSThrea { // bool firstTime = 1; bool busy = 0; - uint32_t busyTo; - char routerMessage[Constants_DATA_PAYLOAD_LEN]; + uint32_t busyTo = 0; + char routerMessage[Constants_DATA_PAYLOAD_LEN] = {0}; uint32_t receivedRecord[50][2] = {{0}}; - PacketHistoryStruct *packetHistory; + PacketHistoryStruct *packetHistory = 0; uint32_t packetHistoryCurrent = 0; - PacketHistoryStruct *packetHistoryTXQueue; - uint32_t packetHistoryTXQueue_size; + PacketHistoryStruct *packetHistoryTXQueue = 0; + uint32_t packetHistoryTXQueue_size = 0; uint32_t packetHistoryTXQueue_index = 0; uint32_t packetTimeMax = 2000; diff --git a/suppressions.txt b/suppressions.txt index d05ed57c9..cdf858753 100644 --- a/suppressions.txt +++ b/suppressions.txt @@ -12,6 +12,8 @@ duplInheritedMember // in src/mesh/MemoryPool.h memsetClassFloat +knownConditionTrueFalse + // no real downside/harm in these unusedFunction unusedPrivateFunction @@ -19,6 +21,18 @@ unusedPrivateFunction // most likely due to a cppcheck configuration issue (like missing an include) syntaxError +// try to quiet a few +//useInitializationList:src/main.cpp +useInitializationList + +//unreadVariable:src/graphics/Screen.cpp +unreadVariable + +redundantInitialization + +//cstyleCast:src/mesh/MemoryPool.h:71 +cstyleCast + // ignore stuff that is not ours *:.pio/* *:*/libdeps/*