refactor: GetFirstStoredBlock() and getblockchaininfo follow-ups #25016
Conversation
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
…anager instead of a global
…erence instead of by pointer, so as to not accept a nullptr.
e12d4f0 to
e2b954e
Compare
|
Updated now that #24956 is merged, and ready for review. |
maflcko
approved these changes
Apr 28, 2022
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 29, 2022
…ninfo follow-ups e2b954e rpc: use GetBlockTime() for getblockchaininfo#time (Jon Atack) 86ce844 blockstorage, refactor: pass GetFirstStoredBlock() start_block by reference (Jon Atack) ed12c0a blockstorage, refactor: make GetFirstStoredBlock() a member of BlockManager (Jon Atack) Pull request description: Picks up the remaining review feedback in bitcoin#21726 and bitcoin#24956. - make the global function `GetFirstStoredBlock()` a member of the `BlockManager` class - pass the `start_block` param of `GetFirstStoredBlock()` by reference instead of a pointer - use `GetBlockTime()` for RPC getblockchaininfo#time ACKs for top commit: MarcoFalke: ACK e2b954e Tree-SHA512: 546e3c2e18245996b5b286829a605ae919eff3510963ec71b7c9ede521b1f501697e5b2f9d35d7a0606a74cbc8907201c58acf1e2cf7daaa86eefe2e3a8e296b
maflcko
reviewed
Apr 30, 2022
| @@ -390,10 +390,10 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex) | |||
| return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0); | |||
| } | |||
|
|
|||
| const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) { | |||
| const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_block) | |||
There was a problem hiding this comment.
In theory start_block could be marked LIFETIMEBOUND.
There was a problem hiding this comment.
In theory
start_blockcould be marked LIFETIMEBOUND.
Ah, yes; done in #25060.
maflcko
pushed a commit
to bitcoin-core/gui
that referenced
this pull request
May 4, 2022
…rstStoredBlock()::start_time 4cb9d21 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack) Pull request description: Suggested in bitcoin/bitcoin#25016 (comment), the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and #22278 for related discussion, and #25040 for a similar example. ACKs for top commit: MarcoFalke: review ACK 4cb9d21 Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 4, 2022
…dBlock()::start_time 4cb9d21 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack) Pull request description: Suggested in bitcoin#25016 (comment), the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and bitcoin#22278 for related discussion, and bitcoin#25040 for a similar example. ACKs for top commit: MarcoFalke: review ACK 4cb9d21 Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Picks up the remaining review feedback in #21726 and #24956.
GetFirstStoredBlock()a member of theBlockManagerclassstart_blockparam ofGetFirstStoredBlock()by reference instead of a pointerGetBlockTime()for RPC getblockchaininfo#time