refactor/chore: update cppcheck to 2.8 with needed refactoring (#4926)

* refactor/chore: update cppcheck to 2.8 with needed refactoring

* use probably invalid index for default

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

* trivial: rename skContributions -> m_sk_contributions and skContributions2 -> skContributions

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

contrib/containers/ci/Dockerfile

@@ -79,7 +79,7 @@ RUN apt-get update && apt-get install $APT_ARGS \
     xorriso \
     && rm -rf /var/lib/apt/lists/*
 
-ARG CPPCHECK_VERSION=2.4
+ARG CPPCHECK_VERSION=2.8
 RUN curl -sL "https://github.com/danmar/cppcheck/archive/${CPPCHECK_VERSION}.tar.gz" | tar -xvzf - --directory /tmp/
 RUN cd /tmp/cppcheck-${CPPCHECK_VERSION} && mkdir build && cd build && cmake .. && cmake --build . -j 8
 ENV PATH "/tmp/cppcheck-${CPPCHECK_VERSION}/build/bin:${PATH}"

src/bls/bls.cpp

@@ -61,7 +61,7 @@ void CBLSSecretKey::MakeNewKey()
     while (true) {
         GetStrongRandBytes(buf, sizeof(buf));
         try {
-            impl = bls::PrivateKey::FromBytes(bls::Bytes((const uint8_t*)buf, SerSize));
+            impl = bls::PrivateKey::FromBytes(bls::Bytes(reinterpret_cast<const uint8_t*>(buf), SerSize));
             break;
         } catch (...) {
         }
@@ -370,7 +370,7 @@ static mt_pooled_secure_allocator<uint8_t>& get_secure_allocator()
 static void* secure_allocate(size_t n)
 {
     uint8_t* ptr = get_secure_allocator().allocate(n + sizeof(size_t));
-    *(size_t*)ptr = n;
+    *reinterpret_cast<size_t*>(ptr) = n;
     return ptr + sizeof(size_t);
 }
 
@@ -380,8 +380,8 @@ static void secure_free(void* p)
         return;
     }
 
-    uint8_t* ptr = (uint8_t*)p - sizeof(size_t);
-    size_t n = *(size_t*)ptr;
+    uint8_t* ptr = reinterpret_cast<uint8_t*>(p) - sizeof(size_t);
+    size_t n = *reinterpret_cast<size_t*>(ptr);
     return get_secure_allocator().deallocate(ptr, n);
 }
 #endif

src/bls/bls.h

@@ -160,13 +160,13 @@ public:
     template <typename Stream>
     inline void Serialize(Stream& s) const
     {
-        s.write((const char*)ToByteVector().data(), SerSize);
+        s.write(reinterpret_cast<const char*>(ToByteVector().data()), SerSize);
     }
     template <typename Stream>
     inline void Unserialize(Stream& s, bool checkMalleable = true)
     {
         std::vector<uint8_t> vecBytes(SerSize, 0);
-        s.read((char*)vecBytes.data(), SerSize);
+        s.read(reinterpret_cast<char*>(vecBytes.data()), SerSize);
         SetByteVector(vecBytes);
 
         if (checkMalleable && !CheckMalleable(vecBytes)) {
@@ -360,14 +360,14 @@ public:
             bufValid = true;
             hash.SetNull();
         }
-        s.write((const char*)vecBytes.data(), vecBytes.size());
+        s.write(reinterpret_cast<const char*>(vecBytes.data()), vecBytes.size());
     }
 
     template<typename Stream>
     inline void Unserialize(Stream& s)
     {
         std::unique_lock<std::mutex> l(mutex);
-        s.read((char*)vecBytes.data(), BLSObject::SerSize);
+        s.read(reinterpret_cast<char*>(vecBytes.data()), BLSObject::SerSize);
         bufValid = true;
         objInitialized = false;
         hash.SetNull();
@@ -427,7 +427,7 @@ public:
         }
         if (hash.IsNull()) {
             CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
-            ss.write((const char*)vecBytes.data(), vecBytes.size());
+            ss.write(reinterpret_cast<const char*>(vecBytes.data()), vecBytes.size());
             hash = ss.GetHash();
         }
         return hash;

src/bls/bls_ies.cpp

@@ -14,9 +14,9 @@ static bool EncryptBlob(const void* in, size_t inSize, Out& out, const void* sym
 {
     out.resize(inSize);
 
-    AES256CBCEncrypt enc((const unsigned char*)symKey, (const unsigned char*)iv, false);
-    int w = enc.Encrypt((const unsigned char*)in, (int)inSize, (unsigned char*)out.data());
-    return w == (int)inSize;
+    AES256CBCEncrypt enc(reinterpret_cast<const unsigned char*>(symKey), reinterpret_cast<const unsigned char*>(iv), false);
+    int w = enc.Encrypt(reinterpret_cast<const unsigned char*>(in), int(inSize), reinterpret_cast<unsigned char*>(out.data()));
+    return w == int(inSize);
 }
 
 template <typename Out>
@@ -24,8 +24,8 @@ static bool DecryptBlob(const void* in, size_t inSize, Out& out, const void* sym
 {
     out.resize(inSize);
 
-    AES256CBCDecrypt enc((const unsigned char*)symKey, (const unsigned char*)iv, false);
-    int w = enc.Decrypt((const unsigned char*)in, (int)inSize, (unsigned char*)out.data());
+    AES256CBCDecrypt enc(reinterpret_cast<const unsigned char*>(symKey), reinterpret_cast<const unsigned char*>(iv), false);
+    int w = enc.Decrypt(reinterpret_cast<const unsigned char*>(in), int(inSize), reinterpret_cast<unsigned char*>(out.data()));
     return w == (int)inSize;
 }
 

src/ctpl_stl.h

@@ -129,7 +129,7 @@ namespace ctpl {
         std::function<void(int)> pop() {
             std::function<void(int id)> * _f = nullptr;
             this->q.pop(_f);
-            std::unique_ptr<std::function<void(int id)>> func(_f); // at return, delete the function even if an exception occurred
+            [[maybe_unused]] std::unique_ptr<std::function<void(int id)>> func(_f); // at return, delete the function even if an exception occurred
             std::function<void(int)> f;
             if (_f)
                 f = *_f;

src/evo/simplifiedmns.h

@@ -28,7 +28,7 @@ public:
     CService service;
     CBLSLazyPublicKey pubKeyOperator;
     CKeyID keyIDVoting;
-    bool isValid;
+    bool isValid{false};
     CScript scriptPayout; // mem-only
     CScript scriptOperatorPayout; // mem-only
 

src/hdchain.h

@@ -125,7 +125,7 @@ private:
     int nVersion{CHDPubKey::CURRENT_VERSION};
 
 public:
-    CExtPubKey extPubKey;
+    CExtPubKey extPubKey{};
     uint256 hdchainID;
     uint32_t nAccountIndex{0};
     uint32_t nChangeIndex{0};

src/llmq/dkgsession.cpp

@@ -144,7 +144,7 @@ void CDKGSession::Contribute(CDKGPendingMessages& pendingMessages)
 
     cxxtimer::Timer t1(true);
     logger.Batch("generating contributions");
-    if (!blsWorker.GenerateContributions(params.threshold, memberIds, vvecContribution, skContributions)) {
+    if (!blsWorker.GenerateContributions(params.threshold, memberIds, vvecContribution, m_sk_contributions)) {
         // this should never happen actually
         logger.Batch("GenerateContributions failed");
         return;
@@ -179,7 +179,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages)
 
     for (const auto i : irange::range(members.size())) {
         const auto& m = members[i];
-        CBLSSecretKey skContrib = skContributions[i];
+        CBLSSecretKey skContrib = m_sk_contributions[i];
 
         if (i != myIdx && ShouldSimulateError("contribution-lie")) {
             logger.Batch("lying for %s", m->dmn->proTxHash.ToString());
@@ -689,7 +689,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const
         }
         logger.Batch("justifying for %s", m->dmn->proTxHash.ToString());
 
-        CBLSSecretKey skContribution = skContributions[i];
+        CBLSSecretKey skContribution = m_sk_contributions[i];
 
         if (i != myIdx && ShouldSimulateError("justify-lie")) {
             logger.Batch("lying for %s", m->dmn->proTxHash.ToString());

src/llmq/dkgsession.h

@@ -245,7 +245,7 @@ private:
     std::map<uint256, size_t> membersMap;
     std::set<uint256> relayMembers;
     BLSVerificationVectorPtr vvecContribution;
-    BLSSecretKeyVector skContributions;
+    BLSSecretKeyVector m_sk_contributions;
 
     BLSIdVector memberIds;
     std::vector<BLSVerificationVectorPtr> receivedVvecs;

src/llmq/signing_shares.h

@@ -35,7 +35,7 @@ constexpr uint32_t UNINITIALIZED_SESSION_ID{std::numeric_limits<uint32_t>::max()
 class CSigShare : virtual public CSigBase
 {
 protected:
-    uint16_t quorumMember;
+    uint16_t quorumMember{std::numeric_limits<uint16_t>::max()};
 public:
     CBLSLazySignature sigShare;
 

src/llmq/snapshot.h

@@ -112,7 +112,7 @@ public:
     CSimplifiedMNListDiff mnListDiffAtHMinus2C;
     CSimplifiedMNListDiff mnListDiffAtHMinus3C;
 
-    bool extraShare;
+    bool extraShare{false};
     std::optional<CQuorumSnapshot> quorumSnapshotAtHMinus4C;
     std::optional<CSimplifiedMNListDiff> mnListDiffAtHMinus4C;
 

src/rpc/governance.cpp

@@ -379,7 +379,6 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
 
     std::string strHash = govobj.GetHash().ToString();
 
-    std::string strError = "";
     bool fMissingConfirmations;
     {
         if (g_txindex) {
@@ -387,6 +386,7 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
         }
 
         LOCK2(cs_main, ::mempool.cs);
+        std::string strError;
         if (!govobj.IsValidLocally(strError, fMissingConfirmations, true) && !fMissingConfirmations) {
             LogPrintf("gobject(submit) -- Object submission rejected because object is not valid - hash = %s, strError = %s\n", strHash, strError);
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Governance object is not valid - " + strHash + " - " + strError);

src/spork.h

@@ -174,7 +174,7 @@ private:
     std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage> > mapSporksActive GUARDED_BY(cs);
 
     std::set<CKeyID> setSporkPubKeyIDs GUARDED_BY(cs);
-    int nMinSporkKeys GUARDED_BY(cs);
+    int nMinSporkKeys GUARDED_BY(cs) {std::numeric_limits<int>::max()};
     CKey sporkPrivKey GUARDED_BY(cs);
 
     /**

src/stacktraces.cpp

@@ -266,7 +266,7 @@ static uint64_t GetBaseAddress()
 #else
 static int dl_iterate_callback(struct dl_phdr_info* info, size_t s, void* data)
 {
-    uint64_t* p = (uint64_t*)data;
+    uint64_t* p = reinterpret_cast<uint64_t*>(data);
     if (info->dlpi_name == nullptr || info->dlpi_name[0] == '\0') {
         *p = info->dlpi_addr;
     }
@@ -367,7 +367,7 @@ static std::vector<stackframe_info> GetStackFrameInfos(const std::vector<uint64_
 struct crash_info_header
 {
     std::string magic;
-    uint16_t version;
+    uint16_t version{0};
     std::string exeFileName;
 
     SERIALIZE_METHODS(crash_info_header, obj)

test/lint/lint-cppcheck-dash.sh

@@ -50,6 +50,8 @@ IGNORED_WARNINGS=(
 #    "Member variable '.*' is not initialized in the constructor."
 
     "unusedFunction"
+    "unknownMacro"
+    "unusedStructMember"
 )
 
 # We should attempt to update this with all dash specific code