Avoid unsigned integer overflow in bitcoin-tx #24139

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2201-utilTxOverflow changing 2 files +5 −3

MarcoFalke commented at 3:10 PM on January 24, 2022: member

While npos means "largest unsigned value" and adding 1 to it yields 0, it may be clearer to just assign 0 to it and only increment otherwise.

This also allows to remove a file-wide suppression for unsigned-integer-overflow.
Avoid unsigned integer overflow in bitcoin-tx faa75fa193
MarcoFalke added the label Refactoring on Jan 24, 2022
MarcoFalke added the label Utils/log/libs on Jan 24, 2022
in src/bitcoin-tx.cpp:436 in faa75fa193
```
 432 | @@ -433,13 +433,16 @@ static void MutateTxAddOutData(CMutableTransaction& tx, const std::string& strIn
 433 |      if (pos==0)
 434 |          throw std::runtime_error("TX output value not specified");
 435 |  
 436 | -    if (pos != std::string::npos) {
 437 | +    if (pos == std::string::npos) {
```
hebasto commented at 3:33 PM on January 24, 2022:

nit: Preserving the original condition, i.e., switching branches if and else, makes diff smaller.

MarcoFalke commented at 4:05 PM on January 24, 2022:

Double negation makes code harder to read

MarcoFalke commented at 4:07 PM on January 24, 2022:

Since npos is already a negation, in this case the else would be triple-negated
hebasto approved
hebasto commented at 3:33 PM on January 24, 2022: member

ACK faa75fa19335e3e826efa4f2280609a2db34425d, I have reviewed the code and it looks OK, I agree it can be merged.
theStack approved
theStack commented at 2:23 PM on January 28, 2022: member

Code-review ACK faa75fa19335e3e826efa4f2280609a2db34425d
MarcoFalke merged this on Jan 28, 2022
MarcoFalke closed this on Jan 28, 2022
MarcoFalke deleted the branch on Jan 28, 2022
sidhujag referenced this in commit df145d3e32 on Jan 28, 2022
Fabcien referenced this in commit d550e775d2 on Dec 9, 2022
DrahtBot locked this on Jan 28, 2023

Contributors

MarcoFalke

hebasto

theStack

Labels

Refactoring Utils/log/libs