liutao-liu opened issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L339
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
p
is not a reference of self.mode.
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L339
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
p
is not a reference of self.mode.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L339
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
p
is not a reference of self.mode.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
```[tasklist]
Tasks
~~~
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L339
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
p
is not a reference of self.mode.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
self.mode
is not a reference of postion, which is value pass inwrite_via_stream
.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
alexcrichton commented on issue #8257:
Could you clarify with a test case what the expected behavior is? Reviewing these mutations I don't see anything amiss myself, but a test case could help understand if there's a bug here
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
self.mode
is not a reference of postion in fd_write , which is passed by value in`write_via_stream
` .Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream` .Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu edited issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu commented on issue #8257:
Could you clarify with a test case what the expected behavior is? Reviewing these mutations I don't see anything amiss myself, but a test case could help understand if there's a bug here
Delete the modification in here All test case can run success. The reason is what I said before:
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream.![image](https://github.com/bytecodealliance/wasmtime/assets/10509166/b2d5fd02-8e3c-4273-8333-c1fe88432862)
liutao-liu edited a comment on issue #8257:
Could you clarify with a test case what the expected behavior is? Reviewing these mutations I don't see anything amiss myself, but a test case could help understand if there's a bug here
Delete the modification in here All test case can run success:
![image](https://github.com/bytecodealliance/wasmtime/assets/10509166/b2d5fd02-8e3c-4273-8333-c1fe88432862)The reason is what I said before:
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream:
![Uploading image.png…]()
liutao-liu edited a comment on issue #8257:
Could you clarify with a test case what the expected behavior is? Reviewing these mutations I don't see anything amiss myself, but a test case could help understand if there's a bug here
Delete the modification in here All test case can run success:
![image](https://github.com/bytecodealliance/wasmtime/assets/10509166/b2d5fd02-8e3c-4273-8333-c1fe88432862)The reason is what I said before:
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream:
![image](https://github.com/bytecodealliance/wasmtime/assets/10509166/49819e93-765e-47dc-a970-eb9dd9afbc08)
alexcrichton commented on issue #8257:
Sorry yeah our test suite for preview2 is not overly comprehensive. I've added a test in https://github.com/bytecodealliance/wasmtime/pull/8271 which fails if the code you're linking to fails.
It's a bit subtle and it needs a few different things in play, but I believe that this code is still necessary. Can you elaborate more on why you're looking to see this deleted?
liutao-liu commented on issue #8257:
The postion has been modified in
fd_write
(fd_read
is the same):
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/preview1.rs#L1705
The implementation offd_write
is as follows: first callwrite_via_stream
, then callblocking_write_and_flush
, and finally modify theposition
.
The test case you added in #8271: callwrite_via_stream
once, then callblocking_write_and_flush
twice.
liutao-liu edited a comment on issue #8257:
The postion has been modified in
fd_write
(fd_read
is the same):
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/preview1.rs#L1705
The implementation offd_write
is as follows: first callwrite_via_stream
, then callblocking_write_and_flush
, and finally modify theposition
.
The test case you added in #8271: callwrite_via_stream
once, then callblocking_write_and_flush
twice, so, in this case, must change the position inblocking_write_and_flush
.
alexcrichton commented on issue #8257:
You'll need to remember that there's a few layers here. The test I added in #8271 specifically uses preview2-based APIs and does not use preview1. You're correct that in a world where preview2 does not exist removing the write is ok, but in a world where preview2 exists removing the write is not correct.
I've asked a few times already, but can you clarify why you'd like this write removed? Is it causing performance problems or something like that? Or are you instead trying to understand why it's needed? (whatever's fine, I'm just curious!)
liutao-liu closed issue #8257:
Two modifications to the
p
code:
https://github.com/bytecodealliance/wasmtime/blob/9c92881d3b936b5d2e593ded370609a6e31717d8/crates/wasi/src/filesystem.rs#L359The former is an invalid modification, because
self.mode
is not a reference of postion in fd_write , which is passed by value in write_via_stream.Please help confirm if this is a bug. If it is, I'll submit a pr with my local changes.
liutao-liu commented on issue #8257:
You'll need to remember that there's a few layers here. The test I added in #8271 specifically uses preview2-based APIs and does not use preview1. You're correct that in a world where preview2 does not exist removing the write is ok, but in a world where preview2 exists removing the write is not correct.
I've asked a few times already, but can you clarify why you'd like this write removed? Is it causing performance problems or something like that? Or are you instead trying to understand why it's needed? (whatever's fine, I'm just curious!)
Sorry, I wasn't thinking from preview2's perspective. I see what you mean, thanks for your reply.
Last updated: Dec 23 2024 at 12:05 UTC