-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Implement Crystal::System::Random for win32 #5539
Conversation
src/crystal/system/win32/random.cr
Outdated
module Crystal::System::Random | ||
def self.random_bytes(buf : Bytes) : Nil | ||
raise NotImplementedError.new("Crystal::System::Random.random_bytes") | ||
unless LibC.RtlGenRandom(buf, buf.size) |
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.
Will this work even though it can't be nil?
Put another way: It'll never enter the unless's body
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.
Oops, fixed.
64ec074
to
7ffa7e4
Compare
Just a question: is win32 just 32 bits of windows? |
@asterite no, it's just the name of the ABI, for example there's |
So Windows API is called Win 32? Even if it's running in a 64 bits machine? At least that's what I understand from the comment above, and why we can't just use |
|
||
@[Link("advapi32")] | ||
lib LibC | ||
fun RtlGenRandom = SystemFunction036(random_buffer : Void*, random_buffer_length : ULong) : BOOLEAN |
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.
Why the alias?
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.
Nevermind, I guess it's a #define
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.
RtlGenRandom is what it's called in the windows documentation. The point of keeping the windows names is to be able to find the documentation easilly, so I think this is correct. It's much easier to read this way anyway.
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.
Have a look to libsodium sysrandom. The function is known as RtlGenRandom, but the symbol is SystemFunction036
and kind of private API, but everybody uses it (libsodium, go, etc) because it avoids bundling the whole crypto API, which is apparently quite big.
An alias, here, is expected, yes :-)
@asterite I think @ysbaddaden explained it perfectly in the comment I linked... Cygwin and mingw also have |
Yup, we must be pass |
Nice and short implementation on this one!