Avoid dereference-of-casted-pointer #10760

pull sipa wants to merge 1 commits into bitcoin:master from sipa:20170707_avoidcastptr changing 8 files +26 −18

sipa commented at 7:55 AM on July 7, 2017: member

And prefer a static_cast to the intended reference type.
fanquake added the label Refactoring on Jul 7, 2017

in src/core_memusage.h:13 in 8be4385d64 outdated

   9 | @@ -10,7 +10,7 @@
  10 |  #include "memusage.h"
  11 |  
  12 |  static inline size_t RecursiveDynamicUsage(const CScript& script) {
  13 | -    return memusage::DynamicUsage(*static_cast<const CScriptBase*>(&script));
  14 | +    return memusage::DynamicUsage(static_cast<const CScriptBase&>(script));

promag commented at 8:45 AM on July 7, 2017:

Remove static cast?

  return memusage::DynamicUsage(script);

In my system it builds fine.

sipa commented at 5:47 PM on July 7, 2017:

Nice catch, fixed.

in src/wallet/walletdb.cpp:97 in 8be4385d64 outdated

  93 | @@ -94,23 +94,23 @@ bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey)
  94 |  
  95 |  bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript)
  96 |  {
  97 | -    return WriteIC(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false);
  98 | +    return WriteIC(std::make_pair(std::string("cscript"), hash), static_cast<const CScriptBase&>(redeemScript), false);

promag commented at 9:00 AM on July 7, 2017:

Removing the cast, shouldn't the compiler resolve to:

template<typename Stream, unsigned int N, typename T>
inline void Serialize(Stream& os, const prevector<N, T>& v)

If not I think we could implement CScript::Serialize to call super and drop all of these casts.

sipa commented at 5:47 PM on July 7, 2017:

I somehow remembered that this didn't work, but it seems I was wrong. Fixed.

practicalswift commented at 10:28 AM on July 7, 2017: contributor

Concept ACK 8be4385d64c5a50c63834b75c06d1b27d66ddb62

Regarding the static_cast preference, see also PR #10498: "Use static_cast instead of C-style casts for non-fundamental types".
Avoid dereference-of-casted-pointer 0aadc11fd8
sipa force-pushed on Jul 7, 2017
promag commented at 5:51 PM on July 7, 2017: member

tACK.
gmaxwell approved
gmaxwell commented at 11:32 AM on July 9, 2017: contributor

utACK
TheBlueMatt commented at 12:08 AM on July 10, 2017: member

utACK 0aadc11fd88b298c7af2dfb69763b2c67dc6b7b0
dcousens approved
laanwj commented at 2:24 PM on July 10, 2017: member

And prefer a static_cast to the intended reference type.

What is the rationale here? Just code cleanup?
sipa commented at 4:06 PM on July 10, 2017: member

Yes, cleanup. The existing practice risks hiding some bugs.
jonasschnelli commented at 7:35 AM on July 13, 2017: contributor

utACK 0aadc11fd88b298c7af2dfb69763b2c67dc6b7b0
sipa merged this on Jul 15, 2017
sipa closed this on Jul 15, 2017
sipa referenced this in commit 10b22e3141 on Jul 15, 2017
PastaPastaPasta referenced this in commit 3f64f63401 on Jul 6, 2019
PastaPastaPasta referenced this in commit 69e3fe3493 on Jul 8, 2019
PastaPastaPasta referenced this in commit 5594694346 on Jul 9, 2019
PastaPastaPasta referenced this in commit b19c08f8c3 on Jul 11, 2019
PastaPastaPasta referenced this in commit 41b42b20a6 on Jul 13, 2019
PastaPastaPasta referenced this in commit 44f1cac512 on Jul 13, 2019
PastaPastaPasta referenced this in commit acf4ce56ed on Jul 13, 2019
PastaPastaPasta referenced this in commit d593e4cafe on Jul 17, 2019
PastaPastaPasta referenced this in commit bd5e509732 on Jul 17, 2019
PastaPastaPasta referenced this in commit 985ec98e00 on Jul 23, 2019
PastaPastaPasta referenced this in commit c1fe4d6c0a on Jul 23, 2019
PastaPastaPasta referenced this in commit dc97d48bbb on Jul 24, 2019
PastaPastaPasta referenced this in commit e4c41e74aa on Jul 24, 2019
barrystyle referenced this in commit 1e35ca764d on Jan 22, 2020
barrystyle referenced this in commit af3cc1d9d8 on Jan 22, 2020
furszy referenced this in commit 4903df0833 on Feb 6, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels