Skip to content
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

Add serdes API #69

Closed
cjihrig opened this issue Dec 28, 2019 · 2 comments · Fixed by #126
Closed

Add serdes API #69

cjihrig opened this issue Dec 28, 2019 · 2 comments · Fixed by #126
Labels
help wanted Extra attention is needed

Comments

@cjihrig
Copy link
Collaborator

cjihrig commented Dec 28, 2019

It would be useful if embedders didn't need to worry about serializing and deserializing between WASI data structures and the WASM memory. For example, Node.js currently does this:

uvwasi_fdstat_t stats;
uvwasi_errno_t err = uvwasi_fd_fdstat_get(&wasi->uvw_, fd, &stats);

if (err == UVWASI_ESUCCESS) {
  wasi->writeUInt8(memory, stats.fs_filetype, buf);
  wasi->writeUInt16(memory, stats.fs_flags, buf + 2);
  wasi->writeUInt64(memory, stats.fs_rights_base, buf + 8);
  wasi->writeUInt64(memory, stats.fs_rights_inheriting, buf + 16);
}

The wasi->write*() calls could be replaced with a uvwasi_serdes_write_fdstat_t(memory, stats, buf).

Another option could be to provide wrappers for each function (uvwasi_serdes_fd_fdstat_get(), etc.). This would take care of bounds checking, reading from memory, executing the WASI API calls, writing back to memory, and forwarding the correct return value. This would be the simplest thing for embedders, but would also require a wrapper function for every WASI API call.

Potentially useful reference: https://github.com/WebAssembly/tool-conventions/blob/master/BasicCABI.md

Any thoughts or opinions from others?

@tniessen
Copy link
Member

I think the first option sounds better to me, simply because we won't need an additional layer for every single call. Having uvwasi_serdes_write_* seems like a good idea!

tniessen added a commit to tniessen/uvwasi that referenced this issue Jan 25, 2020
@tniessen tniessen mentioned this issue Jan 25, 2020
1 task
tniessen added a commit to tniessen/uvwasi that referenced this issue Jan 25, 2020
cjihrig pushed a commit that referenced this issue May 14, 2020
This commit adds serialization and deserialization functions
for converting between WASI data structures and WASM memory.

Refs: #69
PR-URL: #94
cjihrig added a commit that referenced this issue May 14, 2020
cjihrig added a commit that referenced this issue May 14, 2020
UVWASIS --> UVWASI

Refs: #69
Refs: #94
@cjihrig
Copy link
Collaborator Author

cjihrig commented May 14, 2020

#94 has landed, but I'm leaving this open, as there are still a few things I'd like to address in the new serdes API:

  1. It looks like add serdes API #94 didn't include support for uvwasi_ciovec_t and uvwasi_iovec_t.
  2. It would be nice to support serializing/deserializing arrays, which ends up overlapping a lot with item 1.

@cjihrig cjihrig added the help wanted Extra attention is needed label May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants