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.

Sign up for GitHub

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

Jump to bottom

Support nullable timeout in ppoll #1270

Merged
merged 1 commit into from Jul 26, 2020
Merged

Support nullable timeout in ppoll #1270

merged 1 commit into from Jul 26, 2020

Conversation

MikailBag
Copy link
Contributor

No description provided.

asomers
asomers requested changes Jul 7, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

What does a timeout of None mean? Can you add that to the documentation? This will need a CHANGELOG entry too.

@MikailBag
Copy link
Contributor Author

As stated in Linux man:

If tmo_p is specified as NULL, then ppoll() can block indefinitely.

I updated documentation and changelog.

@MikailBag
Copy link
Contributor Author

Dragonfly man:

A null pointer may be passed to indicate that ppoll() should wait indefinitely.

FreeBSD man:

A null pointer may be passed to indicate that ppoll() should wait indefinitely.

asomers
asomers reviewed Jul 24, 2020
View reviewed changes
src/poll.rs Outdated Show resolved Hide resolved
@MikailBag MikailBag force-pushed the timeout-none branch 2 times, most recently from 6cd56ed to ef6d358 Compare July 25, 2020 21:24
@asomers
Copy link
Member

asomers commented Jul 26, 2020

@MikailBag that test failure looks relevant. Could you please investigate?

@MikailBag
Copy link
Contributor Author

Hmm, it seems that without let timeout = &timeout we had a dangling pointer (in match, t.as_ref() created temporary reference and converted it into temporary pointer). So it happened that timeout started pointing to some negative value, causing EINVAL.

I think now there is no UB 😄

@asomers
Copy link
Member

asomers commented Jul 26, 2020

Good. Can you squash it now?

@MikailBag
Copy link
Contributor Author

I've squashed.

asomers
asomers approved these changes Jul 26, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

LGTM!

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 26, 2020

Build succeeded:

@bors bors bot merged commit 2a668fa into nix-rust:master Jul 26, 2020
@MikailBag MikailBag deleted the timeout-none branch July 26, 2020 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MikailBag @asomers