-
Notifications
You must be signed in to change notification settings - Fork 250
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
adds experimental sys.Errno to begin decoupling from the syscall package #1582
Conversation
internal/fsapi/syscall_errno.go
Outdated
|
||
import "syscall" | ||
|
||
func syscallToErrno(errno syscall.Errno) Errno { |
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.
in the future, we can make a build constraint on this for GOOS that don't have syscall.Errno. I really don't know which that is.
d8ba691
to
3a0001c
Compare
This cherry-picks errors we use in our wasip1 implementation as `fsapi.Errno`. This is to start progress towards compiling on all GOOS values (such as plan9), to make future changes a lot easier. See #1578 Signed-off-by: Adrian Cole <adrian@tetrate.io>
4c5d25f
to
e78a609
Compare
// See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html | ||
type Errno uint16 | ||
|
||
// ^-- Note: This will eventually move to the public /sys package. It is |
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.
Here's the note on why, but TL;DR; we already have an internal/sys package but we need one like the public sys one (shared symbols regardless of filesystem, socket, proc, etc.) By placing this into experimental, we can use the same package name and in a lot of cases refer to these errors as sys.Errno
and especially in case of docs, this name will prevent drift mistakes, as eventually it will be the public package sys
@@ -50,7 +51,7 @@ var sockRecv = newHostFunc( | |||
"fd", "ri_data", "ri_data_len", "ri_flags", "result.ro_datalen", "result.ro_flags", | |||
) | |||
|
|||
func sockRecvFn(_ context.Context, mod api.Module, params []uint64) syscall.Errno { | |||
func sockRecvFn(_ context.Context, mod api.Module, params []uint64) sys.Errno { |
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 noticed we haven't audited POSIX, wasi-libc and wasix to figure out all the errno possibilities these should return. I'll do that afterwards. Particularly our errno should have enough values to support outbound sockets like https://github.com/stealthrocket/net
OTOH we shouldn't go too crazy on adding every definable one, as it makes a lot of work for those who aren't using posix systems. We should only add ones our apis say are returnable iotw, at least until our APIs are stable. The, as these are added, it is easier in PR review to ask "did you test this? where is this consumed in wasip1, wasix etc?" This question is harder when every errno is predefined.
@@ -263,47 +264,47 @@ var errnoToString = [...]string{ | |||
// Note: Coercion isn't centralized in sys.FSContext because ABI use different | |||
// error codes. For example, wasi-filesystem and GOOS=js don't map to these | |||
// Errno. | |||
func ToErrno(errno syscall.Errno) Errno { | |||
func ToErrno(errno sys.Errno) Errno { |
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.
This file as all the errnos we currently use. This plus the ones in GOOS=js were the ones defined so far.
This cherry-picks errors we use in our wasip1 implementation as
fsapi.Errno
. This is to start progress towards compiling on all GOOS values (such as plan9), to make future changes a lot easier.See #1578