Fix filedelta type in fd_seek #319
Conversation
src/wasi/WASI.cpp
Outdated
| @@ -263,7 +263,7 @@ void WASI::fd_prestat_dir_name(ExecutionState& state, Value* argv, Value* result | |||
| void WASI::fd_seek(ExecutionState& state, Value* argv, Value* result, Instance* instance) | |||
| { | |||
| uint32_t fd = argv[0].asI32(); | |||
| uint64_t fileDelta = argv[1].asI32(); | |||
| uint64_t fileDelta = argv[1].asI64(); | |||
There was a problem hiding this comment.
This patch looks so simple, is this done?
If so, could you add a TC to verify this?
There was a problem hiding this comment.
It's just a typo fix, thats why it is simple, but I will add a test to verify it then.
Also according to the wasi specs it should be signed 64 bit, so I also missedtyped it. I will be correcting that.
fd_seek documentation
There was a problem hiding this comment.
@clover2123 I have been thinking about how I should write a proper test case for this. Filedelta describes the amount of bytes fd_seek should jump from the location that depends on whence.
To properly test it we would need a signed int32 maximum + 1 amount of bytes in a file, which is quite large, around 2.2 gigabytes and I am concerned about the size. (Please tell me if my math is wrong, it may be I just misscalculated.)
Fd_seek is based on the posix lseek which also uses a signed 64 bit value so I think its safe to say that a test is not that necessary I belive, though I could make a tests that generates a file that big, but I dont think it should be part of the CI.
There was a problem hiding this comment.
Does this do a type check? I think WebAssembly is strongly typed, so it should enforce a 64 bit value if the argument description of the function is correct.
There was a problem hiding this comment.
@zherczeg asI64() checks the type in a debug mode only as far as I know. The argument description is correct in WASI.h but I dont think it actually enforces it to be a 64 bit value and as I have seen uvwasi does not actually check if it is the right type.
There was a problem hiding this comment.
When we find the wasi functions, we should also validate that the interface is the same as expected. This could be a follow-up work.
Signed-off-by: Ádám Kulcsár <[email protected]>
kulcsaradam
force-pushed
the
fd_seek_fix
branch
from
d23fea5 to
9c73270
Compare
No description provided.