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
dfp: cleanup and cache feature #34068
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
runRemoveCallbacks(host); | ||
} | ||
{ | ||
removeCacheEntry(host); |
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.
Out of curiosity, can this be moved outside of the scoped block (which seems to be for the writer_lock)?
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.
good call - I was just copying what was done above.
// If we need to erase the host, hold onto the PrimaryHostInfo object that owns this callback. | ||
// This is defined at function scope so that it is only erased on function exit to avoid | ||
// use-after-free issues | ||
PrimaryHostInfoPtr host_to_erase; |
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.
Up to you but I would be inclined to move this down closer to where it's used (just before the opening of the scoped block).
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.
as these are optional comments, will do in a follow-up. next PR in this space :-)
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]