Stream: git-wasmtime

Topic: wasmtime / PR #2070 virtfs file: update cursor position o...

Wasmtime GitHub notifications bot (Jul 26 2020 at 13:07):

ueno opened PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 13:10):

tschneidereit requested kubkon for a review on PR #2070.

Wasmtime GitHub notifications bot (Jul 26 2020 at 13:10):

tschneidereit requested pchickey and kubkon for a review on PR #2070.

Wasmtime GitHub notifications bot (Jul 26 2020 at 13:23):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 13:42):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 17:16):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 18:42):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 19:37):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 26 2020 at 19:45):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 27 2020 at 04:00):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

Wasmtime GitHub notifications bot (Jul 27 2020 at 04:56):

ueno edited PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 05:00):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

Would you mind extracting this out into a temp variable such as let nread = ... and then doing an assertion assert_eq!(nread, buffer.len(), ...)? This hopefully should aid readability.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

Would you mind tweaking the message here to "seeking to the beginning of the file again" or something like this? This should help in debugging especially in the CI should we introduce a regression at some point in the future.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

In case types::Filesize ever changes its ABI from u64 into something different, would you mind using From trait to do the cast here: read_total.into() or u64::from(read_total)?

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

Like above, if you could tweak this to use the From trait instead of using an explicit as cast.

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

Is there any reason for this name change?

Wasmtime GitHub notifications bot (Jul 27 2020 at 08:03):

kubkon created PR Review Comment:

I think this should either be EFAULT or ENOMEM. The former seems to be accurate for Linux, while the latter for macOS (and probably BSDs in general). But to get another pair of eyes on this, @sunfishcode could you offer some advice here please?

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:03):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:08):

ueno submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:08):

ueno created PR Review Comment:

This is just for consistency; in other tests such as fd_flags_set.rs or file_truncate.rs, "buffer" (or "buf") is used as a name of mutable buffers, and immutable buffers are called "data" or "contents". As we call fd_read below and need to assign a name to the immutable buffer, I thought it might be less confusing to follow the convention.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:09):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:09):

kubkon created PR Review Comment:

Huh, I didn't know we had a convention for this, but this makes sense! I'll make sure that we keep it that way from now on. Thanks!

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:34):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:39):

ueno submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:39):

ueno created PR Review Comment:

Since there is no auto conversion from usize to u64, I added a temporary variable with type annotation and used try_into, but I suppose there is a simpler solution.

Btw, while doing that I also noticed fd_pwrite has the same issue, so I included a fix for that as well.

Wasmtime GitHub notifications bot (Jul 27 2020 at 09:54):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 10:25):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 11:00):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 11:00):

kubkon created PR Review Comment:

@ueno could you replace this as cast with TryInto as well here?

Wasmtime GitHub notifications bot (Jul 27 2020 at 11:52):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 12:59):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 15:06):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 15:06):

kubkon created PR Review Comment:

Would you mind linking in rust-lang/rust#74825 for posterity?

Wasmtime GitHub notifications bot (Jul 27 2020 at 15:06):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 15:06):

kubkon created PR Review Comment:

And here?

Wasmtime GitHub notifications bot (Jul 27 2020 at 15:13):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:04):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:04):

sunfishcode created PR Review Comment:

Looking at POSIX, this looks like more of an EINVAL situation than EFAULT.

https://pubs.opengroup.org/onlinepubs/009695399/functions/pread.html

EINVAL is for "the offset argument is not valid".

Wasmtime GitHub notifications bot (Jul 28 2020 at 06:06):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 28 2020 at 06:06):

ueno submitted PR Review.

Wasmtime GitHub notifications bot (Jul 28 2020 at 06:06):

ueno created PR Review Comment:

Thank you for the pointer; I've updated it to using EINVAL.

Wasmtime GitHub notifications bot (Jul 28 2020 at 06:50):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 28 2020 at 07:29):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 28 2020 at 09:18):

kubkon submitted PR Review.

Wasmtime GitHub notifications bot (Jul 28 2020 at 13:04):

ueno updated PR #2070 from wip/dueno/fd-read-virtfs to main:

If a handle is backed by InMemoryFile, fd_read (turned into Handle::read_vectored) doesn't update the cursor position properly and thus prevents the caller from detecting EOF.

This also fixes a similar issue in fd_pread, which doesn't advance the read offset when reading data into multiple iovecs.

Wasmtime GitHub notifications bot (Jul 29 2020 at 12:19):

kubkon merged PR #2070.

Last updated: Nov 22 2024 at 17:03 UTC