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 test for pwrite with append flag #95

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Add test for pwrite with append flag #95

merged 2 commits into from
Dec 6, 2023

Conversation

yagehu
Copy link
Contributor

@yagehu yagehu commented Dec 4, 2023

WASI specifies fd_pwrite to be similar to POSIX pwritev which specifies that it should ignore the append flag and write from the supplied offset. Currently, only Wasmtime implements this behavior.

NB: There is a documented Linux bug that pwrite does not respect the offset and instead appends. Wasmtime fixes this by tracking the append flag status and hiding it from the OS.

@yagehu
Copy link
Contributor Author

yagehu commented Dec 5, 2023

I've filed an issue with the WasmEdge team. They have expressed desire to bring WasmEdge behavior in line with Wasmtime and add this test case.

WASI specifies `fd_pwrite` to be similar to [POSIX `pwritev`][1] which
specifies that it should ignore the append flag and write from the
supplied offset.  Currently, only Wasmtime implements this behavior.

NB: There is a documented [Linux bug][2] that pwrite does not respect
the offset and instead appends.  Wasmtime fixes this by tracking the
append flag status and hiding it from the OS.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html
[2]: https://linux.die.net/man/2/pwrite
@loganek
Copy link
Collaborator

loganek commented Dec 6, 2023

Thanks, the test looks reasonable. Could you please fix formatting problems reported by the build?

@yagehu
Copy link
Contributor Author

yagehu commented Dec 6, 2023

@loganek Thanks! Fixed format.

@loganek loganek merged commit ec55cdc into WebAssembly:main Dec 6, 2023
11 checks passed
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