From ddbb57d355835b12eac72ae4150ed81348aba6f8 Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <dverkamp@chromium.org>
Date: Tue, 17 May 2022 16:49:15 -0700
Subject: [PATCH] base: unix: timer: factor out timerfd_settime call

Deduplicate the unsafe block and the update of interval into a helper
function so that reset() and clear() can be greatly simplified.

BUG=None
TEST=tools/presubmit --all

Change-Id: Ic0210bc3dd2239b575d47f718709333bce842509
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3653256
Reviewed-by: Anton Romanov <romanton@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
---
 base/src/sys/unix/timer.rs | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/base/src/sys/unix/timer.rs b/base/src/sys/unix/timer.rs
index 2cd1cb4938..154aa8d0d0 100644
--- a/base/src/sys/unix/timer.rs
+++ b/base/src/sys/unix/timer.rs
@@ -43,17 +43,15 @@ impl Timer {
         })
     }
 
-    /// Sets the timer to expire after `dur`.  If `interval` is not `None` it represents
-    /// the period for repeated expirations after the initial expiration.  Otherwise
-    /// the timer will expire just once.  Cancels any existing duration and repeating interval.
-    pub fn reset(&mut self, dur: Duration, interval: Option<Duration>) -> Result<()> {
+    // Calls `timerfd_settime()` and stores the new value of `interval`.
+    fn set_time(&mut self, dur: Option<Duration>, interval: Option<Duration>) -> Result<()> {
         // The posix implementation of timer does not need self.interval, but we
         // save it anyways to keep a consistent interface.
         self.interval = interval;
 
         let spec = libc::itimerspec {
             it_interval: duration_to_timespec(interval.unwrap_or_default()),
-            it_value: duration_to_timespec(dur),
+            it_value: duration_to_timespec(dur.unwrap_or_default()),
         };
 
         // Safe because this doesn't modify any memory and we check the return value.
@@ -65,6 +63,18 @@ impl Timer {
         Ok(())
     }
 
+    /// Sets the timer to expire after `dur`.  If `interval` is not `None` it represents
+    /// the period for repeated expirations after the initial expiration.  Otherwise
+    /// the timer will expire just once.  Cancels any existing duration and repeating interval.
+    pub fn reset(&mut self, dur: Duration, interval: Option<Duration>) -> Result<()> {
+        self.set_time(Some(dur), interval)
+    }
+
+    /// Disarms the timer.
+    pub fn clear(&mut self) -> Result<()> {
+        self.set_time(None, None)
+    }
+
     /// Waits until the timer expires or an optional wait timeout expires, whichever happens first.
     ///
     /// # Returns
@@ -154,22 +164,6 @@ impl Timer {
         }
     }
 
-    /// Disarms the timer.
-    pub fn clear(&mut self) -> Result<()> {
-        // Safe because we are zero-initializing a struct with only primitive member fields.
-        let spec: libc::itimerspec = unsafe { mem::zeroed() };
-
-        // Safe because this doesn't modify any memory and we check the return value.
-        let ret = unsafe { timerfd_settime(self.as_raw_descriptor(), 0, &spec, ptr::null_mut()) };
-        if ret < 0 {
-            return errno_result();
-        }
-
-        self.interval = None;
-
-        Ok(())
-    }
-
     /// Returns the resolution of timers on the host.
     pub fn resolution() -> Result<Duration> {
         // Safe because we are zero-initializing a struct with only primitive member fields.