AvalancheBlockUpdate rely on the two lowers bits of a CBlockIndex* to be zero to work properly. While we know this is true, enforcing this is the case using a static assert is preferable.
Details
Details
- Reviewers
jasonbcox Fabien - Group Reviewers
Restricted Project - Commits
- rSTAGINGba12b8e01f69: [avalanche] Add some safety to AvalancheBlockUpdate using static asserts
rABCba12b8e01f69: [avalanche] Add some safety to AvalancheBlockUpdate using static asserts
make check
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- avacheckalign
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 4406 Build 6877: Bitcoin ABC Buildbot (legacy) Build 6876: arc lint + arc unit
Event Timeline
src/avalanche.h | ||
---|---|---|
153 ↗ | (On Diff #6419) | See the build failure: In file included from ../../src/avalanche.cpp:5:0: ../../src/avalanche.h:153:52: error: expected ‘,’ before ‘)’ token static_assert(alignof(CBlockIndex) >= 1 << BITS); static_assert(statement, message) needs an error message |
Comment Actions
I don't think you're checking anything useful here.
If the arch does not support unaligned access, the static_assert is redundant with the compiler behavior.
If the address is forced unaligned (e.g. using packing), the static_assert will not detect it anyway, and your status and block index will get corrupted.
You can only check at runtime that pindexIn is aligned to a multiple of (1 << STATUS_BITS).
Comment Actions
Unaligned pointers is undefined behavior. See: https://stackoverflow.com/a/28895321/672906
Comment Actions
Got explanation from offline discussion : this check will prevent breaking the code if CBlockIndex get modified so thath its size gets <(1 << STATUS_BITS)