At C side, timer_mod() accepts the int64_t expire_time as an argument.

In fact, int64_t defines the positive value range accepted by
expire_timer is (0, INT64_MAX].

But at Rust side, Timer::modify() accepts an u64 expire_time and convert
it to i64 internally. This is a lazy approach, assuming no one would use
negative values to modify the timer. However, logically speaking, a
negative expire_time is acceptable - it means that the current timer
should fire immediately.

Moreover, specifying expire_time as i64 allows callers to more clearly
understand the accepted value range, preventing them from unexpectedly
finding a `u64` (> `i64::MAX`) silently converted to a negative value.

Make Timer::modify() accept an i64 expire_time.

Signed-off-by: Zhao Liu <[email protected]>
---
 rust/hw/timer/hpet/src/device.rs | 2 +-
 rust/util/src/timer.rs           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index 86638c076666..f0ac01758e72 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -365,7 +365,7 @@ fn arm_timer(&mut self, tick: u64) {
         }

         self.last = ns;
-        self.qemu_timer.modify(self.last);
+        self.qemu_timer.modify(self.last.try_into().unwrap());
     }

     fn set_timer(&mut self) {
diff --git a/rust/util/src/timer.rs b/rust/util/src/timer.rs
index c6b3e4088ecb..c3a9de6b72df 100644
--- a/rust/util/src/timer.rs
+++ b/rust/util/src/timer.rs
@@ -86,10 +86,10 @@ pub fn init_full<'timer, 'opaque: 'timer, T, F>(
         }
     }

-    pub fn modify(&self, expire_time: u64) {
+    pub fn modify(&self, expire_time: i64) {
         // SAFETY: the only way to obtain a Timer safely is via methods that
         // take a Pin<&mut Self>, therefore the timer is pinned
-        unsafe { timer_mod(self.as_mut_ptr(), expire_time as i64) }
+        unsafe { timer_mod(self.as_mut_ptr(), expire_time) }
     }

     pub fn delete(&self) {
--
2.34.1


Reply via email to