fix signed/unsigned in strprintf and CNetAddr::GetByte() #1793

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:fix_signed_unsigned_strprintf changing 5 files +9 −9

Diapolo commented at 9:41 PM on September 5, 2012: none
- I checked every occurance of strprintf() in the code and used %u, where unsigned vars are used
- the change to GetByte() was made, as ip is an unsigned char
As our strprintf() gives not the same kind of compiler-warnings we see with printf(), I checked all direct usage of strprintf() for correct format characters.
in src/netbase.cpp:None in af910fed59 outdated
```
 598 | @@ -599,7 +599,7 @@ bool CNetAddr::SetSpecial(const std::string &strName)
 599 |          *this = vIP[0];
 600 |  }
 601 |  
 602 | -int CNetAddr::GetByte(int n) const
 603 | +unsigned int CNetAddr::GetByte(int n) const
```
laanwj commented at 5:38 AM on September 6, 2012:

Agreed, bitmask arithmethic should be unsigned. Subsequent bit shifting can otherwise result in signed int overflows which are undefined in C(++). Though, scanning the code quickly, none of these happen at the moment. The largest shift we use with GetByte is 8. Still a useful precaution, though.
BitcoinPullTester commented at 1:08 AM on September 7, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/af910fed59f9230c751ffb8441d045b6883d7e01 for binaries and test log.
Diapolo commented at 7:20 AM on September 11, 2012: none

RC3 fix?

fix signed/unsigned in strprintf and CNetAddr::GetByte()

- I checked every occurance of strprintf() in the code and used %u, where
  unsigned vars are used
- the change to GetByte() was made, as ip is an unsigned char

463a1cab43

Diapolo commented at 7:34 PM on September 12, 2012: none

Rebased fixing merge conflict.
BitcoinPullTester commented at 4:49 AM on September 13, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/463a1cab43fda24f89f71fffc876756dc1bc155a for binaries and test log.
Diapolo commented at 5:25 PM on September 20, 2012: none

@sipa As this touches a part of your networking code, can you take a look here, too?
sipa commented at 1:00 PM on September 21, 2012: member

ACK
laanwj referenced this in commit e96a8c7d86 on Sep 21, 2012
laanwj merged this on Sep 21, 2012
laanwj closed this on Sep 21, 2012
DrahtBot locked this on Sep 8, 2021

Contributors