Skip to content

Implement fd_fdstat_set_flags for Windows. - #711

Merged
sunfishcode merged 3 commits into
bytecodealliance:masterfrom
peterhuene:implement-fdstat-set-flags
Jan 10, 2020
Merged

Implement fd_fdstat_set_flags for Windows.#711
sunfishcode merged 3 commits into
bytecodealliance:masterfrom
peterhuene:implement-fdstat-set-flags

Conversation

@peterhuene

@peterhuenepeterhuene commented Dec 13, 2019•
edited
Loading

Copy link
Copy Markdown
Member

This PR implements fd_fdstat_set_flags for Windows.

Additionally, it fixes a problem where O_APPEND was not working correctly
because GENERIC_WRITE was always being set; as a result, FILE_WRITE_DATA
could not be removed from the permission set to properly enable append-only
mode.

It also treats O_TRUNC with O_APPEND as an invalid argument error. This is
because Windows cannot support these two flags together. To support O_TRUNC,
the GENERIC_WRITE bit must be set for the file access flags. Setting this
bit will cause FILE_WRITE_DATA to be set, which will not properly treat the
file as append-only (it requires FILE_APPEND_DATA without FILE_WRITE_DATA).

@peterhuene

peterhuene commented Dec 13, 2019

Copy link
Copy Markdown
MemberAuthor

@marmistrz for review too please :)

@peterhuenepeterhuene added the wasi:impl Issues pertaining to WASI implementation in Wasmtime label Dec 13, 2019
@peterhuene
peterhueneforce-pushed the implement-fdstat-set-flags branch from 75e8130 to 9fdc356CompareDecember 13, 2019 06:00
marmistrz
marmistrz suggested changes Dec 15, 2019
View reviewed changes
Comment threadcrates/test-programs/wasi-tests/src/bin/fd_flags_set.rs
Comment threadcrates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs Outdated
Comment threadcrates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs
Comment threadcrates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs Outdated
Comment threadcrates/wasi-common/src/hostcalls_impl/fs.rs Outdated
Comment threadcrates/wasi-common/winx/src/file.rs
Comment threadcrates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs Outdated
Comment threadcrates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs Outdated
@peterhuenepeterhuene mentioned this pull request Dec 16, 2019
Closed
kubkon
kubkon approved these changes Dec 16, 2019
View reviewed changes

@kubkonkubkon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@peterhuene
peterhueneforce-pushed the implement-fdstat-set-flags branch 2 times, most recently from 96592d3 to 5614c43CompareJanuary 9, 2020 22:37
@peterhuene
peterhuene requested a review from kubkonJanuary 9, 2020 22:39
Peter Huene added 3 commits January 9, 2020 14:52
Peter Huene
Implement fd_fdstat_set_flags for Windows.
b723a62
This commit implements `fd_fdstat_set_flags` for Windows.
Additionally, it fixes a problem where `O_APPEND` was not working correctly
because `GENERIC_WRITE` was always being set; as a result, `FILE_WRITE_DATA`
could not be removed from the permission set to properly enable append-only
mode.
It also treats `O_TRUNC` with `O_APPEND` as an invalid argument error. This is
because Windows cannot support these two flags together. To support `O_TRUNC`,
the `GENERIC_WRITE` bit must be set for the file access flags. Setting this
bit will cause `FILE_WRITE_DATA` to be set, which will not properly treat the
file as append-only (it requires `FILE_APPEND_DATA` without `FILE_WRITE_DATA`).
Peter Huene
Add the Cargo.lock for the test programs.
7aa59e2
Peter Huene
Additional PR feedback changes.
995c32c
* Add more comments.
* Use `contains` from bitflags.
* Format wasi-test source.
* Remove permission check from Windows `path_open` impl.
@peterhuene
peterhueneforce-pushed the implement-fdstat-set-flags branch from 5614c43 to 995c32cCompareJanuary 9, 2020 22:52
@sunfishcode
sunfishcode merged commit 4b7677e into bytecodealliance:masterJan 10, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunfishcodesunfishcodeAwaiting requested review from sunfishcode
@kubkonkubkonAwaiting requested review from kubkon
1 more reviewer
@marmistrzmarmistrzmarmistrz requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

wasi:implIssues pertaining to WASI implementation in Wasmtime

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@peterhuene@kubkon@marmistrz@sunfishcode