Skip to content

Rename errno constants to use the wasi values#9545

Merged
kripken merged 13 commits intoincomingfrom
renam
Sep 30, 2019
Merged

Rename errno constants to use the wasi values#9545
kripken merged 13 commits intoincomingfrom
renam

Conversation

@kripken
Copy link
Member

@kripken kripken commented Sep 30, 2019

Redefine errno values to be consistent with wasi. This will let us avoid
needing to convert the values back and forth as we use more wasi APIs (which
I experiemented with and it adds 1K or so).

This is an ABI change, which should not be noticeable from user code
unless you use errno defines (like EAGAIN) and keep around binaries
compiled with an older version that you link against. In that case, you
should rebuild them.

Fix NODEFS, which basically just routed the node errno code to ours, which
only worked when the underlying system had codes similar to musl - which seems
to have been the case on some linuxes. This does make NODEFS larger, as it
uses the ERRNO lookup table now, I'll look into a way to avoid including
it unnecessarily as a followup (would be a breaking change).

Most of the changes in this PR are test changes, places where we printed
raw errno codes.

ex = e;
}
assert(ex instanceof FS.ErrnoError && ex.errno === 2); // ENOENT
assert(ex instanceof FS.ErrnoError && ex.errno === 44); // ENOENT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the macros here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess the macros would work as this EM_ASM code, so the C preprocessor does run on it. May make sense as a followup, yeah.

errno: 2
X_OK(writeable): -1
errno: 13
errno: 2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it would be better if these tests use strerror() to print errno? No need to make that part of this PR now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, will leave that for a followup too.

@kripken kripken merged commit 09e9050 into incoming Sep 30, 2019
@kripken kripken deleted the renam branch September 30, 2019 17:24
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
Redefine errno values to be consistent with wasi. This will let us avoid
needing to convert the values back and forth as we use more wasi APIs (which
I experiemented with and it adds 1K or so).

This is an ABI change, which should not be noticeable from user code
unless you use errno defines (like EAGAIN) and keep around binaries
compiled with an older version that you link against. In that case, you
should rebuild them.

Fix NODEFS, which basically just routed the node errno code to ours, which
only worked when the underlying system had codes similar to musl - which seems
to have been the case on some linuxes. This does make NODEFS larger, as it
uses the ERRNO lookup table now, I'll look into a way to avoid including
it unnecessarily as a followup (would be a breaking change).

Most of the changes in this PR are test changes, places where we printed
raw errno codes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants