Skip to content

Conversation

@achan1989
Copy link
Contributor

Fixes #14324

No testing performed. Original issue says "you can see this plainly in the source" -- agreed.

Copy link
Member

@squeek502 squeek502 left a comment

Choose a reason for hiding this comment

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

It might be nice to include comments with quotes from the documentation.

@achan1989
Copy link
Contributor Author

It might be nice to include comments with quotes from the documentation.

Rather than annotate each of the call sites, I could instead add the documentation once to the rights_t struct in std/os/wasi.zig ? If so, I'll do the rest of the fields too.

@squeek502
Copy link
Member

squeek502 commented Oct 6, 2024

The only intention I had with the inline comments is to note that POLL_FD_READWRITE is contingent on the FD_WRITE/FD_READ rights, i.e. it only grants an extra read-related privilege when FD_READ is also set, and it only grants an extra write-related privilege when FD_WRITE is set. Just seems like the sort of info that would be useful to have when looking at this bit of code in the future.

Not super important, though, and doc comments in rights_t might work just as well.

@andrewrk andrewrk merged commit d5c9d85 into ziglang:master Oct 6, 2024
10 checks passed
@andrewrk
Copy link
Member

andrewrk commented Oct 6, 2024

Thanks.

@achan1989 achan1989 deleted the issue_14324 branch October 7, 2024 07:38
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.

std: std.fs.Dir.createFile on WASI targets should include POLL_FD_READWRITE right by default

3 participants