rippled
rippled copied to clipboard
Implement changes for Issue 3417
If merged, this code will:
- Implement the simplified condition evaluation
- removes the complex bitwise and(&) operator
- Implement the second proposed solution in Nik Bougalis's comment - https://github.com/XRPLF/rippled/issues/3417#issuecomment-635116233
- I have tested this code change by performing RPC calls with the commands
server_info
,server_state
,peers
andvalidation_info
. These commands worked as expected.
@nbougalis I have a question about the choice of integers inside the Condition enum. Does it necessarily have to be powers of 2? With respect to the operations in this file, it suffices to choose different integer values right?
I understand that the choice of even/odd values is important because we do bit-wise and operator.
They have to be powers of two if you want to be able to set and test the bits individually.
For example if you define:
enum { A=1, B=2, C=3};
and you do:
uint32_t x = C;
the value of x will be 0x3 (the two lower bits set), so you would have both (x & A) != 0
and (x & B) != 0
if you use powers of two the bits can be set and tested individually. I used to use enums like:
enum {
A = 1 << 0,
B = 1 << 1,
C = 1 << 2,
...
};
They have to be powers of two if you want to be able to set and test the bits individually.
For example if you define:
enum { A=1, B=2, C=3};
and you do:
uint32_t x = C;
the value of x will be 0x3 (the two lower bits set), so you would have both
(x & A) != 0
and(x & B) != 0
if you use powers of two the bits can be set and tested individually. I used to use enums like:
enum { A = 1 << 0, B = 1 << 1, C = 1 << 2, ... };
okay got it, thanks for the explanation! I would individually check all the conditions and that is quite verbose, this is concise 👍
You can use a sequence 1,2,3,4,5,... if all the conditions are exclusive from one another. But if they are flags that can be set independently, then it has to be separate bits to be workable.
You can use a sequence 1,2,3,4,5,... if all the conditions are exclusive from one another. But if they are flags that can be set independently, then it has to be separate bits to be workable.
Okay, got it 👍
@ckeshava would you like to pick this up?
Hello, I have completed the code changes. The unit tests are passing locally, I'm not sure why the CI tests are failing on the ubuntu machine.
(Holding this for 1.12 unless someone requests otherwise)
This has CI failures which need to be resolved (or explained). @ckeshava or @thejohnfreeman - could you have a look?
Please merge develop
.
OK, I've rebased to the develop
branch, thanks for the tip!
@HowardHinnant because there have been changes pushed after your earlier approval, could you do a quick re-review?