Skip to content
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

opt: Stacked DRG: swap buffers instead of memcpy #1197 #1198

Merged
merged 2 commits into from
Jul 9, 2020
Merged

Conversation

keyvank
Copy link
Contributor

@keyvank keyvank commented Jul 2, 2020

Resolves #1197

cryptonemo
cryptonemo previously approved these changes Jul 2, 2020
Copy link
Collaborator

@cryptonemo cryptonemo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I didn't realize that std::mem::swap 'Swaps the values at two mutable locations, without deinitializing either one.' That last part is key for this solution (at least that's what I was missing about it).

porcuquine
porcuquine previously approved these changes Jul 3, 2020
Copy link
Collaborator

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks good!

I pushed a commit I think clarifies the meaning of these buffers and will help minimize confusion for future readers. This was a little easier than trying to explain my hope. If you accept these changes, I think this is good to go (after rebase).

@keyvank
Copy link
Contributor Author

keyvank commented Jul 3, 2020

@porcuquine LGTM

cryptonemo
cryptonemo previously approved these changes Jul 6, 2020
porcuquine
porcuquine previously approved these changes Jul 8, 2020
@porcuquine
Copy link
Collaborator

Needs rebase.

@keyvank keyvank merged commit 1d1097e into master Jul 9, 2020
@keyvank keyvank deleted the fps-19-1197 branch July 9, 2020 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security Audit FPS-19
3 participants