refactor: add LIFETIMEBOUND to blockfilter where needed#25967
Merged
maflcko merged 1 commit intobitcoin:masterfrom Sep 1, 2022
Merged
refactor: add LIFETIMEBOUND to blockfilter where needed#25967maflcko merged 1 commit intobitcoin:masterfrom
maflcko merged 1 commit intobitcoin:masterfrom
Conversation
maflcko
approved these changes
Aug 31, 2022
Member
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
ACK. Seems unlikely that this would ever be used in a way to violate lifetimes, but adding the attribute can't hurt.
Ensure that the return values do not have a lifetime that exceeds the lifetime of what it is bound to. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound
stickies-v
force-pushed
the
blockfilter-add-lifetimebound
branch
from
60063f1 to
89576cc
Compare
Contributor
Author
|
Rebase to fix CI failure - no other changes. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 1, 2022
… needed 89576cc refactor: add LIFETIMEBOUND to blockfilter where needed (stickies-v) Pull request description: Noticed from bitcoin#25637 (comment) that [`BlockFilter::GetFilter()`](https://github.com/bitcoin/bitcoin/blob/01e1627e25bc5477c40f51da03c3c31b609a85c9/src/blockfilter.h#L132) returns a reference to a member variable. Added LIFETIMEBOUND to all blockfilter-related code to ensure that the return values do not have a lifetime that exceeds the lifetime of what it is bound to. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound or bitcoin#25060 for a similar example. I used `grep -E '[a-zA-Z>0-9][&*] ([a-zA-Z]*)\((.*)\)' src/**/blockfilter*` to grep all possible occurrences (not all of them require LIFETIMEBOUND) ACKs for top commit: brunoerg: crACK 89576cc Tree-SHA512: 6fe61fc0c1ed9e446edce083d1b093e1a5e2ef8c39ff74125bb12a24e514d45711845809817fbd4a04d7a9c23c8b362203771c17b6d831d2560b1af268453019
maflcko
pushed a commit
to bitcoin-core/gui
that referenced
this pull request
Sep 16, 2022
… index names 26cf9ea scripted-diff: rename pszThread to thread_name (stickies-v) 200d84d refactor: use std::string for index names (stickies-v) 97f5b20 refactor: use std::string for thread names (stickies-v) Pull request description: As a follow-up to bitcoin/bitcoin#25967 (comment), this PR changes the return type of [`BaseIndex::GetName()`](https://github.com/bitcoin/bitcoin/blob/fa5c224d444802dabec5841009e029b9754c92f1/src/index/base.h#L120) to `const std::string&` instead of `const char*`. The first commit is not essential for this change, but since the code is touched and index names are commonly used to specify thread names, I've made the same update there. No behaviour change, just refactoring to further phase out C-style strings. Note: `util::ThreadRename()` used to take an rvalue ref, but since it then passes this to `SetInternalName()` by value, I don't think there's any benefit to having both an rvalue and lvalue ref function so I just changed it into lvalue ref. Not 100% sure I'm missing something? ACKs for top commit: MarcoFalke: review ACK 26cf9ea only change is new scripted-diff 😀 hebasto: ACK 26cf9ea, I have reviewed the code and it looks OK. w0xlt: reACK bitcoin/bitcoin@26cf9ea Tree-SHA512: 44a03ebf2bb86ca1411a36222a575217cdba8ee3a3c985e74d74c934516f002b27336147fa22f59eda7dac21204a93951563317005d475da95b23c427014d77b
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.
4 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.
Noticed from #25637 (comment) that
BlockFilter::GetFilter()returns a reference to a member variable. Added LIFETIMEBOUND to all blockfilter-related code to ensure that the return values do not have a lifetime that exceeds the lifetime of what it is bound to. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound or #25060 for a similar example.I used
grep -E '[a-zA-Z>0-9][&*] ([a-zA-Z]*)\((.*)\)' src/**/blockfilter*to grep all possible occurrences (not all of them require LIFETIMEBOUND)