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.
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
mem: introduce
mem
package to facilitate memory reuse #7432mem: introduce
mem
package to facilitate memory reuse #7432Changes from all commits
be4d4ab
5624e37
169406f
dace88f
14c288c
ae308d4
01ac2c6
1261d3e
50bda24
4d0eaa5
f6f8b48
5aa0784
2ee31ca
3a8cac5
aaca195
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is the clearing necessary? I would think we could remove it. Or is there an attack vector for code already running inside your binary that wouldn't otherwise be possible?
If the clearing is important, then we should add a test for it.
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.
I remember making the same comment in one of the older PRs. Paul had some rationale for it, but I don't remember it now. Will wait for Paul on this.
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.
I'll throw a test around this. It seems like a good idea to have since anything that remains in the buffer can leak when it's reused.
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.
Added test
Check warning on line 156 in mem/buffer_pool.go
mem/buffer_pool.go#L156
Check warning on line 162 in mem/buffer_pool.go
mem/buffer_pool.go#L162