Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve noop comparison warning #218

Merged
merged 2 commits into from
Oct 24, 2020
Merged

Conversation

xloem
Copy link
Contributor

@xloem xloem commented Oct 22, 2020

libbitcoin-protocol/src/zmq/certificate.cpp:82:42: warning: comparison is always true due to limited range of data type [-Wtype-limits]
     for (uint8_t attempt = 0; attempt <= max_uint8; attempt++)

attempt will wrap and the loop never terminates. It looks like that's fine rare behavior.

@evoskuil
Copy link
Member

Thanks. It looks like the intended behavior was to terminate with an error after 256 failed attempts, not to loop indefinitely. That’s corrected by changing the condition to <. Unlimited attempts is probably ok, but probabilistic halting never feels right.

@evoskuil evoskuil added the build label Oct 22, 2020
@thecodefactory
Copy link
Member

I prefer the bounded option. Another option is to update the settings loader to not need the repeated generations (as specified in the comments).

@xloem
Copy link
Contributor Author

xloem commented Oct 23, 2020

Where are the settings deserialized?

@evoskuil
Copy link
Member

Where are the settings deserialized?

https://github.com/libbitcoin/libbitcoin-system/blob/master/include/bitcoin/system/config/parser.hpp

@xloem
Copy link
Contributor Author

xloem commented Oct 23, 2020

Submission has been changed to the proposed solution; # situation addressed at boostorg/program_options#101 .

@evoskuil evoskuil merged commit 5c66b6f into libbitcoin:master Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants