Fix sign comparison warnings#437
Open
MitchellCash wants to merge 6 commits intoPaycoinFoundation:masterfrom
Open
Fix sign comparison warnings#437MitchellCash wants to merge 6 commits intoPaycoinFoundation:masterfrom
MitchellCash wants to merge 6 commits intoPaycoinFoundation:masterfrom
Conversation
Contributor
|
@MitchellCash so does this more or less replace #366? |
to eliminate signed/unsigned comparison warnings
nFile's null value is -1. Cast that to unsigned int, to avoid warning. Additionally, avoid nFile==0 because the first valid value is 1.
MitchellCash
force-pushed
the
sign-compare
branch
from
7f81026 to
a35e228
Compare
this was in bitcoinrpc.cpp but has been moved since the original patch
Contributor
Author
|
@IngCr3at1on now it does ;) |
| if (nRndPos1 == nRndPos2) | ||
| return; | ||
|
|
||
| assert(nRndPos1 >= 0 && nRndPos2 >= 0); |
Contributor
There was a problem hiding this comment.
So essentially this is meant to ensure that the RndPos# variables are positive...
I'm wondering if there is any chance that they could ever be negative (if they were and this was left in place it would actually crash the application; I'm not sure what the behavior would be with this check missing); just because it was removed in Bitcoin doesn't necessarily mean it's safe or a good idea for us to remove it though...
Thoughts?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This does take some thunder from #366 but I think the commit messages are more detailed and it also includes some extra changes/improvements that #366 does not.