refactor: Fix warnings from cppcheck

contrib/containers/ci/Dockerfile

@@ -43,9 +43,15 @@ RUN apt-get update && apt-get install $APT_ARGS g++-arm-linux-gnueabihf && rm -r
 RUN apt-get update && apt-get install $APT_ARGS g++-mingw-w64-x86-64 && rm -rf /var/lib/apt/lists/*
 RUN apt-get update && apt-get install $APT_ARGS wine-stable wine32 wine64 bc nsis xorriso libncurses5 && rm -rf /var/lib/apt/lists/*
 RUN apt-get update && apt-get install $APT_ARGS python3-zmq && rm -rf /var/lib/apt/lists/*
-RUN apt-get update && apt-get install $APT_ARGS cppcheck gawk jq && rm -rf /var/lib/apt/lists/*
+RUN apt-get update && apt-get install $APT_ARGS gawk jq && rm -rf /var/lib/apt/lists/*
 RUN apt-get update && apt-get install $APT_ARGS imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools && rm -rf /var/lib/apt/lists/*
 
+ARG CPPCHECK_VERSION=2.4
+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}"
+RUN mkdir /usr/local/share/Cppcheck && ln -s /tmp/cppcheck-${CPPCHECK_VERSION}/cfg/ /usr/local/share/Cppcheck/cfg
+
 ARG SHELLCHECK_VERSION=v0.7.1
 RUN curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/
 ENV PATH "/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}"

src/bench/bls.cpp

@@ -319,10 +319,10 @@ static void BLS_Verify_BatchedParallel(benchmark::Bench& bench)
     blsWorker.Start();
 
     // Benchmark.
-    size_t i = 0;
     bench.minEpochIterations(1000).run([&] {
         if (futures.size() < 100) {
             while (futures.size() < 10000) {
+                size_t i = 0;
                 auto f = blsWorker.AsyncVerifySig(sigs[i], pubKeys[i], msgHashes[i], cancelCond);
                 futures.emplace_back(std::make_pair(i, std::move(f)));
                 i = (i + 1) % pubKeys.size();

src/bench/bls_dkg.cpp

@@ -54,7 +54,7 @@ class DKG
     }
 
 public:
-    DKG(int quorumSize)
+    explicit DKG(int quorumSize)
     {
         members.reserve(quorumSize);
         ids.reserve(quorumSize);

src/bls/bls.h

@@ -320,7 +320,7 @@ class CBLSLazyWrapper
         bufValid = true;
     }
 
-    CBLSLazyWrapper(const CBLSLazyWrapper& r)
+    explicit CBLSLazyWrapper(const CBLSLazyWrapper& r)
     {
         *this = r;
     }

src/bls/bls_worker.cpp

@@ -154,9 +154,9 @@ struct Aggregator : public std::enable_shared_from_this<Aggregator<T>> {
                DoneCallback _doneCallback) :
             parallel(_parallel),
             workerPool(_workerPool),
-            doneCallback(std::move(_doneCallback))
+            doneCallback(std::move(_doneCallback)),
+            inputVec(std::make_shared<std::vector<const T*>>(count))
     {
-        inputVec = std::make_shared<std::vector<const T*> >(count);
         for (size_t i = 0; i < count; i++) {
             (*inputVec)[i] = pointer(_inputVec[start + i]);
         }

src/cachemap.h

@@ -123,7 +123,7 @@ class CacheMap
         if(it == mapIndex.end()) {
             return false;
         }
-        item_t& item = *(it->second);
+        const item_t& item = *(it->second);
         value = item.value;
         return true;
     }

src/ctpl_stl.h

@@ -73,7 +73,7 @@ namespace ctpl {
     public:
 
         thread_pool() { this->init(); }
-        thread_pool(int nThreads) { this->init(); this->resize(nThreads); }
+        explicit thread_pool(int nThreads) { this->init(); this->resize(nThreads); }
 
         // the destructor waits for all the functions in the queue to be finished
         ~thread_pool() {

src/dbwrapper.h

@@ -695,7 +695,7 @@ class CDBTransaction {
         Clear();
     }
 
-    bool IsClean() {
+    bool IsClean() const {
         return writes.empty() && deletes.empty();
     }
 

src/evo/deterministicmns.h

@@ -58,13 +58,13 @@ class CDeterministicMNState
 
 public:
     CDeterministicMNState() = default;
-    explicit CDeterministicMNState(const CProRegTx& proTx)
+    explicit CDeterministicMNState(const CProRegTx& proTx) :
+            keyIDOwner(proTx.keyIDOwner),
+            keyIDVoting(proTx.keyIDVoting),
+            addr(proTx.addr),
+            scriptPayout(proTx.scriptPayout)
     {
-        keyIDOwner = proTx.keyIDOwner;
         pubKeyOperator.Set(proTx.pubKeyOperator);
-        keyIDVoting = proTx.keyIDVoting;
-        addr = proTx.addr;
-        scriptPayout = proTx.scriptPayout;
     }
     template <typename Stream>
     CDeterministicMNState(deserialize_type, Stream& s)
@@ -228,7 +228,7 @@ class CDeterministicMN
 
     uint256 proTxHash;
     COutPoint collateralOutpoint;
-    uint16_t nOperatorReward;
+    uint16_t nOperatorReward{0};
     CDeterministicMNStateCPtr pdmnState;
 
     template <typename Stream, typename Operation>

src/governance/governance.cpp

@@ -592,9 +592,9 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv)
         return false;
     }
 
-    auto it = setHash->find(inv.hash);
-    if (it == setHash->end()) {
-        setHash->insert(inv.hash);
+    const auto& [_, inserted] = setHash->insert(inv.hash);
+
+    if (inserted) {
         LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added inv to requested set\n");
     }
 

src/governance/object.cpp

@@ -262,14 +262,12 @@ std::set<uint256> CGovernanceObject::RemoveInvalidVotes(const COutPoint& mnOutpo
         mapCurrentMNVotes.erase(it);
     }
 
-    if (!removedVotes.empty()) {
-        std::string removedStr;
-        for (auto& h : removedVotes) {
-            removedStr += strprintf("  %s\n", h.ToString());
-        }
-        LogPrintf("CGovernanceObject::%s -- Removed %d invalid votes for %s from MN %s:\n%s", __func__, removedVotes.size(), nParentHash.ToString(), mnOutpoint.ToString(), removedStr); /* Continued */
-        fDirtyCache = true;
+    std::string removedStr;
+    for (auto& h : removedVotes) {
+        removedStr += strprintf("  %s\n", h.ToString());
     }
+    LogPrintf("CGovernanceObject::%s -- Removed %d invalid votes for %s from MN %s:\n%s", __func__, removedVotes.size(), nParentHash.ToString(), mnOutpoint.ToString(), removedStr); /* Continued */
+    fDirtyCache = true;
 
     return removedVotes;
 }

src/hdchain.h

@@ -121,15 +121,15 @@ class CHDPubKey
 {
 private:
     static const int CURRENT_VERSION = 1;
-    int nVersion;
+    int nVersion{CHDPubKey::CURRENT_VERSION};
 
 public:
     CExtPubKey extPubKey;
     uint256 hdchainID;
-    uint32_t nAccountIndex;
-    uint32_t nChangeIndex;
+    uint32_t nAccountIndex{0};
+    uint32_t nChangeIndex{0};
 
-    CHDPubKey() : nVersion(CHDPubKey::CURRENT_VERSION), nAccountIndex(0), nChangeIndex(0) {}
+    CHDPubKey() = default;
 
     SERIALIZE_METHODS(CHDPubKey, obj)
     {

src/llmq/chainlocks.cpp

@@ -35,9 +35,9 @@ std::string CChainLockSig::ToString() const
     return strprintf("CChainLockSig(nHeight=%d, blockHash=%s)", nHeight, blockHash.ToString());
 }
 
-CChainLocksHandler::CChainLocksHandler()
+CChainLocksHandler::CChainLocksHandler() :
+    scheduler(std::make_unique<CScheduler>())
 {
-    scheduler = std::make_unique<CScheduler>();
     CScheduler::Function serviceLoop = std::bind(&CScheduler::serviceQueue, scheduler.get());
     scheduler_thread = std::make_unique<std::thread>(std::bind(&TraceThread<CScheduler::Function>, "cl-schdlr", serviceLoop));
 }

src/llmq/dkgsession.cpp

@@ -470,12 +470,10 @@ void CDKGSession::VerifyConnectionAndMinProtoVersions() const
         if (m->dmn->proTxHash == myProTxHash) {
             continue;
         }
-
-        auto it = protoMap.find(m->dmn->proTxHash);
-        if (it == protoMap.end()) {
+        if (auto it = protoMap.find(m->dmn->proTxHash); it == protoMap.end()) {
             m->badConnection = fShouldAllMembersBeConnected;
             logger.Batch("%s is not connected to us, badConnection=%b", m->dmn->proTxHash.ToString(), m->badConnection);
-        } else if (it != protoMap.end() && it->second < MIN_MASTERNODE_PROTO_VERSION) {
+        } else if (it->second < MIN_MASTERNODE_PROTO_VERSION) {
             m->badConnection = true;
             logger.Batch("%s does not have min proto version %d (has %d)", m->dmn->proTxHash.ToString(), MIN_MASTERNODE_PROTO_VERSION, it->second);
         }

src/llmq/dkgsessionmgr.cpp

@@ -22,9 +22,9 @@ static const std::string DB_SKCONTRIB = "qdkg_S";
 static const std::string DB_ENC_CONTRIB = "qdkg_E";
 
 CDKGSessionManager::CDKGSessionManager(CBLSWorker& _blsWorker, bool unitTests, bool fWipe) :
-    blsWorker(_blsWorker)
+        db(std::make_unique<CDBWrapper>(unitTests ? "" : (GetDataDir() / "llmq/dkgdb"), 1 << 20, unitTests, fWipe)),
+        blsWorker(_blsWorker)
 {
-    db = std::make_unique<CDBWrapper>(unitTests ? "" : (GetDataDir() / "llmq/dkgdb"), 1 << 20, unitTests, fWipe);
     MigrateDKG();
 
     for (const auto& qt : Params().GetConsensus().llmqs) {

src/llmq/instantsend.cpp

@@ -54,10 +54,9 @@ uint256 CInstantSendLock::GetRequestId() const
 
 ////////////////
 
-CInstantSendDb::CInstantSendDb(bool unitTests, bool fWipe)
-{
-    db = std::make_unique<CDBWrapper>(unitTests ? "" : (GetDataDir() / "llmq/isdb"), 32 << 20, unitTests, fWipe);
-}
+CInstantSendDb::CInstantSendDb(bool unitTests, bool fWipe) :
+        db(std::make_unique<CDBWrapper>(unitTests ? "" : (GetDataDir() / "llmq/isdb"), 32 << 20, unitTests, fWipe))
+{}
 
 void CInstantSendDb::Upgrade()
 {

src/llmq/quorums.cpp

@@ -624,8 +624,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand,
     }
 
     if (strCommand == NetMsgType::QDATA) {
-        auto verifiedProRegTxHash = pFrom->GetVerifiedProRegTxHash();
-        if ((!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) || pFrom == nullptr || (verifiedProRegTxHash.IsNull() && !pFrom->qwatch)) {
+        if ((!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) || pFrom == nullptr || (pFrom->GetVerifiedProRegTxHash().IsNull() && !pFrom->qwatch)) {
             errorHandler("Not a verified masternode or a qwatch connection");
             return;
         }
@@ -635,7 +634,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand,
 
         {
             LOCK2(cs_main, cs_data_requests);
-            auto it = mapQuorumDataRequests.find(std::make_pair(verifiedProRegTxHash, true));
+            auto it = mapQuorumDataRequests.find(std::make_pair(pFrom->GetVerifiedProRegTxHash(), true));
             if (it == mapQuorumDataRequests.end()) {
                 errorHandler("Not requested");
                 return;

src/llmq/quorums.h

@@ -56,14 +56,14 @@ class CQuorumDataRequest
     };
 
 private:
-    Consensus::LLMQType llmqType;
+    Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE};
     uint256 quorumHash;
-    uint16_t nDataMask;
+    uint16_t nDataMask{0};
     uint256 proTxHash;
-    Errors nError;
+    Errors nError{UNDEFINED};
 
-    int64_t nTime;
-    bool fProcessed;
+    int64_t nTime{GetTime()};
+    bool fProcessed{false};
 
     static constexpr int64_t EXPIRATION_TIMEOUT{300};
 
@@ -74,10 +74,8 @@ class CQuorumDataRequest
         llmqType(llmqTypeIn),
         quorumHash(quorumHashIn),
         nDataMask(nDataMaskIn),
-        proTxHash(proTxHashIn),
-        nError(UNDEFINED),
-        nTime(GetTime()),
-        fProcessed(false) {}
+        proTxHash(proTxHashIn)
+        {}
 
     SERIALIZE_METHODS(CQuorumDataRequest, obj)
     {
@@ -153,7 +151,7 @@ class CQuorum
 public:
     const Consensus::LLMQParams& params;
     CFinalCommitmentPtr qc;
-    const CBlockIndex* m_quorum_base_block_index;
+    const CBlockIndex* m_quorum_base_block_index{nullptr};
     uint256 minedBlockHash;
     std::vector<CDeterministicMNCPtr> members;
 

src/llmq/signing.cpp

@@ -38,9 +38,9 @@ UniValue CRecoveredSig::ToJson() const
     return ret;
 }
 
-CRecoveredSigsDb::CRecoveredSigsDb(bool fMemory, bool fWipe)
+CRecoveredSigsDb::CRecoveredSigsDb(bool fMemory, bool fWipe) :
+        db(std::make_unique<CDBWrapper>(fMemory ? "" : (GetDataDir() / "llmq/recsigdb"), 8 << 20, fMemory, fWipe))
 {
-    db = std::make_unique<CDBWrapper>(fMemory ? "" : (GetDataDir() / "llmq/recsigdb"), 8 << 20, fMemory, fWipe);
     MigrateRecoveredSigs();
 }
 

src/masternode/sync.cpp

@@ -16,10 +16,10 @@
 class CMasternodeSync;
 CMasternodeSync masternodeSync;
 
-CMasternodeSync::CMasternodeSync()
+CMasternodeSync::CMasternodeSync() :
+    nTimeAssetSyncStarted(GetTime()),
+    nTimeLastBumped(GetTime())
 {
-    nTimeAssetSyncStarted = GetTime();
-    nTimeLastBumped = GetTime();
 }
 
 void CMasternodeSync::Reset(bool fForce, bool fNotifyReset)

src/netfulfilledman.cpp

@@ -27,17 +27,6 @@ bool CNetFulfilledRequestManager::HasFulfilledRequest(const CService& addr, cons
             it->second[strRequest] > GetTime();
 }
 
-void CNetFulfilledRequestManager::RemoveFulfilledRequest(const CService& addr, const std::string& strRequest)
-{
-    LOCK(cs_mapFulfilledRequests);
-    CService addrSquashed = Params().AllowMultiplePorts() ? addr : CService(addr, 0);
-    fulfilledreqmap_t::iterator it = mapFulfilledRequests.find(addrSquashed);
-
-    if (it != mapFulfilledRequests.end()) {
-        it->second.erase(strRequest);
-    }
-}
-
 void CNetFulfilledRequestManager::RemoveAllFulfilledRequests(const CService& addr)
 {
     LOCK(cs_mapFulfilledRequests);

src/netfulfilledman.h

@@ -24,8 +24,6 @@ class CNetFulfilledRequestManager
     fulfilledreqmap_t mapFulfilledRequests;
     mutable CCriticalSection cs_mapFulfilledRequests;
 
-    void RemoveFulfilledRequest(const CService& addr, const std::string& strRequest);
-
 public:
     CNetFulfilledRequestManager() {}
 

src/qt/governancelist.h

@@ -35,7 +35,7 @@ class Proposal : public QObject
     QString m_url;
 
 public:
-    Proposal(const CGovernanceObject* p, QObject* parent = nullptr);
+    explicit Proposal(const CGovernanceObject* p, QObject* parent = nullptr);
     QString title() const;
     QString hash() const;
     QDateTime startDate() const;

src/stacktraces.cpp

@@ -702,18 +702,18 @@ crash_info GetCrashInfoFromException(const std::exception_ptr& e)
     try {
         // rethrow and catch the exception as there is no other way to reliably cast to the real type (not possible with RTTI)
         std::rethrow_exception(e);
-    } catch (const std::exception& e) {
+    } catch (const std::exception& e2) {
         type = getExceptionType();
-        what = GetExceptionWhat(e);
-    } catch (const std::string& e) {
+        what = GetExceptionWhat(e2);
+    } catch (const std::string& e2) {
         type = getExceptionType();
-        what = GetExceptionWhat(e);
-    } catch (const char* e) {
+        what = GetExceptionWhat(e2);
+    } catch (const char* e2) {
         type = getExceptionType();
-        what = GetExceptionWhat(e);
-    } catch (int e) {
+        what = GetExceptionWhat(e2);
+    } catch (int e2) {
         type = getExceptionType();
-        what = GetExceptionWhat(e);
+        what = GetExceptionWhat(e2);
     } catch (...) {
         type = getExceptionType();
         what = "<unknown>";
@@ -740,7 +740,6 @@ static void terminate_handler()
     auto exc = std::current_exception();
 
     crash_info ci;
-    ci.crashDescription = "std::terminate() called, aborting";
 
     if (exc) {
         auto ci2 = GetCrashInfoFromException(exc);
@@ -750,10 +749,9 @@ static void terminate_handler()
     } else {
         ci.crashDescription = "std::terminate() called due unknown reason";
         ci.stackframes = GetStackFrames(0, 16);
+        ci.stackframeInfos = GetStackFrameInfos(ci.stackframes);
     }
 
-    ci.stackframeInfos = GetStackFrameInfos(ci.stackframes);
-
     PrintCrashInfo(ci);
 
     skipAbortSignal = true;