From a298eb2b934044edef11ba6118c688e2dec940e2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 7 Jan 2021 09:03:35 +0100 Subject: [PATCH] Merge #20584: Declare de facto const reference variables/member functions as const 31b136e5802e1b1e5f9a9589736afe0652f34da2 Don't declare de facto const reference variables as non-const (practicalswift) 1c65c075ee4c7f98d9c1fac5ed7576b96374d4e9 Don't declare de facto const member functions as non-const (practicalswift) Pull request description: _Meta: This is the second and final part of the `const` refactoring series (part one: #20581). **I promise: no more refactoring PRs from me in a while! :)** I'll now go back to focusing on fuzzing/hardening!_ Changes in this PR: * Don't declare de facto const member functions as non-const * Don't declare de facto const reference variables as non-const Awards for finding candidates for the above changes go to: * `clang-tidy`'s [`readability-make-member-function-const`](https://clang.llvm.org/extra/clang-tidy/checks/readability-make-member-function-const.html) check ([list of `clang-tidy` checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html)) * `cppcheck`'s `constVariable` check ([list of `cppcheck` checks](https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/)) See #18920 for instructions on how to analyse Bitcoin Core using Clang Static Analysis, `clang-tidy` and `cppcheck`. ACKs for top commit: ajtowns: ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2 jonatack: ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2 theStack: ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2 :snowflake: Tree-SHA512: f58f8f00744219426874379e9f3e9331132b9b48e954d24f3a85cbb858fdcc98009ed42ef7e7b4619ae8af9fc240a6d8bfc1c438db2e97b0ecd722a80dcfeffe --- src/addrman.cpp | 2 +- src/logging.h | 4 ++-- src/miner.cpp | 2 +- src/miner.h | 2 +- src/net.cpp | 2 +- src/net.h | 2 +- src/rpc/blockchain.cpp | 2 +- src/script/interpreter.cpp | 4 ++-- src/script/sign.cpp | 2 +- src/test/checkqueue_tests.cpp | 8 ++++---- src/test/util/setup_common.cpp | 4 ++-- src/test/util/setup_common.h | 4 ++-- src/validation.cpp | 2 +- src/wallet/wallet.cpp | 4 ++-- 14 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 50ec471981..b850488869 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -725,7 +725,7 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return CAddrInfo(); } - CAddrInfo& newInfo = mapInfo[id_new]; + const CAddrInfo& newInfo = mapInfo[id_new]; // which tried bucket to move the entry to int tried_bucket = newInfo.GetTriedBucket(nKey, m_asmap); diff --git a/src/logging.h b/src/logging.h index 657fe54cf3..7810e318ce 100644 --- a/src/logging.h +++ b/src/logging.h @@ -158,9 +158,9 @@ namespace BCLog { bool WillLogCategory(LogFlags category) const; /** Returns a vector of the log categories */ - std::vector LogCategoriesList(); + std::vector LogCategoriesList() const; /** Returns a string with the log categories */ - std::string LogCategoriesString() + std::string LogCategoriesString() const { return Join(LogCategoriesList(), ", ", [&](const LogCategory& i) { return i.category; }); }; diff --git a/src/miner.cpp b/src/miner.cpp index 47c6de81df..059c0781d7 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -290,7 +290,7 @@ bool BlockAssembler::TestPackage(uint64_t packageSize, unsigned int packageSigOp // Perform transaction-level checks before adding to block: // - transaction finality (locktime) // - safe TXs in regard to ChainLocks -bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& package) +bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& package) const { for (CTxMemPool::txiter it : package) { if (!IsFinalTx(it->GetTx(), nHeight, nLockTimeCutoff)) diff --git a/src/miner.h b/src/miner.h index e321c13278..18a3c022c0 100644 --- a/src/miner.h +++ b/src/miner.h @@ -205,7 +205,7 @@ private: * locktime * These checks should always succeed, and they're here * only as an extra check in case of suboptimal node configuration */ - bool TestPackageTransactions(const CTxMemPool::setEntries& package); + bool TestPackageTransactions(const CTxMemPool::setEntries& package) const; /** Return true if given transaction from mapTx has already been evaluated, * or if the transaction's cached data in mapTx is incorrect. */ bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set& mapModifiedTx, CTxMemPool::setEntries& failedTx) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs); diff --git a/src/net.cpp b/src/net.cpp index ed0154e8c3..ecd96534e0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1441,7 +1441,7 @@ void CConnman::CalculateNumConnectionsChangedStats() statsClient.gauge("peers.torConnections", torNodes, 1.0f); } -void CConnman::InactivityCheck(CNode *pnode) +void CConnman::InactivityCheck(CNode *pnode) const { int64_t nTime = GetSystemTimeInSeconds(); if (nTime - pnode->nTimeConnected > m_peer_connect_timeout) diff --git a/src/net.h b/src/net.h index b7be35ae20..08716e5a4a 100644 --- a/src/net.h +++ b/src/net.h @@ -532,7 +532,7 @@ private: void DisconnectNodes(); void NotifyNumConnectionsChanged(); void CalculateNumConnectionsChangedStats(); - void InactivityCheck(CNode *pnode); + void InactivityCheck(CNode *pnode) const; bool GenerateSelectSet(std::set &recv_set, std::set &send_set, std::set &error_set); #ifdef USE_KQUEUE void SocketEventsKqueue(std::set &recv_set, std::set &send_set, std::set &error_set, bool fOnlyPoll); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8416a1655c..f18a588d32 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -68,7 +68,7 @@ extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, CTxMemPool NodeContext& EnsureAnyNodeContext(const CoreContext& context) { - auto* node_context = GetContext(context); + auto* const node_context = GetContext(context); if (!node_context) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Node context not found"); } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index d48debdb33..abcd1b7e91 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -301,8 +301,8 @@ private: uint32_t m_first_false_pos = NO_FALSE; public: - bool empty() { return m_stack_size == 0; } - bool all_true() { return m_first_false_pos == NO_FALSE; } + bool empty() const { return m_stack_size == 0; } + bool all_true() const { return m_first_false_pos == NO_FALSE; } void push_back(bool f) { if (m_first_false_pos == NO_FALSE && !f) { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 063c560660..89bf363c65 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -327,7 +327,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType) { assert(nIn < txTo.vin.size()); - CTxIn& txin = txTo.vin[nIn]; + const CTxIn& txin = txTo.vin[nIn]; assert(txin.prevout.n < txFrom.vout.size()); const CTxOut& txout = txFrom.vout[txin.prevout.n]; diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index bce35b3d7b..bb34715832 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -24,7 +24,7 @@ static const unsigned int QUEUE_BATCH_SIZE = 128; static const int SCRIPT_CHECK_THREADS = 3; struct FakeCheck { - bool operator()() + bool operator()() const { return true; } @@ -45,7 +45,7 @@ struct FailingCheck { bool fails; FailingCheck(bool _fails) : fails(_fails){}; FailingCheck() : fails(true){}; - bool operator()() + bool operator()() const { return !fails; } @@ -74,7 +74,7 @@ struct UniqueCheck { struct MemoryCheck { static std::atomic fake_allocated_memory; bool b {false}; - bool operator()() + bool operator()() const { return true; } @@ -105,7 +105,7 @@ struct FrozenCleanupCheck { // Freezing can't be the default initialized behavior given how the queue // swaps in default initialized Checks. bool should_freeze {false}; - bool operator()() + bool operator()() const { return true; } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 11a0defcff..96eca1f50e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -430,12 +430,12 @@ TestChainSetup::~TestChainSetup() SetMockTime(0); } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const { return FromTx(MakeTransactionRef(tx)); } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { return CTxMemPoolEntry(tx, nFee, nTime, nHeight, spendsCoinbase, sigOpCount, lp); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 14b49ea2e8..bf5ebb2b73 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -181,8 +181,8 @@ struct TestMemPoolEntryHelper nFee(0), nTime(0), nHeight(1), spendsCoinbase(false), sigOpCount(1) { } - CTxMemPoolEntry FromTx(const CMutableTransaction& tx); - CTxMemPoolEntry FromTx(const CTransactionRef& tx); + CTxMemPoolEntry FromTx(const CMutableTransaction& tx) const; + CTxMemPoolEntry FromTx(const CTransactionRef& tx) const; // Change the default value TestMemPoolEntryHelper &Fee(CAmount _fee) { nFee = _fee; return *this; } diff --git a/src/validation.cpp b/src/validation.cpp index be691495ed..54621f3c93 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -717,7 +717,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) m_view.SetBackend(m_viewmempool); assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(m_active_chainstate.CoinsTip())); - CCoinsViewCache& coins_cache = m_active_chainstate.CoinsTip(); + const CCoinsViewCache& coins_cache = m_active_chainstate.CoinsTip(); // do all inputs exist? for (const CTxIn& txin : tx.vin) { if (!coins_cache.HaveCoinInCache(txin.prevout)) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 74a4080191..de5230bc06 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -592,7 +592,7 @@ void CWallet::AddToSpends(const uint256& wtxid) { auto it = mapWallet.find(wtxid); assert(it != mapWallet.end()); - CWalletTx& thisTx = it->second; + const CWalletTx& thisTx = it->second; if (thisTx.IsCoinBase()) // Coinbases don't spend anything! return; @@ -1093,7 +1093,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) // Can't mark abandoned if confirmed or in mempool auto it = mapWallet.find(hashTx); assert(it != mapWallet.end()); - CWalletTx& origtx = it->second; + const CWalletTx& origtx = it->second; if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool() || origtx.IsLockedByInstantSend()) { return false; }