Use static_cast instead of C-style casts for non-fundamental types #10498

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:static_cast changing 24 files +44 −44
  1. practicalswift commented at 12:35 PM on June 1, 2017: contributor

    A C-style cast is equivalent to try casting in the following order:

    1. const_cast(...)
    2. static_cast(...)
    3. const_cast(static_cast(...))
    4. reinterpret_cast(...)
    5. const_cast(reinterpret_cast(...))

    By using static_cast<T>(...) explicitly we avoid the possibility of an unintentional and dangerous reinterpret_cast. Furthermore static_cast<T>(...) allows for easier grepping of casts.

    For a more thorough discussion, see "ES.49: If you must use a cast, use a named cast" in the C++ Core Guidelines (Stroustrup & Sutter).

  2. fanquake added the label Refactoring on Jun 1, 2017
  3. in src/net_processing.cpp:2632 in 1a3c1e24cb outdated 
    2628 | @@ -2629,7 +2629,7 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman& connman)
2629 |      CNodeState &state = *State(pnode->GetId());
2630 |  
2631 |      BOOST_FOREACH(const CBlockReject& reject, state.rejects) {
2632 | -        connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, (std::string)NetMsgType::BLOCK, reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
2633 | +        connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, static_cast<std::string>(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
    benma commented at 12:53 PM on June 1, 2017:

    maybe std::string(NetMsgType::BLOCK) is nicer here.

  4. in src/primitives/block.h:89 in 1a3c1e24cb outdated 
      85 | @@ -86,14 +86,14 @@ class CBlock : public CBlockHeader
  86 |      CBlock(const CBlockHeader &header)
  87 |      {
  88 |          SetNull();
  89 | -        *((CBlockHeader*)this) = header;
  90 | +        *(static_cast<CBlockHeader*>(this)) = header;
    benma commented at 12:58 PM on June 1, 2017:

    Why doesn't this need a cont_cast? this is const, isn't it?

    practicalswift commented at 9:12 PM on June 4, 2017:

    @benma this isn't const in the constructor:

    The type of this in a member function of class X is X* (pointer to X). If the member function is cv-qualified, the type of this is cv X* (pointer to identically cv-qualified X). Since constructors and destructors cannot be cv-qualified, the type of this in them is always X*, even when constructing or destroying a const object. (cppreference.com)

    benma commented at 9:20 PM on June 4, 2017:

    Of course, this is only const if the function is const. My bad.

  5. benma approved
  6. benma commented at 1:03 PM on June 1, 2017: none

    utACK 1a3c1e24cbce60910843f434dc6c489d03eaed1d

  7. practicalswift force-pushed on Jun 1, 2017
  8. practicalswift force-pushed on Jun 1, 2017
  9. benma commented at 9:28 PM on June 4, 2017: none

    utACK 6090e51b5d3f8024197a31e6a5a50b812ba10aca

  10. MarcoFalke commented at 4:02 PM on June 5, 2017: member

    utACK 6090e51b5d3f8024197a31e6a5a50b812ba10aca does not change the binariy on my platform

  11. practicalswift force-pushed on Jun 18, 2017
  12. practicalswift force-pushed on Jul 15, 2017
  13. practicalswift force-pushed on Jul 17, 2017
  14. practicalswift commented at 6:14 PM on July 17, 2017: contributor

    Rebased!

  15. practicalswift force-pushed on Aug 14, 2017
  16. practicalswift commented at 3:44 PM on August 14, 2017: contributor

    Rebased!

  17. practicalswift force-pushed on Sep 10, 2017
  18. practicalswift commented at 7:06 PM on September 10, 2017: contributor

    Rebased! :-)

  19. practicalswift force-pushed on Sep 21, 2017
  20. practicalswift force-pushed on Sep 21, 2017
  21. Use static_cast instead of C-style casts for non-fundamental types
    A C-style cast is equivalent to try casting in the following order:

1. const_cast(...)
2. static_cast(...)
3. const_cast(static_cast(...))
4. reinterpret_cast(...)
5. const_cast(reinterpret_cast(...))

By using static_cast<T>(...) explicitly we avoid the possibility
of an unintentional and dangerous reinterpret_cast. Furthermore
static_cast<T>(...) allows for easier grepping of casts.
    9ad6746ccd
  22. practicalswift force-pushed on Sep 22, 2017
  23. practicalswift commented at 2:05 PM on September 22, 2017: contributor

    Rebased!

  24. practicalswift commented at 12:45 PM on January 31, 2018: contributor

    Anyone willing to review? ACK or NACK? :-)

  25. ghost commented at 5:34 PM on January 31, 2018: none

    Why not go the extra mile and also add -Wold-style-cast, otherwise you'll be back fixing it a week later.

  26. ryanofsky commented at 5:53 PM on January 31, 2018: member

    utACK 9ad6746ccd6dc31141fd0144686b641e31bf626b

  27. MarcoFalke commented at 6:07 PM on January 31, 2018: member

    re-reviewed

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-utACK 9ad6746ccd6dc31141fd0144686b641e31bf626b
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJacgWeAAoJENLqSFDnUoslDH0P/0t30sv4UootDVMUutXKfnsK
mHKRDOKMbWLYQonJ93x89Oll+U+r+B7m7CzlxqxUNI2Zd6DETOprwLAG8F1yKHez
JUjjndpfP/6wgzyv+YEJPehFL2vVSu3Di0j82qk/1vSaYHA94bh6Y+pRnVE65951
YdpuD0scPfXIMuR93swiGpw8ttFEqZVcGtjzWlwgKO23cdJaIlZatmpi1Mw/eQea
pqhol7AgtzZeEnISYDRVcObPxVzUabGDkVXUjafJK14enMLpg6ddjV4tldLCtVfR
J5yEfxGGTcYLm5TYpvYGQS58GWZSdbUm22HPDCcjYZW+NVvXRcvsaIYPE+BZJ/u/
q9jEs3wAP7lhTp0XiFmJ6UFlyrkVBopGmNI1KPWtsxdvjoouAH8XpO3RsB6/pNZq
nLZcvg/QNSbJ5ypMFshcZ2iJjVtgmIuwC/kceAuy3nK0EkK9BFUd9qS3dzdAbYiZ
aYfZjn6fu0PWxItMzEMActleYwAdhQLjc6tX35tSlVE0ZOSJx0FDWi6dIPyzlX6L
jWGKpMCm2m/Xt19av4IS9AHk+xoIGsQrgaWt+7TYc7vssR/dlw8IXsKI2D68AEJE
IXxh6HGNeBu/2cbTQ4bwA4CtAaE5sS44EdbxbduF1mAPhUJEoywP51F9VkNAqaQQ
64GCcFS6tzqUfliIrLGl
=Gwy1
-----END PGP SIGNATURE-----
  28. practicalswift commented at 8:33 AM on February 1, 2018: contributor

    @kekimusmaximus That would be nice but I think there might be some instances of C-style casts for non-fundamental types in our dependencies, no? :-)

  29. ryanofsky commented at 9:10 PM on February 7, 2018: member

    Seems like this could be merged: obvious change, has 3 acks.

  30. MarcoFalke merged this on Feb 7, 2018
  31. MarcoFalke closed this on Feb 7, 2018
  32. MarcoFalke referenced this in commit 0277173b1d on Feb 7, 2018
  33. in src/qt/bitcoin.cpp:712 in 9ad6746ccd 
     708 | @@ -709,7 +709,7 @@ int main(int argc, char *argv[])
 709 |          if (BitcoinCore::baseInitialize()) {
 710 |              app.requestInitialize();
 711 |  #if defined(Q_OS_WIN) && QT_VERSION >= 0x050000
 712 | -            WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), (HWND)app.getMainWinId());
 713 | +            WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), static_cast<HWND>(app.getMainWinId()));
    Sjors commented at 9:01 PM on February 10, 2018:

    ~Maybe unrelated, but~ I'm trying to make a Gitian Windows build for a branch based off of master and it throws:

    qt/bitcoin.cpp: In function ‘int main(int, char**)’:
qt/bitcoin.cpp:709:173: error: invalid static_cast from type ‘WId {aka unsigned int}’ to type ‘HWND’
             WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), static_cast<HWND>(app.getMainWinId()));                                                                                                                                                                       ^
    Sjors commented at 9:09 PM on February 10, 2018:

    Just noticed #12386...

  34. practicalswift referenced this in commit f40df29d96 on Feb 12, 2018
  35. MarcoFalke referenced this in commit c997f88082 on Feb 12, 2018
  36. esotericnonsense referenced this in commit d4a24b5471 on Feb 19, 2018
  37. Willtech referenced this in commit a3fa7055aa on Feb 23, 2018
  38. HashUnlimited referenced this in commit d7183ebaf6 on Jul 31, 2018
  39. lionello referenced this in commit 8b245ac953 on Nov 7, 2018
  40. jasonbcox referenced this in commit 256ef1e899 on Apr 5, 2019
  41. jonspock referenced this in commit b0ce0d03b4 on Apr 7, 2019
  42. jonspock referenced this in commit 51aa76af4c on Apr 8, 2019
  43. jonspock referenced this in commit e6f978735f on Apr 8, 2019
  44. jonspock referenced this in commit e41df7d91d on Apr 8, 2019
  45. PastaPastaPasta referenced this in commit a1de165dbe on Jun 9, 2020
  46. PastaPastaPasta referenced this in commit d51b037fe4 on Jun 9, 2020
  47. PastaPastaPasta referenced this in commit e10ab64311 on Jun 10, 2020
  48. PastaPastaPasta referenced this in commit a09681c84f on Jun 10, 2020
  49. PastaPastaPasta referenced this in commit 8a1710c3dc on Jun 10, 2020
  50. PastaPastaPasta referenced this in commit 26ca61ec21 on Jun 11, 2020
  51. PastaPastaPasta referenced this in commit c7ef89ec3a on Jun 11, 2020
  52. PastaPastaPasta referenced this in commit a6c210a755 on Jun 11, 2020
  53. PastaPastaPasta referenced this in commit 13ca02c45f on Jun 11, 2020
  54. PastaPastaPasta referenced this in commit e4f13ff508 on Jun 11, 2020
  55. PastaPastaPasta referenced this in commit 1b4ba4fafd on Jun 11, 2020
  56. PastaPastaPasta referenced this in commit b8bb2a7ab5 on Jun 12, 2020
  57. random-zebra referenced this in commit 9232a36130 on Feb 13, 2021
  58. practicalswift deleted the branch on Apr 10, 2021
  59. gades referenced this in commit a0257555e7 on Feb 5, 2022
  60. DrahtBot locked this on Aug 18, 2022
Contributors
practicalswiftbenmaMarcoFalkeghostryanofskySjors
Labels
Refactoring
Linked (view graph)
#12325 Use dynamic_cast for downcasting instead of static_cast.#12416 Fix Windows build errors introduced in #10498
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-04-22 06:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me