Investigate unnecessary uses of raw pointer parameters #19062

issue practicalswift openend 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):

     0$ git grep -E '^[a-zA-Z].* [a-zA-Z:]+\([^()]*\*[^()]*\)' -- "src/**.cpp" "src/**.h" \
     1      ":(exclude)src/bench/" ":(exclude)src/compat/" ":(exclude)src/crc32c/" \
     2      ":(exclude)src/leveldb/" ":(exclude)src/qt/" ":(exclude)src/secp256k1/" \
     3      ":(exclude)src/test/" ":(exclude)src/tinyformat.h" ":(exclude)src/univalue/" \
     4      ":(exclude)src/zmq/" ":(exclude)src/wallet/" | grep -vE "char *\*" | \
     5      cut -f2 -d'(' | cut -f1 -d')' | tr "," "\n" | grep "\*" | sed 's/^ *const  *//g' | \
     6      sed 's/^ *//g' | sed 's/ \*/* /g' | sed 's/\*.*$/*/g' | grep -E '[a-zA-Z]' | \
     7      sort | uniq -c | sort -rn
     8    118 CBlockIndex*
     9     24 CNode*
    10     11 ScriptError*
    11     10 void*
    12      9 FILE*
    13      9 bool*
    14      8 CConnman*
    15      7 std::string*
    16      6 RPCTimerInterface*
    17      6 CNetAddr*
    18      4 std::vector<int>*
    19      4 SigningProvider*
    20      4 LockPoints*
    21      4 HTTPRequest*
    22      4 FlatFilePos*
    23      4 CValidationInterface*
    24      4 CScriptWitness*
    25      4 CBlockHeader*
    26      3 uint32_t*
    27      3 struct sockaddr*
    28      3 EstimationResult*
    29      3 CNodeState*
    30      3 CCoinsView*
    31      2 uint8_t*
    32      2 uint64_t*
    33      2 struct bufferevent*
    34      2 std::vector<unsigned char>*
    35      2 std::vector<CScriptCheck>*
    36      2 std::vector<const CBlockIndex*
    37      2 SignatureData*
    38      2 PrecomputedTransactionData*
    39      2 int64_t*
    40      2 int*
    41      2 FillableSigningProvider*
    42      2 double*
    43      2 CRPCCommand*
    44      2 CCoinsViewCache*
    45      2 CBlock*
    46      1 WorkQueue<HTTPClosure>*
    47      1 struct timeval*
    48      1 struct in_addr*
    49      1 struct evhttp*
    50      1 struct event_base*
    51      1 std::vector<COutPoint>*
    52      1 std::list<QueuedBlock>::iterator*
    53      1 socklen_t*
    54      1 secp256k1_ecdsa_signature*
    55      1 leveldb::Options*
    56      1 FeeCalculation*
    57      1 DisconnectedBlockTransactions*
    58      1 CTxMemPoolEntry*
    59      1 Coin*
    60      1 CCoinsViewCursor*
    61      1 CChainState*
    62      1 CBlockFileInfo*
    63      1 BaseRequestHandler*
    64      1 BanMan*
    

    A subset of functions with raw pointer parameters:

     0$ git grep -E '^[a-zA-Z].* [a-zA-Z:]+\([^()]*\*[^()]*\)' -- "src/**.cpp" "src/**.h" \
     1      ":(exclude)src/bench/" ":(exclude)src/compat/" ":(exclude)src/crc32c/" \
     2      ":(exclude)src/leveldb/" ":(exclude)src/qt/" ":(exclude)src/secp256k1/" \
     3      ":(exclude)src/test/" ":(exclude)src/tinyformat.h" ":(exclude)src/univalue/" \
     4      ":(exclude)src/zmq/" ":(exclude)src/wallet/" | grep -vE "char *\*"
     5src/addrman.cpp:CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
     6src/addrman.cpp:CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId)
     7src/bitcoin-cli.cpp:static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args)
     8src/blockfilter.cpp:bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
     9src/chain.cpp:void CChain::SetTip(CBlockIndex *pindex) {
    10src/chain.cpp:CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const {
    11src/chain.cpp:const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb) {
    12src/chain.h:const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb);
    13src/coins.cpp:bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
    14src/consensus/merkle.cpp:uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated) {
    15src/consensus/merkle.cpp:uint256 BlockMerkleRoot(const CBlock& block, bool* mutated)
    16src/consensus/merkle.cpp:uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
    17src/consensus/merkle.h:uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated = nullptr);
    18src/consensus/merkle.h:uint256 BlockMerkleRoot(const CBlock& block, bool* mutated = nullptr);
    19src/consensus/merkle.h:uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr);
    20src/consensus/tx_verify.cpp:std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
    21src/consensus/tx_verify.cpp:bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
    22src/consensus/tx_verify.h:std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
    23src/consensus/tx_verify.h:bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
    24src/crypto/ripemd160.cpp:void inline Initialize(uint32_t* s)
    25
    
  2. MarcoFalke added the label Brainstorming on May 24, 2020
  3. MarcoFalke referenced this in commit a79bca2f1f on Jun 8, 2020
  4. sidhujag referenced this in commit b8f1025f30 on Jun 8, 2020
  5. klementtan commented at 1:00 pm on June 20, 2021: contributor
    #22282 fixes CheckFinalTx pass by pointer to pass by reference
  6. practicalswift closed this on Jul 7, 2021

  7. DrahtBot 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: 2024-11-17 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me