remove duplicate boundary check in script.cpp #15815

pull r8921039 wants to merge 1 commits into bitcoin:master from r8921039:remove_duplicate_boundary_check changing 1 files +0 −2

r8921039 commented at 4:20 PM on April 14, 2019: contributor
If I'm not mistaken, the statement
```
if (pc >= end) return false;
```
is logically equivalent to the next statement
```
if (end - pc < 1) return false;
```
If that's the case, the PR removes the former for better efficiency and code readability.
remove one duplicate boundary check 'if (pc >= end) return false;' as the logic is equivalent to the boundary check right after 'if (end - pc < 1) return false;' 0545a049a2
scravy commented at 5:05 PM on April 14, 2019: contributor

For readability I guess removing the latter and keeping the former would be more clear. Also it's less instructions actually (single comparison vs subtraction (a pointer diff!) and a comparison).
andrewtoth commented at 5:12 PM on April 14, 2019: contributor

The former also returns false if pc and end are equal, while the latter does not.
r8921039 commented at 5:23 PM on April 14, 2019: contributor

Thank you @scravy for your comment, that's good points. I chose to keep the pattern of the latter as it is used repeatedly in the rest of the function whereas the former is not. I will modify the PR to keep the former instead if that the preference of the majority.
r8921039 commented at 5:32 PM on April 14, 2019: contributor

@andrewtoth the latter also returns false as 0 < 1 is true, which then returns false.
andrewtoth commented at 5:35 PM on April 14, 2019: contributor

Oops right.
JeremyRubin commented at 6:03 PM on April 14, 2019: contributor

It's a great find, but ultimately removing such a check is not worth it given the sensitivity of this code, especially if it makes the code less readable. The optimizer should optimize this in any case.

Nack
promag commented at 9:41 PM on April 14, 2019: member

Agree with @JeremyRand.
fanquake commented at 12:51 AM on April 15, 2019: member

Thanks for the contribution @r8921039, however I agree with the commenters above.
fanquake closed this on Apr 15, 2019
jnewbery commented at 1:28 AM on April 15, 2019: member

@r8921039: if you're looking for places to contribute, look at the contributor guidelines and good first issue list.
r8921039 commented at 5:21 AM on April 15, 2019: contributor

Totally understandable, thanks you all for the reviews and comments.
r8921039 deleted the branch on Apr 15, 2019
MarcoFalke commented at 1:58 PM on April 15, 2019: member
If you are looking for small things to help out with, you can
- search through the good first issues or the ones that are up for grabs.
- write tests to improve the coverage (both unit tests and rpc tests are welcome)
- help with review and testing (There are easy ones such as the gui and rpc)
- join us on irc and let us know what you are interested in.
DrahtBot locked this on Dec 16, 2021

Contributors