crypto: fixup randomFill size and offset handling#38138
crypto: fixup randomFill size and offset handling#38138jasnell wants to merge 3 commits intonodejs:masterfrom
Conversation
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>
023bb27 to
b394ef2
Compare
addaleax
left a comment
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
left a comment
There was a problem hiding this comment.
Let’s at least leave a comment about why this is so confusing
Co-authored-by: Anna Henningsen <anna@addaleax.net>
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 |
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 |
Fixes: #38137
Signed-off-by: James M Snell jasnell@gmail.com