Improve readability by removing redundant casts to same type (on all platforms) #9598

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:remove-redundant-casts changing 2 files +4 −4
  1. practicalswift commented at 8:45 am on January 20, 2017: contributor

    Same binaries check under Linux:

     0$ ../bitcoin-maintainer-tools/build-for-compare.py 874f13821f4193bd037cd37d005ee76b5a849398 82274c02ed2d82537dc55f008a29edb1bc09bbc4 --executables "src/bitcoind,src/bitcoin-cli,src/bitcoin-tx"
 1
 2$ sha256sum /tmp/compare/*.stripped
 31fe1a8827474f7f24475ce3dc851e7ac658d4ed0ae38d11e67f5a810671eaa15  /tmp/compare/bitcoin-cli.82274c02ed2d82537dc55f008a29edb1bc09bbc4.stripped
 41fe1a8827474f7f24475ce3dc851e7ac658d4ed0ae38d11e67f5a810671eaa15  /tmp/compare/bitcoin-cli.874f13821f4193bd037cd37d005ee76b5a849398.stripped
 5342c2ed0e60b60990a58cbf5845b256a4f9e3baff9db074baba5e34a620a60ea  /tmp/compare/bitcoind.82274c02ed2d82537dc55f008a29edb1bc09bbc4.stripped
 6342c2ed0e60b60990a58cbf5845b256a4f9e3baff9db074baba5e34a620a60ea  /tmp/compare/bitcoind.874f13821f4193bd037cd37d005ee76b5a849398.stripped
 7e4b2a80b2361d5cefd67a47eeb9298b8b712c26c7779d979348be8b2c7e3ec93  /tmp/compare/bitcoin-tx.82274c02ed2d82537dc55f008a29edb1bc09bbc4.stripped
 8e4b2a80b2361d5cefd67a47eeb9298b8b712c26c7779d979348be8b2c7e3ec93  /tmp/compare/bitcoin-tx.874f13821f4193bd037cd37d005ee76b5a849398.stripped
 9
10$ git diff -W --word-diff /tmp/compare/874f13821f4193bd037cd37d005ee76b5a849398 /tmp/compare/82274c02ed2d82537dc55f008a29edb1bc09bbc4
11
12$
  2. MarcoFalke added the label Refactoring on Jan 20, 2017
  3. practicalswift renamed this:
    Improve readability by removing redundant casts to same type (on all platforms)
    Improve readability by removing redundant casts to same type (on all platforms) [WIP]
    on Jan 20, 2017
  4. practicalswift force-pushed on Jan 20, 2017
  5. practicalswift renamed this:
    Improve readability by removing redundant casts to same type (on all platforms) [WIP]
    Improve readability by removing redundant casts to same type (on all platforms)
    on Jan 20, 2017
  6. in src/primitives/transaction.cpp: in 874f13821f outdated 
    107@@ -108,7 +108,7 @@ unsigned int CTransaction::CalculateModifiedSize(unsigned int nTxSize) const
108         nTxSize = (GetTransactionWeight(*this) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR;
109     for (std::vector<CTxIn>::const_iterator it(vin.begin()); it != vin.end(); ++it)
110     {
111-        unsigned int offset = 41U + std::min(110U, (unsigned int)it->scriptSig.size());
112+        unsigned int offset = 41U + std::min(110U, it->scriptSig.size());
    luke-jr commented at 11:42 pm on January 20, 2017:

    It seems like this is a regression? size_t is not necessarily the same size as unsigned int.

    But the current code is also non-ideal. It should probably be:

    0size_t offset = 41U + std::min(size_t(110), it->scriptSig.size());
    sipa commented at 11:45 pm on January 20, 2017:

    Even better:

    0size_t offset = 41U + std::min<size_t>(110, it->scriptSig.size());
    practicalswift commented at 6:47 am on January 21, 2017:
    @luke-jr @sipa Good points! Fixed, squashed and pushed :-)
  7. luke-jr changes_requested
  8. practicalswift force-pushed on Jan 21, 2017
  9. dcousens approved
  10. practicalswift commented at 10:18 pm on February 27, 2017: contributor
    Any changes needed before merge? :-)
  11. paveljanik commented at 12:31 pm on February 28, 2017: contributor
    What is the problem you are trying to solve? It is very hard to review similar tasks because of multiple architectures…
  12. practicalswift commented at 1:23 pm on February 28, 2017: contributor

    @paveljanik The goal of this PR is to improve readability by removing redundant casts. Let me know if you see any need for any of these explicit casts and I’ll adjust my PR accordingly. My reading is that they’re all redundant.

    More specifically my reading is that:

    • &vDataToHash[0] + nblocks * 4 is guaranteed to be of type const uint8_t*.
    • FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS is guaranteed to be of type int.
    • &privkey[…] is guaranteed to be of type unsigned char*.
    • &vchSig[…] is guaranteed to be of type unsigned char*.

    Let me know if I’m mistaken :-)

  13. practicalswift force-pushed on Mar 8, 2017
  14. practicalswift commented at 9:21 am on March 8, 2017: contributor
    Conflicts resolved. Rebased and pushed!
  15. practicalswift commented at 12:16 pm on April 10, 2017: contributor
    Adding link to “Avoid casts” (ES.48) in the C++ Core Guidelines.
  16. practicalswift force-pushed on Jul 15, 2017
  17. practicalswift commented at 1:00 pm on July 15, 2017: contributor
    Rebased!
  18. sipa commented at 6:04 pm on July 15, 2017: member
    utACK a0e84aa34827538cbc06d5c9a32c658b0919d181
  19. TheBlueMatt commented at 3:36 pm on July 16, 2017: member
    utACK a0e84aa34827538cbc06d5c9a32c658b0919d181
  20. practicalswift commented at 12:58 pm on October 30, 2017: contributor
    Is this one ready for merge? :-)
  21. Improve readability by removing redundant casts to same type (on all platforms) 06edc23f74
  22. practicalswift force-pushed on Dec 21, 2017
  23. practicalswift commented at 12:36 pm on December 21, 2017: contributor
    Rebased!
  24. sipa commented at 6:38 pm on March 6, 2018: member
    Still utACK 06edc23f7409160adaaea5dd8d80b5dcaf696f99
  25. practicalswift commented at 9:42 pm on March 6, 2018: contributor
    @laanwj Does this PR have a chance of getting merged? If so I’ll keep rebasing :-)
  26. MarcoFalke commented at 1:34 pm on March 7, 2018: member 
     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3utACK 06edc23f7409160adaaea5dd8d80b5dcaf696f99
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIcBAEBCgAGBQJan+puAAoJENLqSFDnUoslqv0P/0oRpXH3nuTlCdwrsNgba/ol
 7TeJvBMzrFWerQQ4bqX1qSKTkBDDqvAqmybEm4nFsEmR74FaPSftCVP2P8PFpxkZP
 8WtVCFvsWhiTtvfDXWui0c2llsFrmqsB4Ivf3rbHkU92OIqMS5tqvmmBuaNiA5K/Q
 9k7vqj6lkl2zG81mjXQFj18mps9L2Ik8yVS7VbHZO5FyLnJxQsRJQ96nr86h0rO7Z
10IFiOdlkfGmEBwD0KWwFMNqetQUw399maMHsBYibugofWuupyOqD6eZp6AOkre7dD
110ickdOPF6ATzWI6me34SIueviJiuTf9nF19E0GmTu1sVkRsJVx6EKXGN3YhwJmlL
126hKuUe/RWRWHMVEP1KkXF+jxHgEjoWfTg3kocG2ZRkR6vvIJNPi1ux0ZGQQDfj5P
13Gaz84AKrxl9hpYwQy1+35F7H885cSQvU+FVigpUCmxtHvHxcwy5vs9xvlMQwIDy1
14M9EWN3Mu1oeNx9hyJMt+kZu0XQsUIJa+sE1sGg7PhsrG29znOpI7/o6LisaP2pYY
15CJUy+5jRUDK8GhRP9qnzgooMm89h9Fer/LZKNyoo7xs/tPY2829VPk3JnDpSNH1A
1613Bb0EwWBhjP33tTXzLL1VrH3OhvMBY9ruv5R/+d72jeVZHlD40iLEbvviSYb4Q8
17O4eaEgNGRgovkm3mteye
18=nBtN
19-----END PGP SIGNATURE-----
  28. laanwj merged this on Mar 7, 2018
  29. laanwj closed this on Mar 7, 2018
  30. laanwj referenced this in commit a34ac6ae07 on Mar 7, 2018
  31. anton48 commented at 10:35 am on June 19, 2018: none

    Hi!

    the change to file ‘init.cpp’ breaks compilation on FreeBSD:

    init.cpp:965:32: error: no matching function for call to ‘min’ nMaxConnections = std::max(std::min(nMaxConnections, FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS), 0); ^~~~~~~~ /usr/include/c++/v1/algorithm:2581:1: note: candidate template ignored: deduced conflicting types for parameter ‘_Tp’ (‘int’ vs. ‘unsigned int’) min(const _Tp& __a, const _Tp& __b) ^ /usr/include/c++/v1/algorithm:2591:1: note: candidate template ignored: could not match ‘initializer_list’ against ‘int’ min(initializer_list<_Tp> __t, _Compare __comp) ^ /usr/include/c++/v1/algorithm:2573:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided min(const _Tp& __a, const _Tp& __b, _Compare __comp) ^ /usr/include/c++/v1/algorithm:2599:1: note: candidate function template not viable: requires single argument ‘__t’, but 2 arguments were provided min(initializer_list<_Tp> __t) ^ 1 error generated.

    and actually this change reverts pull request from 2013: #2695

  32. practicalswift commented at 12:00 pm on June 19, 2018: contributor
    @anton48 Thanks for reporting! I’ll investigate! :-)
  33. practicalswift commented at 1:50 pm on June 19, 2018: contributor
    @anton48 Fix in #13503. Please confirm it solves the problem :-)
  34. MarcoFalke referenced this in commit c655b2c2df on Jun 27, 2018
  35. jasonbcox referenced this in commit db003f8301 on Oct 25, 2019
  36. jonspock referenced this in commit f90a849b67 on Dec 27, 2019
  37. jonspock referenced this in commit 8cb6c66f09 on Dec 29, 2019
  38. codablock referenced this in commit ac02dcf7d5 on Apr 7, 2020
  39. codablock referenced this in commit 9591199629 on Apr 8, 2020
  40. deadalnix referenced this in commit ee323c8b68 on Jun 9, 2020
  41. PastaPastaPasta referenced this in commit 23ff62c406 on Jun 13, 2020
  42. PastaPastaPasta referenced this in commit f90c47d7b5 on Jun 13, 2020
  43. PastaPastaPasta referenced this in commit 8acd51ae11 on Jun 17, 2020
  44. PastaPastaPasta referenced this in commit bf6dbb348d on Jun 18, 2020
  45. ftrader referenced this in commit add9abcee0 on Aug 17, 2020
  46. zkbot referenced this in commit 7d94064616 on Sep 29, 2020
  47. ckti referenced this in commit 136846ae40 on Mar 28, 2021
  48. practicalswift deleted the branch on Apr 10, 2021
  49. gades referenced this in commit 1b227c47ff on Jun 25, 2021
  50. malbit referenced this in commit aa9efc23e4 on Nov 5, 2021
  51. gades referenced this in commit 8c955a9ce0 on Apr 1, 2022
  52. gades referenced this in commit d41be722da on Apr 1, 2022
  53. DrahtBot locked this on Aug 18, 2022


practicalswift luke-jr sipa dcousens paveljanik TheBlueMatt MarcoFalke meshcollider anton48

Labels
Refactoring

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-22 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me