Add clock_getres(3) and improve clockid_t constants.

[nodakai]

This is still a draft

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[nodakai]

@nbaksalyar It would be great if you could check the Solaris part. I don't quite understand what's available as clockid_t and what is not on Solaris.

Memo: checker tool

use std::time::Instant;

#[allow(non_camel_case_types)]
type clockid_t = i32;

extern {
    fn clock_gettime(clkid: clockid_t, t: *mut Instant) -> i32;
    fn clock_getres(clkid: clockid_t, t: *mut Instant) -> i32;
}

fn f(clkid: clockid_t) {
    let mut res = Instant::now();
    let mut now0 = Instant::now();
    let mut now1 = Instant::now();
    assert_eq!(0, unsafe { clock_getres(clkid, &mut res as *mut _) } );
    assert_eq!(0, unsafe { clock_gettime(clkid, &mut now0 as *mut _) } );
    assert_eq!(0, unsafe { clock_gettime(clkid, &mut now1 as *mut _) } );

    println!("{}: {:?} {:?} {:?} {:?}", clkid, res, now0, now1, now1 - now0);
}

fn main() {
    f(0);
    f(1);
    f(2);
    f(3);
}

[alexcrichton]

We actually need to avoid enum as it's basically pretty easy to violate memory safety in the FFI case (as FFI can return any value). Perhaps the can all be pub const instead?

[alexcrichton]

You can leave out the documentation in this library as these are all just straight bindings to libc, no need to document how to use each and every one

[nodakai]

@alexcrichton

it's basically pretty easy to violate memory safety in the FFI case (as FFI can return any value)

Can you elaborate? How can const X: i32 contribute to safety/robustness in a way that #[repr(i32)] enum E { X ... } cannot?

[alexcrichton]

If a C function returns an enum then it can actually return any integer. In Rust it's not memory safe to have a value of type T where T is an enum and the value is outside the range.

[nodakai]

Right, I had only passing an enum to C API in mind, like in this case of clockid_t which is returned from no known C API except OpenBSD clock_getcpuclockid(3). Do you think there's still a large risk here?

[alexcrichton]

Yeah unfortunately there's still the problems of:

• This is a breaking change as the type accepted is no longer i32 but an emum
• This isn't forward compatible if new clock ids are added over time because crates can exhaustively match the Rust enum

[nodakai]

if new clock ids are added over time

Well, then a downstream user should stop and think how to deal with the newly added case, so isn't breakage rather desirable? I'd say it's only propagating a semantic incompatibility brought by the OS.

Anyways, you're telling me this project should be a literally "raw bindings" to the system libc (and such considerations should perhaps be addressed by another, high-level wrapper of libc.) Fair enough, I reverted them back to plain int pub consts.

Any comments on constants that might not exist in older kernels? Of course an user is expected to check the return value of APIs, but I just want to know about the general principles for handling backward-incompatibilities if there are any.

[nodakai]

More than half of Travis failures came from the style checker (on comment lines longer than 80 chars in length,) BUT others indeed highlighted a compatibility issue. For example, the aarch64 cross tools shipped with Trusty/14.04 seems to lack CLOCK_TAI

• https://travis-ci.org/rust-lang/libc/jobs/119964887#L447

when the native build tools had no issues with it:

• https://travis-ci.org/rust-lang/libc/jobs/119964881

@alexcrichton Any suggestions? It's true that few people will complain when CLOCK_TAI is missing from the libc crate.

[alexcrichton]

Anyways, you're telling me this project should be a literally "raw bindings" to the system libc

Yes

Any comments on constants that might not exist in older kernels?

It's the job of consumers of libc to handle this, we simply bind everything and assume the kernel retains ABI compatibility with older releases.

It's true that few people will complain when CLOCK_TAI is missing from the libc crate.

It's fine to leave out this constant on aarch64 for now. We may just need to update the aarch64 glibc or something like that

[posborne]

Anyways, you're telling me this project should be a literally "raw bindings" to the system libc (and such considerations should perhaps be addressed by another, high-level wrapper of libc.)

Hi @nodakai. Providing greater safety on top of the libc APIs is the primary goal of the nix project (https://github.com/nix-rust/nix). We would welcome changes there as well once the core bits have made it into libc. Nix is the core behind a number of libraries such as mio (at least on *nix).

CC @kamalmarhubi

[nodakai]

@posborne Thanks, I'll consider merging some of my private binding to it

@alexcrichton Now all Travis failures come from missing CLOCK_TAI (as well as similar but much less relevant CLOCK_SGI_CYCLE) which requires glibc 2.21 and Linux 3.10 or later.

[kamalmarhubi]

typo: obsoleted

[kamalmarhubi]

@nodakai just to set expectations, we're still figuring out how to handle enum-style constants in nix (nix-rust/nix#254). We'd definitely welcome experiences and ideas on how to to that though, so a PR would be quite welcome!

Also thanks for adding the new stuff in libc. I was unaware of CLOCK_TAI on Linux. It appears its in <linux/time.h> and not in <time.h>, which brings to mind #235.

[alexcrichton]

Perhaps we can just leave out the constants for platforms that don't have it yet? I'd also remove the comments here as well as the duplication across the source isn't buying us much and this library isn't really intended to be read for documentation.

[nodakai]

@alexcrichton Moved CLOCK_TAI to musl/mod.rs though it should in principle be as portable as Linux kernel is. Also removed doc comments.

[alexcrichton]

@bors: r+ 7e752a3

[bors]

⌛ Testing commit 7e752a3 with merge 331d705...

[bors]

☀️ Test successful - status-appveyor, travis