Skip to content

Commit

Permalink
utils/clock: Use rustix, without unsafe; infallible
Browse files Browse the repository at this point in the history
Rustix's `lock_gettime` does not return an error, and notes it is
infallible, while `clock_gettime_dynamic` can handle some additional
clocks that may result in an error.

I don't think the clock helper in Smithay needs to handle any of those,
so returning a `Result` in `new` is unneeded.

This also uses an associated const for `ClockSource::ID`, so the clock
id doesn't need to be stored a runtime when it's already expressed in
the type. Const generics would be suitable, but don't work with enums
yet.
  • Loading branch information
ids1024 committed Oct 18, 2023
1 parent 028db93 commit 36ff703
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 44 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ libc = "0.2.103"
libseat = { version = "0.2.1", optional = true, default_features = false }
libloading = { version="0.8.0", optional = true }
nix = { version = "0.27.0" }
rustix = { version = "0.38.18", features = ["event", "fs", "mm", "net"] }
rustix = { version = "0.38.18", features = ["event", "fs", "mm", "net", "time"] }
once_cell = "1.8.0"
rand = "0.8.4"
scopeguard = { version = "1.1.0", optional = true }
Expand Down
2 changes: 1 addition & 1 deletion anvil/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl<BackendData: Backend + 'static> AnvilState<BackendData> {
) -> AnvilState<BackendData> {
let dh = display.handle();

let clock = Clock::new().expect("failed to initialize clock");
let clock = Clock::new();

// init wayland clients
let socket_name = if listen_on_socket {
Expand Down
2 changes: 1 addition & 1 deletion src/desktop/wayland/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl OutputPresentationFeedback {
{
let time = time.into();
let refresh = refresh.into();
let clk_id = Kind::id() as u32;
let clk_id = Kind::ID as u32;
if let Some(output) = self.output.upgrade() {
for mut callback in self.callbacks.drain(..) {
callback.presented(&output, clk_id, time, refresh, seq, flags);
Expand Down
58 changes: 17 additions & 41 deletions src/utils/clock.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{cmp::Ordering, marker::PhantomData, mem::MaybeUninit, time::Duration};
use rustix::time::{ClockId, Timespec};
use std::{cmp::Ordering, marker::PhantomData, time::Duration};

/// Marker for clock source that never returns a negative [`Time`]
pub trait NonNegativeClockSource: ClockSource {}
Expand All @@ -8,9 +9,7 @@ pub trait NonNegativeClockSource: ClockSource {}
pub struct Monotonic;

impl ClockSource for Monotonic {
fn id() -> libc::clockid_t {
libc::CLOCK_MONOTONIC
}
const ID: ClockId = ClockId::Monotonic;
}

impl NonNegativeClockSource for Monotonic {}
Expand All @@ -20,51 +19,41 @@ impl NonNegativeClockSource for Monotonic {}
pub struct Realtime;

impl ClockSource for Realtime {
fn id() -> libc::clockid_t {
libc::CLOCK_REALTIME
}
const ID: ClockId = ClockId::Realtime;
}

/// Id for a clock according to unix clockid_t
pub trait ClockSource {
/// Gets the id of the clock source
fn id() -> libc::clockid_t;
const ID: ClockId;
}

/// Defines a clock with a specific kind
#[derive(Debug)]
pub struct Clock<Kind> {
clk_id: libc::clockid_t,
pub struct Clock<Kind: ClockSource> {
_kind: PhantomData<Kind>,
}

impl<Kind: ClockSource> Clock<Kind> {
/// Initialize a new clock
pub fn new() -> std::io::Result<Self> {
let clk_id = Kind::id();
clock_get_time(clk_id)?;
Ok(Clock {
clk_id,
_kind: PhantomData,
})
pub fn new() -> Self {
Clock { _kind: PhantomData }
}

/// Returns the current time
pub fn now(&self) -> Time<Kind> {
clock_get_time(self.clk_id)
.expect("failed to get clock time")
.into()
rustix::time::clock_gettime(Kind::ID).into()
}

/// Gets the id of the clock
pub fn id(&self) -> libc::clockid_t {
Kind::id()
pub fn id(&self) -> ClockId {
Kind::ID
}
}

/// A point in time for a clock with a specific kind
pub struct Time<Kind> {
tp: libc::timespec,
tp: Timespec,
_kind: PhantomData<Kind>,
}

Expand Down Expand Up @@ -128,7 +117,7 @@ impl<Kind> Ord for Time<Kind> {

impl<Kind: NonNegativeClockSource> From<Duration> for Time<Kind> {
fn from(tp: Duration) -> Self {
let tp = libc::timespec {
let tp = Timespec {
tv_sec: tp.as_secs() as libc::time_t,
#[cfg(all(target_arch = "x86_64", target_pointer_width = "32"))]
tv_nsec: tp.subsec_nanos() as i64,
Expand All @@ -142,8 +131,8 @@ impl<Kind: NonNegativeClockSource> From<Duration> for Time<Kind> {
}
}

impl<Kind> From<libc::timespec> for Time<Kind> {
fn from(tp: libc::timespec) -> Self {
impl<Kind> From<Timespec> for Time<Kind> {
fn from(tp: Timespec) -> Self {
Time {
tp,
_kind: PhantomData,
Expand All @@ -157,7 +146,7 @@ const NANOS_PER_SEC: i64 = 1_000_000_000;
#[cfg(not(all(target_arch = "x86_64", target_pointer_width = "32")))]
const NANOS_PER_SEC: std::os::raw::c_long = 1_000_000_000;

fn saturating_sub_timespec(lhs: libc::timespec, rhs: libc::timespec) -> Option<Duration> {
fn saturating_sub_timespec(lhs: Timespec, rhs: Timespec) -> Option<Duration> {
if let Some(mut secs) = lhs.tv_sec.checked_sub(rhs.tv_sec) {
let nanos = if lhs.tv_nsec >= rhs.tv_nsec {
lhs.tv_nsec - rhs.tv_nsec
Expand All @@ -174,19 +163,6 @@ fn saturating_sub_timespec(lhs: libc::timespec, rhs: libc::timespec) -> Option<D
}
}

fn clock_get_time(clk_id: libc::clockid_t) -> Result<libc::timespec, std::io::Error> {
let mut tp = MaybeUninit::zeroed();
unsafe {
let res = libc::clock_gettime(clk_id, tp.as_mut_ptr());

if res < 0 {
return Err(std::io::Error::last_os_error());
}

Ok(tp.assume_init())
}
}

#[cfg(test)]
mod test {
use std::time::Duration;
Expand All @@ -195,7 +171,7 @@ mod test {

#[test]
fn monotonic() {
let clock_source: Clock<Monotonic> = Clock::new().unwrap();
let clock_source: Clock<Monotonic> = Clock::new();
let now = clock_source.now();
let zero = Time::<Monotonic>::from(Duration::ZERO);
assert_eq!(Time::<Monotonic>::elapsed(&zero, now), now.into());
Expand Down

0 comments on commit 36ff703

Please sign in to comment.