-
Notifications
You must be signed in to change notification settings - Fork 37.7k
refactor: add LIFETIMEBOUND to blockfilter where needed #25967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: add LIFETIMEBOUND to blockfilter where needed #25967
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
60063f1
to
89576cc
Compare
Rebase to fix CI failure - no other changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crACK 89576cc
Coincidentally I was studying about LIFETIMEBOUND this morning. Nice catch!
… 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
… 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
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)