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

Weird epoll comment around close #1100

Open
SUPERCILEX opened this issue Aug 1, 2024 · 4 comments
Open

Weird epoll comment around close #1100

SUPERCILEX opened this issue Aug 1, 2024 · 4 comments

Comments

@SUPERCILEX
Copy link
Contributor

https://docs.rs/rustix/latest/rustix/event/epoll/fn.add.html says that closing a FD doesn't do anything which directly contradicts the man pages: https://man7.org/linux/man-pages/man7/epoll.7.html#:~:text=Will%20closing%20a%20file%20descriptor%20cause%20it%20to%20be%20removed%20from%20all%0A%20%20%20%20%20%20%20%20%20%20epoll%20interest%20lists%3F

@notgull is there context I'm missing or can we just remove that comment in favor of the man pages?

@SUPERCILEX
Copy link
Contributor Author

SUPERCILEX commented Aug 1, 2024

Hmmm, also confused about why EventVec is a thing. Shouldn't that just use the same API as https://docs.rs/rustix/latest/rustix/fs/fn.readlink.html? If you and @sunfishcode code are ok with that, I'd like to get rid of it. Also the event_list.events.set_len(0); is very suspicious—like yes everything is copy so whatever, but why not just call clear?

@SUPERCILEX
Copy link
Contributor Author

Actually the Into interface seems annoying since an &mut Vec<Event> would do. Though we can go a step further and take an &mut [MaybeUninit<Event>] and return an &mut [Event] so you can pass in stack arrays. The interface would look like this:

fn wait<I: Into<SliceOrVec>>(..., event_buf: I) -> io::Result<&mut [Event]>

enum SliceOrVec<'a> {
  Slice(&'a mut [MaybeUninit<Event>]),
  Vec(&'a mut Vec<Event>),
}

impl From slice/vec for SliceOrVec

// Usage
for event in wait(..., &mut stack OR vec.spare_capacity_mut()) {}

let mut vec = ...;
wait(..., vec.spare_capacity_mut());
do_stuff(&vec)

Thoughts? This seems way more flexible and less code than the current version.

@SUPERCILEX
Copy link
Contributor Author

Also I don't think the API should clear/set_len(0) the vec at all, that's the caller's problem (use drain) and they might want to only partially process events for some reason. I guess maybe it's fine? But seems weird to kill stuff in your buffer.

@SUPERCILEX
Copy link
Contributor Author

Ooo, pretty sure this is backwards compatible with a from impl for EventVec! So we could do all this in 0.38 with a deprecation and only drop EventVec later.

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

No branches or pull requests

1 participant