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
wasi: Implement
random_get
function #241wasi: Implement
random_get
function #241Changes from 6 commits
4cfd6cd
19b7cc2
0a0f55b
3afe8a5
b2c3597
dd8da96
f83c659
f8243b5
de2930f
575ec58
ba6212e
5e347c5
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.
ErronoInval is for invalid arguments right? Is this the correct error for here?
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 would recommend take a look at spec and also WASI-libc and see how other impl(e.g. Wasm time) handle errors
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.
Wasmer return ErrnoIO for this case
https://github.com/wasmerio/wasmer/blob/53c2c29bed1cf1fee8c5616632ee8c2bccc27880/lib/wasi/src/syscalls/mod.rs#L2541
wasi-libc just cast error fron syscal, and it can return bunch of them
https://man7.org/linux/man-pages/man2/getrandom.2.html
wasmtime also maps syscal error into Errno
https://github.com/bytecodealliance/wasmtime/blob/853a025613e012c6c29002ddcccfced67073a8d0/crates/wasi-common/src/snapshots/preview_1.rs#L111
Used
ErrnoIO
, added TODO in sourceThere 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.
FYI for bad memory offsets, we discussed offline to use ErrnoFault. Main thing is to document what is used and be consistent even if no one else is :D
Later, people can argue validly with these choices and probably we need to raise an issue on the wasi spec as it is underspecified. Main thing now is to document what you do.
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.
@codefromthecrypt thanks for info.
So, for invalid
buf
,bufLen
return error isErrnoFault
. Am I getting right?And how to deal with errors from random source ?
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 think ErrnoIO for the source is fine for now. we can change it easily later if we find a most standard code
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 found the issue that has been punted quite a while so far WebAssembly/WASI#215
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.
Error codes stuff gets you confused even more in functions that a more compilated than
random_get
. Spec need to address 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.
totally agree. appreciate if you can make your case on the spec issue because it was 2 years already so somehow they aren't prioritizing. maybe if more scream they will 🤷