Merge #19053: refactor: replace CNode pointers by references within net_processing.{h,cpp}

8b3136bd30 refactor: replace CNode pointers by references within net_processing.{h,cpp} (Sebastian Falbesoner)

Pull request description:

  This PR is inspired by a [recent code review comment](https://github.com/bitcoin/bitcoin/pull/19010#discussion_r426954791) on a PR that introduced new functions to the net_processing module. The point of the discussion was basically that whenever we pass something not by value (in the concrete example it was about `CNode*` and `CConnman*`) we should either use
  * a pointer (```CType*```) with null pointer check or
  * a reference (```CType&```)

  To keep things simple, this PR for a first approach
  * only tackles `CNode*` pointers
  * only within the net_processing module, i.e. no changes that would need adaption in other modules
  * keeps the names of the variables as they are

  I'm aware that PRs like this are kind of a PITA to review, but I think the code quality would increase if we get rid of pointers without nullptr check -- bloating up the code by adding all the missing checks would be the worse alternative, in my opinion.

  Possible follow-up PRs, in case this is received well:
  * replace CNode pointers by references for net module
  * replace CConnman pointers by references for net_processing module
  * ...

ACKs for top commit:
  MarcoFalke:
    ACK 8b3136bd30 🔻
  practicalswift:
    ACK 8b3136bd30

Tree-SHA512: 15b6a569ecdcb39341002b9f4e09b38ed4df077e3a3a50dfb1b72d98bdc9f9769c7c504f106456aa7748af8591af7bb836b72d46086df715ab116e4ac3224b3b
This commit is contained in:
MarcoFalke 2020-06-04 14:44:42 -04:00
commit 01b45b2e01
No known key found for this signature in database
GPG Key ID: CE2B75697E69A548
3 changed files with 334 additions and 334 deletions

File diff suppressed because it is too large Load Diff

View File

@ -31,7 +31,7 @@ private:
ChainstateManager& m_chainman;
CTxMemPool& m_mempool;
bool CheckIfBanned(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool CheckIfBanned(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
public:
PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
@ -74,7 +74,7 @@ public:
bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing);
/** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
void ConsiderEviction(CNode *pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
void ConsiderEviction(CNode& pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound */
void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams);
/** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */

View File

@ -30,7 +30,7 @@
#include <string>
#include <vector>
bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc);
bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc);
namespace {
@ -77,7 +77,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
connman.AddTestNode(p2p_node);
g_setup->m_node.peer_logic->InitializeNode(&p2p_node);
try {
(void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false});
(void)ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false});
} catch (const std::ios_base::failure&) {
}
SyncWithValidationInterfaceQueue();