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

Timespec #475

Merged
merged 5 commits into from
Dec 2, 2016
Merged

Timespec #475

merged 5 commits into from
Dec 2, 2016

Conversation

asomers
Copy link
Member

@asomers asomers commented Nov 20, 2016

Make TimeVal an opaque Newtype, and add another Newtype for Timespec.

By exposing its members, nix allowed users to create denormalized
TimeVals, which don't work with the derived() Eq and Ord methods.
Better to make TimeVal opaque, so it will always be normalized.
Also, add trait TimeValLike, so some code can be shared between TimeSpec
and TimeVal.
Copy link
Contributor

@fiveop fiveop left a comment

Choose a reason for hiding this comment

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

Do you plan on adding anything that uses TimeSpec? Right now, it seems a bit useless.

And the test_select is broken on i686 builds (except for Rust 1.7 for some reason)

Regarding Type(..) newtypes: iirc we once discussed using Type { field: libctype } over Type(libctype), but I cannot remember the reason. Right now I cannot think of one, and would prefer Type(libctype) in general (because it's simpler :)).

Otherwise looks good.

@asomers
Copy link
Member Author

asomers commented Nov 22, 2016

@fiveop Yes, I have another WIP branch that adds POSIX AIO support, and aio_suspend uses a timespec timeout. I'm aware of the i686 failures, and I'll fix them as soon as I get a working i686 VM.

Also, fix the TimeSpec::cmp and TimeVal::cmp methods, and fix some
formatting
@fiveop
Copy link
Contributor

fiveop commented Dec 2, 2016

Looks good to me (except for missing documentation).

@homu r+

@homu
Copy link
Contributor

homu commented Dec 2, 2016

📌 Commit 24eefcc has been approved by fiveop

@homu
Copy link
Contributor

homu commented Dec 2, 2016

⚡ Test exempted - status

@homu homu merged commit 24eefcc into nix-rust:master Dec 2, 2016
homu added a commit that referenced this pull request Dec 2, 2016
Timespec

Make TimeVal an opaque Newtype, and add another Newtype for Timespec.
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.

3 participants