crypto: fixup randomFill size and offset handling #38138
Closed
Conversation
This file contains 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
nodejs-github-bot
added
crypto
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: James M Snell <jasnell@gmail.com>
jasnell
force-pushed
the
fixup-randomfill-size
branch
from
023bb27
to
b394ef2
Compare
addaleax
reviewed
Apr 7, 2021
There was a problem hiding this comment.
I guess this LGTM in the sense that it makes the code work correctly now, but I’m not really eager to click “approve” given how hard to understand this code is (Functions whose names start with assert that silently use different units for input and output? I think we can do better…)
Agreed. I left these as is during the refactor and didn't like the way these were implemented then and still don't. It'll be good to refactor these next but let's address the immediate bug first. |
addaleax
approved these changes
Apr 8, 2021
Co-authored-by: Anna Henningsen <anna@addaleax.net>
jasnell
added
the
author ready
tniessen
approved these changes
Apr 8, 2021
RaisinTen
reviewed
Apr 8, 2021
RaisinTen
approved these changes
Apr 8, 2021
Co-authored-by: Darshan Sen <raisinten@gmail.com>
|
CI is green on this and it fixes a bug. There's no reason to make it wait the full 48 hours. Please 👍🏻 to fast track |
Trott
approved these changes
Apr 9, 2021
lpinca
approved these changes
Apr 9, 2021
jasnell
added a commit
that referenced
this pull request
Apr 9, 2021
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #38138 Fixes: #38137 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
|
Landed in d2f116c |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes: #38137
Signed-off-by: James M Snell jasnell@gmail.com