Improve readability of DecodeBase58Check(...) #10961

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:DecodeBase58Check-cleanup changing 1 files +1 −1

practicalswift commented at 10:32 AM on July 31, 2017: contributor
Use the more readable form ...
```
&vchRet[vchRet.size() - 4]
```
... instead of ...
```
&v.end()[-n]
```
Has the added benefit of eliminating a spurious static analyzer warning about improper use of negative values.
practicalswift force-pushed on Jul 31, 2017
promag commented at 2:50 PM on July 31, 2017: member

utACK 6ab2ce8.
jonasschnelli commented at 9:52 AM on August 3, 2017: contributor

This seems to be an over-cosmetic change... not sure if its worth. utACK 6ab2ce83d9268fc81ac3bec41365ce2d2adfd349
jonasschnelli added the label Refactoring on Aug 3, 2017
in src/base58.cpp:139 in 6ab2ce83d9 outdated
```
 135 | @@ -136,7 +136,7 @@ bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet)
 136 |      }
 137 |      // re-calculate the checksum, ensure it matches the included 4-byte checksum
 138 |      uint256 hash = Hash(vchRet.begin(), vchRet.end() - 4);
 139 | -    if (memcmp(&hash, &vchRet.end()[-4], 4) != 0) {
 140 | +    if (memcmp(&hash, vchRet.data() + vchRet.size() - 4, 4) != 0) {
```
laanwj commented at 10:05 AM on August 23, 2017:

I guess there's some reason vchRet.end() - 4 doesn't work (pointer versus iterator conflict or something like that)?

meshcollider commented at 8:20 PM on August 23, 2017:

Yeah vchRet.end()-4 would still be an iterator I believe, so you'd have to do some ugly thing like &*(vchRet.end()-4) to make it into a pointer iirc?

sipa commented at 10:59 PM on August 23, 2017:

How about &vchRet[vchRet.size() - 4] ?

meshcollider commented at 12:56 AM on August 24, 2017:

Believe that would work, and yeah that's a lot clearer imo 👍
practicalswift closed this on Aug 24, 2017
practicalswift force-pushed on Aug 24, 2017
practicalswift reopened this on Aug 24, 2017
Improve readability of DecodeBase58Check(...) c6a995e7e5
practicalswift force-pushed on Aug 24, 2017
practicalswift commented at 7:23 AM on August 24, 2017: contributor

@laanwj @MeshCollider @sipa Thanks for reviewing! Changed to &vchRet[vchRet.size() - 4]. Looks good? :-)
meshcollider commented at 8:24 AM on August 24, 2017: contributor

Just confirmed this should be identical behavior, ACK https://github.com/bitcoin/bitcoin/pull/10961/commits/c6a995e7e5e6ae37dc1684f6060038e96864e947
practicalswift commented at 7:49 AM on August 25, 2017: contributor

Travis have completed without errors, but GitHub still reports "1 pending check".
practicalswift commented at 8:26 PM on August 28, 2017: contributor

Seems like Travis CI is stuck. Anyone able to re-start Travis? :-)
ryanofsky commented at 6:01 PM on August 29, 2017: member

utACK c6a995e7e5e6ae37dc1684f6060038e96864e947
sipa commented at 6:12 PM on August 29, 2017: member

utACK c6a995e7e5e6ae37dc1684f6060038e96864e947
laanwj merged this on Oct 9, 2017
laanwj closed this on Oct 9, 2017
laanwj referenced this in commit da0478e6e5 on Oct 9, 2017
PastaPastaPasta referenced this in commit 2cb7e22f92 on Jan 31, 2020
PastaPastaPasta referenced this in commit 2c124395a6 on Jan 31, 2020
PastaPastaPasta referenced this in commit ced1ed1190 on Feb 4, 2020
PastaPastaPasta referenced this in commit 8a944a1a1a on Feb 9, 2020
random-zebra referenced this in commit 961e2d2a70 on Jun 27, 2020
ckti referenced this in commit 68d68314a3 on Mar 28, 2021
practicalswift deleted the branch on Apr 10, 2021
gades referenced this in commit dcd247b0ce on Jul 1, 2021
gades referenced this in commit 7f7ff75741 on Feb 11, 2022
DrahtBot locked this on Aug 18, 2022

Contributors

Labels