add safe libc::clock_nanosleep wrapper #1315
Conversation
|
|
|
this PR is waiting on the next minor release of libc |
looks like libc 0.2.80 was cut a couple of weeks ago, and includes rust-lang/libc#1922. could you bump the dependency version in |
It seems to me that the bump is already part of the PR: The libc dependency has been updated in master since then, so this needs a rebase. |
|
The new problem with this PR is that |
Actually, that's not a problem. For one thing, Rust's FFI bindings don't know whether a function is present at build time. And they don't care whether it's present at runtime unless somebody tries to use it. For another thing, FreeBSD 11.0 is EoL. We shouldn't worry about it anymore. Long Live FreeBSD 11.4. |
|
Are there any updates to this? |
|
Is it realistic that there will be progress here? |
|
Gentle ping on the author @maxbla, are you still interested in finishing this PR? If not, I think I will pick it up add it to Nix:) |
There was a problem hiding this comment.
I have left some comments on what needs to be changed if you want to finish this, and a CHANGELOG entry is needed, please see CONTRIBUTING.md on how to add one.
| bitflags! { | ||
| /// Flags that are used for arming the timer. | ||
| pub struct ClockNanosleepFlags: libc::c_int { | ||
| const TIMER_ABSTIME = libc::TIMER_ABSTIME; | ||
| } | ||
| } |
There was a problem hiding this comment.
We should use the libc_flags macro instead:
| bitflags! { | |
| /// Flags that are used for arming the timer. | |
| pub struct ClockNanosleepFlags: libc::c_int { | |
| const TIMER_ABSTIME = libc::TIMER_ABSTIME; | |
| } | |
| } | |
| libc_bitflags! { | |
| /// Flags that are used for arming the timer. | |
| pub struct ClockNanosleepFlags: libc::c_int { | |
| TIMER_ABSTIME; | |
| } | |
| } |
There was a problem hiding this comment.
And, adding a doc comment for this flag would be great:)
| #[cfg(any( | ||
| target_os = "freebsd", | ||
| target_os = "netbsd", | ||
| target_os = "linux", | ||
| target_os = "android", | ||
| target_os = "solaris", | ||
| target_os = "illumos", | ||
| ))] |
There was a problem hiding this comment.
We use cfg aliases now:
| #[cfg(any( | |
| target_os = "freebsd", | |
| target_os = "netbsd", | |
| target_os = "linux", | |
| target_os = "android", | |
| target_os = "solaris", | |
| target_os = "illumos", | |
| ))] | |
| #[cfg(any( | |
| linux_android, | |
| solarish, | |
| target_os = "freebsd", | |
| target_os = "netbsd" | |
| ))] |
| if ret == 0 { | ||
| Ok(rmtp) | ||
| } else { | ||
| Err(Error::Sys(Errno::from_i32(ret))) |
There was a problem hiding this comment.
| Err(Error::Sys(Errno::from_i32(ret))) | |
| Err(Errno::from_i32(ret)) |
Fixes #1299