Use core::time::Duration instead of u64 to represent timeouts Replace use of raw u64 with naming conventions representing timeouts, timedeltas, durations, and similar. Tests: all unit tests pass Change-Id: I21259dfb8552ff74b333bc01f4aa91b930eaad20
diff --git a/gbl/efi/src/fastboot.rs b/gbl/efi/src/fastboot.rs index 485e3ab..a703a38 100644 --- a/gbl/efi/src/fastboot.rs +++ b/gbl/efi/src/fastboot.rs
@@ -22,7 +22,10 @@ ops::Ops, }; use alloc::{boxed::Box, vec::Vec}; -use core::{cmp::min, fmt::Write, future::Future, mem::take, pin::Pin, sync::atomic::AtomicU64}; +use core::{ + cmp::min, fmt::Write, future::Future, mem::take, pin::Pin, sync::atomic::AtomicU64, + time::Duration, +}; use efi::{ efi_print, efi_println, protocol::{gbl_efi_fastboot_usb::GblFastbootUsbProtocol, Protocol}, @@ -33,7 +36,7 @@ use liberror::{Error, Result}; use libgbl::fastboot::{run_gbl_fastboot, GblTcpStream, GblUsbTransport, PinFutContainer}; -const DEFAULT_TIMEOUT_MS: u64 = 5_000; +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(5); const FASTBOOT_TCP_PORT: u16 = 5554; struct EfiFastbootTcpTransport<'a, 'b, 'c> { @@ -49,12 +52,12 @@ impl TcpStream for EfiFastbootTcpTransport<'_, '_, '_> { /// Reads to `out` for exactly `out.len()` number bytes from the TCP connection. async fn read_exact(&mut self, out: &mut [u8]) -> Result<()> { - self.socket.receive_exact(out, DEFAULT_TIMEOUT_MS).await + self.socket.receive_exact(out, DEFAULT_TIMEOUT).await } /// Sends exactly `data.len()` number bytes from `data` to the TCP connection. async fn write_exact(&mut self, data: &[u8]) -> Result<()> { - self.socket.send_exact(data, DEFAULT_TIMEOUT_MS).await + self.socket.send_exact(data, DEFAULT_TIMEOUT).await } } @@ -63,12 +66,12 @@ let efi_entry = self.socket.efi_entry; self.socket.poll(); // If not listenining, start listening. - // If not connected but it's been `DEFAULT_TIMEOUT_MS`, restart listening in case the remote + // If not connected but it's been `DEFAULT_TIMEOUT`, restart listening in case the remote // client disconnects in the middle of TCP handshake and leaves the socket in a half open // state. if !self.socket.is_listening_or_handshaking() || (!self.socket.check_active() - && self.socket.time_since_last_listen() > DEFAULT_TIMEOUT_MS) + && self.socket.time_since_last_listen() > DEFAULT_TIMEOUT) { let _ = self .socket @@ -150,7 +153,7 @@ let mut curr = &packet[..]; while !curr.is_empty() { let to_send = min(curr.len(), self.max_packet_size); - self.protocol.send_packet(&curr[..to_send], DEFAULT_TIMEOUT_MS).await?; + self.protocol.send_packet(&curr[..to_send], DEFAULT_TIMEOUT).await?; // Forces a yield to the executor if the data received/sent reaches a certain // threshold. This is to prevent the async code from holding up the CPU for too long // in case IO speed is high and the executor uses cooperative scheduling.
diff --git a/gbl/efi/src/net.rs b/gbl/efi/src/net.rs index ef668b7..2a3de28 100644 --- a/gbl/efi/src/net.rs +++ b/gbl/efi/src/net.rs
@@ -20,11 +20,12 @@ use core::{ fmt::Write, sync::atomic::{AtomicU64, Ordering}, + time::Duration, }; use efi::{ efi_print, efi_println, protocol::{simple_network::SimpleNetworkProtocol, Protocol}, - utils::{ms_to_100ns, Timeout}, + utils::Timeout, DeviceHandle, EfiEntry, Event, EventNotify, EventType, Tpl, }; use efi_types::{EfiEvent, EfiMacAddress, EFI_TIMER_DELAY_TIMER_PERIODIC}; @@ -46,8 +47,8 @@ /// Ethernet frame size for frame pool. const ETHERNET_FRAME_SIZE: usize = 1536; -// Update period in milliseconds for `NETWORK_TIMESTAMP`. -const NETWORK_TIMESTAMP_UPDATE_PERIOD: u64 = 50; +// Update period for `NETWORK_TIMESTAMP`. +const NETWORK_TIMESTAMP_UPDATE_PERIOD: Duration = Duration::from_millis(50); // Size of the socket tx/rx application data buffer. const SOCKET_TX_RX_BUFFER: usize = 256 * 1024; @@ -118,8 +119,14 @@ // Implements network device trait backend for the `smoltcp` crate. impl<'a> Device for EfiNetworkDevice<'a> { - type RxToken<'b> = RxToken<'b> where Self: 'b; - type TxToken<'b> = TxToken<'a, 'b> where Self: 'b; + type RxToken<'b> + = RxToken<'b> + where + Self: 'b; + type TxToken<'b> + = TxToken<'a, 'b> + where + Self: 'b; fn capabilities(&self) -> DeviceCapabilities { // Taken from upstream example. @@ -204,7 +211,9 @@ F: FnOnce(&mut [u8]) -> R, { loop { - match loop_with_timeout(self.efi_entry, 5000, || self.try_get_buffer().ok_or(false)) { + match loop_with_timeout(self.efi_entry, Duration::from_secs(5), || { + self.try_get_buffer().ok_or(false) + }) { Ok(Some(send_buffer)) => { // SAFETY: // * The pointer is confirmed to come from one of `self.tx_frames`. It's @@ -320,7 +329,7 @@ pub fn listen(&mut self, port: u16) -> Result<()> { self.get_socket().abort(); self.get_socket().listen(port).map_err(listen_to_unified)?; - self.last_listen_timestamp = Some(self.timestamp(0)); + self.last_listen_timestamp = Some(self.timestamp(0).as_millis() as u64); Ok(()) } @@ -330,9 +339,9 @@ } /// Returns the amount of time elapsed since last call to `Self::listen()`. If `listen()` has - /// never been called, `u64::MAX` is returned. - pub fn time_since_last_listen(&mut self) -> u64 { - self.last_listen_timestamp.map(|v| self.timestamp(v)).unwrap_or(u64::MAX) + /// never been called, `Duration::MAX` is returned. + pub fn time_since_last_listen(&mut self) -> Duration { + self.last_listen_timestamp.map(|v| self.timestamp(v)).unwrap_or(Duration::MAX) } /// Polls network device. @@ -364,7 +373,7 @@ } /// Receives exactly `out.len()` number of bytes to `out`. - pub async fn receive_exact(&mut self, out: &mut [u8], timeout: u64) -> Result<()> { + pub async fn receive_exact(&mut self, out: &mut [u8], timeout: Duration) -> Result<()> { let timer = Timeout::new(self.efi_entry, timeout)?; let mut curr = &mut out[..]; while !curr.is_empty() { @@ -394,7 +403,7 @@ } /// Sends exactly `data.len()` number of bytes from `data`. - pub async fn send_exact(&mut self, data: &[u8], timeout: u64) -> Result<()> { + pub async fn send_exact(&mut self, data: &[u8], timeout: Duration) -> Result<()> { let timer = Timeout::new(self.efi_entry, timeout)?; let mut curr = &data[..]; let mut last_send_queue = self.get_socket().send_queue(); @@ -437,19 +446,19 @@ &self.interface } - /// Returns the number of milliseconds elapsed since the `base` timestamp. - pub fn timestamp(&self, base: u64) -> u64 { + /// Returns the duration elapsed since the `base` timestamp. + pub fn timestamp(&self, base_in_millis: u64) -> Duration { let curr = self.timestamp.load(Ordering::Relaxed); // Assume there can be at most one overflow. - match curr < base { - true => u64::MAX - (base - curr), - false => curr - base, - } + Duration::from_millis(match curr < base_in_millis { + true => u64::MAX - (base_in_millis - curr), + false => curr - base_in_millis, + }) } /// Returns a smoltcp time `Instant` value. fn instant(&self) -> Instant { - to_smoltcp_instant(self.timestamp(0)) + to_smoltcp_instant(self.timestamp(0).as_millis() as u64) } /// Broadcasts Fuchsia Fastboot MDNS service once. @@ -551,7 +560,10 @@ // Initializes notification functions. if self.notify_fn.is_none() { self.notify_fn = Some(Box::new(|_: EfiEvent| { - self.timestamp.fetch_add(NETWORK_TIMESTAMP_UPDATE_PERIOD, Ordering::Relaxed); + self.timestamp.fetch_add( + NETWORK_TIMESTAMP_UPDATE_PERIOD.as_millis() as u64, + Ordering::Relaxed, + ); })); self.notify = Some(EventNotify::new(Tpl::Callback, self.notify_fn.as_mut().unwrap())); } @@ -569,7 +581,7 @@ bs.set_timer( &_time_update_event, EFI_TIMER_DELAY_TIMER_PERIODIC, - ms_to_100ns(NETWORK_TIMESTAMP_UPDATE_PERIOD)?, + NETWORK_TIMESTAMP_UPDATE_PERIOD, )?; // Gets our MAC address and IPv6 address.
diff --git a/gbl/efi/src/ops.rs b/gbl/efi/src/ops.rs index f6bd684..2ce0432 100644 --- a/gbl/efi/src/ops.rs +++ b/gbl/efi/src/ops.rs
@@ -26,7 +26,7 @@ use arrayvec::ArrayVec; use core::{ cmp::min, ffi::CStr, fmt::Write, mem::MaybeUninit, num::NonZeroUsize, ops::DerefMut, ptr::null, - slice::from_raw_parts_mut, + slice::from_raw_parts_mut, time::Duration, }; use efi::{ efi_print, efi_println, @@ -271,7 +271,7 @@ let found = wait_key_stroke( self.efi_entry, |key| key.unicode_char == 0x08 || (key.unicode_char == 0x0 && key.scan_code == 0x08), - 2000, + Duration::from_secs(2), ); if matches!(found, Ok(true)) { efi_println!(self.efi_entry, "Backspace pressed, entering fastboot");
diff --git a/gbl/efi/src/utils.rs b/gbl/efi/src/utils.rs index b1c5390..dfe1d75 100644 --- a/gbl/efi/src/utils.rs +++ b/gbl/efi/src/utils.rs
@@ -14,6 +14,7 @@ use crate::efi; use ::efi::EfiMemoryAttributesTable; +use core::time::Duration; use efi::{ protocol::{ device_path::{DevicePathProtocol, DevicePathText, DevicePathToTextProtocol}, @@ -92,18 +93,22 @@ /// Repetitively runs a closure until it signals completion or timeout. /// /// * If `f` returns `Ok(R)`, an `Ok(Some(R))` is returned immediately. -/// * If `f` has been repetitively called and returning `Err(false)` for `timeout_ms`, an +/// * If `f` has been repetitively called and returning `Err(false)` for `timeout_duration`, an /// `Ok(None)` is returned. This is the time out case. /// * If `f` returns `Err(true)` the timeout is reset. -pub fn loop_with_timeout<F, R>(efi_entry: &EfiEntry, timeout_ms: u64, mut f: F) -> Result<Option<R>> +pub fn loop_with_timeout<F, R>( + efi_entry: &EfiEntry, + timeout_duration: Duration, + mut f: F, +) -> Result<Option<R>> where F: FnMut() -> core::result::Result<R, bool>, { - let timeout = Timeout::new(efi_entry, timeout_ms)?; + let timeout = Timeout::new(efi_entry, timeout_duration)?; while !timeout.check()? { match f() { Ok(v) => return Ok(Some(v)), - Err(true) => timeout.reset(timeout_ms)?, + Err(true) => timeout.reset(timeout_duration)?, _ => {} } } @@ -116,13 +121,13 @@ pub fn wait_key_stroke( efi_entry: &EfiEntry, pred: impl Fn(EfiInputKey) -> bool, - timeout_ms: u64, + timeout: Duration, ) -> Result<bool> { let input = efi_entry .system_table() .boot_services() .find_first_and_open::<SimpleTextInputProtocol>()?; - loop_with_timeout(efi_entry, timeout_ms, || -> core::result::Result<Result<bool>, bool> { + loop_with_timeout(efi_entry, timeout, || -> core::result::Result<Result<bool>, bool> { match input.read_key_stroke() { Ok(Some(key)) if pred(key) => Ok(Ok(true)), Err(e) => Ok(Err(e.into())),
diff --git a/gbl/libefi/mocks/utils.rs b/gbl/libefi/mocks/utils.rs index b3d9a75..96049cd 100644 --- a/gbl/libefi/mocks/utils.rs +++ b/gbl/libefi/mocks/utils.rs
@@ -15,6 +15,7 @@ //! Mock utils. use crate::MockEfiEntry; +use core::time::Duration; use liberror::Result; use mockall::mock; @@ -22,11 +23,11 @@ /// Mock [efi::utils::Timeout]. pub Timeout { /// Creates a new [MockTimeout]. - pub fn new(efi_entry: &MockEfiEntry, timeout_ms: u64) -> Result<Self>; + pub fn new(efi_entry: &MockEfiEntry, timeout: Duration) -> Result<Self>; /// Checks the timeout. pub fn check(&self) -> Result<bool>; /// Resets the timeout. - pub fn reset(&self, timeout_ms: u64) -> Result<()>; + pub fn reset(&self, timeout: Duration) -> Result<()>; } } /// Map to the libefi name so code under test can just use one name.
diff --git a/gbl/libefi/src/lib.rs b/gbl/libefi/src/lib.rs index c247776..6ffcdfd 100644 --- a/gbl/libefi/src/lib.rs +++ b/gbl/libefi/src/lib.rs
@@ -67,7 +67,7 @@ #[cfg(not(test))] use core::{fmt::Write, panic::PanicInfo}; -use core::{marker::PhantomData, ptr::null_mut, slice::from_raw_parts}; +use core::{marker::PhantomData, ptr::null_mut, slice::from_raw_parts, time::Duration}; use efi_types::{ EfiBootService, EfiConfigurationTable, EfiEvent, EfiGuid, EfiHandle, EfiMemoryAttributesTableHeader, EfiMemoryDescriptor, EfiMemoryType, EfiRuntimeService, @@ -496,11 +496,16 @@ &self, event: &Event, delay_type: EfiTimerDelay, - trigger_time: u64, + trigger_time: Duration, ) -> Result<()> { // SAFETY: EFI_BOOT_SERVICES method call. unsafe { - efi_call!(self.boot_services.set_timer, event.efi_event, delay_type, trigger_time) + efi_call!( + self.boot_services.set_timer, + event.efi_event, + delay_type, + (trigger_time.as_nanos() / 100).try_into()? + ) } } }
diff --git a/gbl/libefi/src/protocol/gbl_efi_fastboot_usb.rs b/gbl/libefi/src/protocol/gbl_efi_fastboot_usb.rs index 7b37ef7..7c3e385 100644 --- a/gbl/libefi/src/protocol/gbl_efi_fastboot_usb.rs +++ b/gbl/libefi/src/protocol/gbl_efi_fastboot_usb.rs
@@ -19,6 +19,7 @@ utils::with_timeout, {efi_call, Event}, }; +use core::time::Duration; use efi_types::{EfiGuid, GblEfiFastbootUsbProtocol}; use gbl_async::yield_now; use liberror::{Error, Result}; @@ -130,8 +131,8 @@ } /// Sends a packet over the USB. - pub async fn send_packet(&self, data: &[u8], timeout_ms: u64) -> Result<()> { + pub async fn send_packet(&self, data: &[u8], timeout: Duration) -> Result<()> { self.fastboot_usb_send(data)?; - with_timeout(self.efi_entry(), self.wait_send(), timeout_ms).await?.ok_or(Error::Timeout)? + with_timeout(self.efi_entry(), self.wait_send(), timeout).await?.ok_or(Error::Timeout)? } }
diff --git a/gbl/libefi/src/utils.rs b/gbl/libefi/src/utils.rs index f226687..5cf00f3 100644 --- a/gbl/libefi/src/utils.rs +++ b/gbl/libefi/src/utils.rs
@@ -15,7 +15,7 @@ //! This file provides some utilities built on EFI APIs. use crate::{EfiEntry, Event, EventType}; -use core::future::Future; +use core::{future::Future, time::Duration}; use efi_types::EFI_TIMER_DELAY_TIMER_RELATIVE; use gbl_async::{select, yield_now}; use liberror::Result; @@ -34,10 +34,10 @@ impl<'a> Timeout<'a> { /// Creates a new instance and starts the timeout timer. - pub fn new(efi_entry: &'a EfiEntry, timeout_ms: u64) -> Result<Self> { + pub fn new(efi_entry: &'a EfiEntry, timeout: Duration) -> Result<Self> { let bs = efi_entry.system_table().boot_services(); let timer = bs.create_event(EventType::Timer)?; - bs.set_timer(&timer, EFI_TIMER_DELAY_TIMER_RELATIVE, ms_to_100ns(timeout_ms)?)?; + bs.set_timer(&timer, EFI_TIMER_DELAY_TIMER_RELATIVE, timeout)?; Ok(Self { efi_entry, timer }) } @@ -47,17 +47,17 @@ } /// Resets the timeout. - pub fn reset(&self, timeout_ms: u64) -> Result<()> { + pub fn reset(&self, timeout: Duration) -> Result<()> { let bs = self.efi_entry.system_table().boot_services(); - bs.set_timer(&self.timer, EFI_TIMER_DELAY_TIMER_RELATIVE, ms_to_100ns(timeout_ms)?)?; + bs.set_timer(&self.timer, EFI_TIMER_DELAY_TIMER_RELATIVE, timeout)?; Ok(()) } } /// Waits for a given amount of time. -pub async fn wait(efi_entry: &EfiEntry, duration_ms: u64) -> Result<()> { +pub async fn wait(efi_entry: &EfiEntry, duration: Duration) -> Result<()> { // EFI boot service has a `stall` API. But it's not async. - let timeout = Timeout::new(efi_entry, duration_ms)?; + let timeout = Timeout::new(efi_entry, duration)?; while !timeout.check()? { yield_now().await; } @@ -74,9 +74,9 @@ pub async fn with_timeout<F: Future<Output = R>, R>( efi_entry: &EfiEntry, fut: F, - timeout_ms: u64, + timeout: Duration, ) -> Result<Option<R>> { - let (timeout_res, res) = select(wait(efi_entry, timeout_ms), fut).await; + let (timeout_res, res) = select(wait(efi_entry, timeout), fut).await; match timeout_res { Some(Err(e)) => return Err(e), _ => Ok(res),