Avoid using numeric_limits for sequence numbers and lock times#14636
Merged
Conversation
The deleted LOC is a dup so far as `std::numeric_limits<unsigned int>::min()` == 0
Switches to named constants, because numeric_limits calls can be harder to read and less portable. Change was suggested by James O'Beirne <james.obeirne@gmail.com> in bitcoin#10973 (comment) There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where SignalsOptInRBF() and MutateTxAddInput() functions would now work correctly.
Member
|
Tagged "refactoring" because the platforms that are broken are supposedly not supported. |
Contributor
|
utACK 8041cd3 |
hebasto
reviewed
Nov 1, 2018
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Member
|
@ryanofsky Can you combine the test change from #14463 into this? I'll close that PR, as this has more extensive changes. |
Contributor
Author
|
Updated 8041cd3 -> 90a9e72 (pr/climit.1 -> pr/climit.2) adding commits from #14463 |
gmaxwell
reviewed
Nov 5, 2018
Contributor
Author
|
Updated 90a9e72 -> 5352030 (pr/climit.2 -> pr/climit.3) adding U suffix (doesn't hurt to be explicit). |
Contributor
|
@ryanofsky I think the More specifically that the integer literal of That assumption is false :-) Proof: |
Contributor
Author
|
The u suffix isn't needed, but it's nice because it makes it more obvious that compiler will choose an unsigned type. (According to the "type of the literal" table in https://en.cppreference.com/w/cpp/language/integer_literal, the compiler without the u would choose smallest signed or unsigned type that holds the value.) |
Contributor
|
ACK 5352030 |
Contributor
|
utACK 5352030 |
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Nov 7, 2018
…and lock times 5352030 Avoid using numeric_limits for sequence numbers and lock times (Russell Yanofsky) bafb921 Remove duplicated code (Hennadii Stepanov) e4dc39b Replace platform dependent type with proper const (Hennadii Stepanov) Pull request description: Switches to named constants, because numeric_limits calls can be harder to read and less portable. Change was suggested by jamesob in bitcoin#10973 (comment) There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and `MutateTxAddInput` functions would now work correctly. Tree-SHA512: 3f5c6393c260551f65a0edfba55ef7eb3625232eec8d85b1457f26e144aa0b90c7ef5f44b2fd2f7d9be3c3bcb301030a9f5473c21b3bac566cc59b8c8780737c
5tefan
added a commit
to 5tefan/dash
that referenced
this pull request
Jul 28, 2021
Merges bitcoin#14636: Avoid using numeric_limits for sequence numbers and lock times. 5352030 Avoid using numeric_limits for sequence numbers and lock times (Russell Yanofsky) bafb921 Remove duplicated code (Hennadii Stepanov) e4dc39b Replace platform dependent type with proper const (Hennadii Stepanov) Pull request description: Switches to named constants, because numeric_limits calls can be harder to read and less portable. Change was suggested by jamesob in bitcoin#10973 (comment) There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and `MutateTxAddInput` functions would now work correctly.
PastaPastaPasta
pushed a commit
to dashpay/dash
that referenced
this pull request
Jul 28, 2021
…and lock times (#4296) Merges bitcoin#14636: Avoid using numeric_limits for sequence numbers and lock times. 5352030 Avoid using numeric_limits for sequence numbers and lock times (Russell Yanofsky) bafb921 Remove duplicated code (Hennadii Stepanov) e4dc39b Replace platform dependent type with proper const (Hennadii Stepanov) Pull request description: Switches to named constants, because numeric_limits calls can be harder to read and less portable. Change was suggested by jamesob in bitcoin#10973 (comment) There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and `MutateTxAddInput` functions would now work correctly.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
9 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.
Switches to named constants, because numeric_limits calls can be harder to read and less portable.
Change was suggested by jamesob in #10973 (comment)
There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where
SignalsOptInRBFandMutateTxAddInputfunctions would now work correctly.