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

Change poll's timeout from c_int to Option<&Timespec>. #1285

Merged
merged 8 commits into from
Feb 1, 2025

Conversation

sunfishcode
Copy link
Member

This harmonizes the timeout with the rest of rustix, which uses Timespec for all time values. And, it eliminates the awkwardness of using -1 as a sentinel value.

On platforms with ppoll, the Timespec can be passed straight to the OS. On platforms without ppoll, we have to do a fallible conversion into c_int.

@sunfishcode sunfishcode mentioned this pull request Jan 27, 2025
21 tasks
This harmonizes the timeout with the rest of rustix, which uses
`Timespec` for all time values. And, it eliminates the awkwardness
of using `-1` as a sentinel value.

On platforms with `ppoll`, the `Timespec` can be passed straight
to the OS. On platforms without `ppoll`, we have to do a fallible
conversion into `c_int`.
@sunfishcode sunfishcode force-pushed the sunfishcode/poll-timeout branch from 80fbb61 to 5be3893 Compare January 28, 2025 00:14
@SUPERCILEX
Copy link
Contributor

Will take a look sometime this week.

}
secs.checked_mul(1000)
.and_then(|millis| {
millis.checked_add((i64::from(timeout.tv_nsec) + 999_999) / 1_000_000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment that says the purpose of this is to round up to the nearest millisecond?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet, this looks amazing!

@sunfishcode sunfishcode merged commit 40733be into main Feb 1, 2025
45 checks passed
@sunfishcode sunfishcode deleted the sunfishcode/poll-timeout branch February 1, 2025 21:37
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