Investigate unnecessary uses of raw pointer parameters #19062
Description
The developer note's recommendations on how to pass (non-)fundamental types can be found here.
In #19053 @theStack replaces some unnecessary uses of raw pointer parameters with references.
There are a few similar cases in our code base where references (or something else) might be more appropriate than raw pointer parameters.
If anyone is interested in investigating such cases the commands below might be helpful to find potential candidates.
Note: It probably goes without saying, but any change of this type needs case-by-case evaluation :) Each suggested individual parameter change must make sense and be worth doing: the commands are only provided to help find potential candidates.
Top list of types passed as raw pointer parameters (incomplete top list, but it should be somewhat representative):
$ git grep -E '^[a-zA-Z].* [a-zA-Z:]+\([^()]*\*[^()]*\)' -- "src/**.cpp" "src/**.h" \
":(exclude)src/bench/" ":(exclude)src/compat/" ":(exclude)src/crc32c/" \
":(exclude)src/leveldb/" ":(exclude)src/qt/" ":(exclude)src/secp256k1/" \
":(exclude)src/test/" ":(exclude)src/tinyformat.h" ":(exclude)src/univalue/" \
":(exclude)src/zmq/" ":(exclude)src/wallet/" | grep -vE "char *\*" | \
cut -f2 -d'(' | cut -f1 -d')' | tr "," "\n" | grep "\*" | sed 's/^ *const *//g' | \
sed 's/^ *//g' | sed 's/ \*/* /g' | sed 's/\*.*$/*/g' | grep -E '[a-zA-Z]' | \
sort | uniq -c | sort -rn
118 CBlockIndex*
24 CNode*
11 ScriptError*
10 void*
9 FILE*
9 bool*
8 CConnman*
7 std::string*
6 RPCTimerInterface*
6 CNetAddr*
4 std::vector<int>*
4 SigningProvider*
4 LockPoints*
4 HTTPRequest*
4 FlatFilePos*
4 CValidationInterface*
4 CScriptWitness*
4 CBlockHeader*
3 uint32_t*
3 struct sockaddr*
3 EstimationResult*
3 CNodeState*
3 CCoinsView*
2 uint8_t*
2 uint64_t*
2 struct bufferevent*
2 std::vector<unsigned char>*
2 std::vector<CScriptCheck>*
2 std::vector<const CBlockIndex*
2 SignatureData*
2 PrecomputedTransactionData*
2 int64_t*
2 int*
2 FillableSigningProvider*
2 double*
2 CRPCCommand*
2 CCoinsViewCache*
2 CBlock*
1 WorkQueue<HTTPClosure>*
1 struct timeval*
1 struct in_addr*
1 struct evhttp*
1 struct event_base*
1 std::vector<COutPoint>*
1 std::list<QueuedBlock>::iterator*
1 socklen_t*
1 secp256k1_ecdsa_signature*
1 leveldb::Options*
1 FeeCalculation*
1 DisconnectedBlockTransactions*
1 CTxMemPoolEntry*
1 Coin*
1 CCoinsViewCursor*
1 CChainState*
1 CBlockFileInfo*
1 BaseRequestHandler*
1 BanMan*
A subset of functions with raw pointer parameters:
$ git grep -E '^[a-zA-Z].* [a-zA-Z:]+\([^()]*\*[^()]*\)' -- "src/**.cpp" "src/**.h" \
":(exclude)src/bench/" ":(exclude)src/compat/" ":(exclude)src/crc32c/" \
":(exclude)src/leveldb/" ":(exclude)src/qt/" ":(exclude)src/secp256k1/" \
":(exclude)src/test/" ":(exclude)src/tinyformat.h" ":(exclude)src/univalue/" \
":(exclude)src/zmq/" ":(exclude)src/wallet/" | grep -vE "char *\*"
src/addrman.cpp:CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
src/addrman.cpp:CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId)
src/bitcoin-cli.cpp:static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args)
src/blockfilter.cpp:bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
src/chain.cpp:void CChain::SetTip(CBlockIndex *pindex) {
src/chain.cpp:CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const {
src/chain.cpp:const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb) {
src/chain.h:const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb);
src/coins.cpp:bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
src/consensus/merkle.cpp:uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated) {
src/consensus/merkle.cpp:uint256 BlockMerkleRoot(const CBlock& block, bool* mutated)
src/consensus/merkle.cpp:uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
src/consensus/merkle.h:uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated = nullptr);
src/consensus/merkle.h:uint256 BlockMerkleRoot(const CBlock& block, bool* mutated = nullptr);
src/consensus/merkle.h:uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr);
src/consensus/tx_verify.cpp:std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
src/consensus/tx_verify.cpp:bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
src/consensus/tx_verify.h:std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
src/consensus/tx_verify.h:bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
src/crypto/ripemd160.cpp:void inline Initialize(uint32_t* s)
…