Skip to content

Rename PollAdd's "flags" arg to "poll_mask" and make it type-safe#12

Open
asayers wants to merge 3 commits into
jordanisaacs:mainfrom
asayers:poll-add
Open

Rename PollAdd's "flags" arg to "poll_mask" and make it type-safe#12
asayers wants to merge 3 commits into
jordanisaacs:mainfrom
asayers:poll-add

Conversation

@asayers

@asayers asayers commented Mar 29, 2025

Copy link
Copy Markdown
Contributor

The "io_uring_prep_poll_add" man page refers to this field as "poll_mask". I think it's worth following their naming, since io_uring_prep_poll_update() contains an unrelated field called "flags", in addition to "poll_mask". (This opcode is not yet supported by rustix-uring, but I imagine it might be in the future.)

@asayers

asayers commented Mar 29, 2025

Copy link
Copy Markdown
Contributor Author

I wonder if op_flags_union's poll_32 variant should also be typed as EventFlags?

@sunfishcode

Copy link
Copy Markdown
Collaborator

Sounds reasonable. It appears there's one test failure to look into though.

@sunfishcode

Copy link
Copy Markdown
Collaborator

Would you mind resolving the merge conflicts that have popped up in this PR?

asayers added 3 commits April 24, 2025 09:06
This it what the "io_uring_prep_poll_add" man page calls it.  I think
it's worth following their naming, since `io_uring_prep_poll_update()`
contains an unrelated field called "flags", in addition to "poll_mask".
(This opcode is not yet supported by rustix-uring, but I imagine it
might be in the future.)
@asayers

asayers commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

I wonder if op_flags_union's poll_32 variant should also be typed as EventFlags?

Ah, I didn't notice that this type lives in rustix::io_uring, so this would be a seperate PR.

@asayers

asayers commented May 4, 2025

Copy link
Copy Markdown
Contributor Author

I think this is ready to merge.

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