Investigate unnecessary uses of raw pointer parameters #19062

issue practicalswift opened this issue on May 24, 2020
  1. practicalswift commented at 12:27 PM on May 24, 2020: contributor

    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)
    …
    
  2. MarcoFalke added the label Brainstorming on May 24, 2020
  3. theStack cross-referenced this on May 26, 2020 from issue refactor: replace pointers by references within tx_verify.{h,cpp} by theStack
  4. MarcoFalke referenced this in commit a79bca2f1f on Jun 8, 2020
  5. sidhujag referenced this in commit b8f1025f30 on Jun 8, 2020
  6. practicalswift cross-referenced this on Jun 11, 2021 from issue refactor: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested by MarcoFalke
  7. practicalswift cross-referenced this on Jun 20, 2021 from issue refactor: CheckFinalTx pass by reference instead of pointer by klementtan
  8. klementtan commented at 1:00 PM on June 20, 2021: contributor

    #22282 fixes CheckFinalTx pass by pointer to pass by reference

  9. practicalswift closed this on Jul 7, 2021

  10. bitcoin locked this on Aug 18, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC