diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-06-04 14:44:42 -0400 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-06-04 14:45:32 -0400 |
commit | 01b45b2e016f0b0907929e818216edf7157fb03a (patch) | |
tree | 2c42bedb714ad45b398626f9d944cdffcb7b8930 /src/test | |
parent | 39afe5b1c68c5979b2ef2f03b60ec6a57901328f (diff) | |
parent | 8b3136bd307123a255b9166aa42a497a44bcce70 (diff) | |
download | bitcoin-01b45b2e016f0b0907929e818216edf7157fb03a.tar.xz |
Merge #19053: refactor: replace CNode pointers by references within net_processing.{h,cpp}
8b3136bd307123a255b9166aa42a497a44bcce70 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 8b3136bd307123a255b9166aa42a497a44bcce70 🔻
practicalswift:
ACK 8b3136bd307123a255b9166aa42a497a44bcce70
Tree-SHA512: 15b6a569ecdcb39341002b9f4e09b38ed4df077e3a3a50dfb1b72d98bdc9f9769c7c504f106456aa7748af8591af7bb836b72d46086df715ab116e4ac3224b3b
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/fuzz/process_message.cpp | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index c51ee3cf29..211a84b5f2 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -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(); |