Messages in this thread | | | From | Alice Ryhl <> | Date | Tue, 12 Dec 2023 10:51:22 +0100 | Subject | Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout` |
| |
On Fri, Dec 8, 2023 at 8:04 PM Benno Lossin <benno.lossin@proton.me> wrote: > > On 12/6/23 11:09, Alice Ryhl wrote: > > + /// Atomically releases the given lock (whose ownership is proven by the guard) and puts the > > + /// thread to sleep. It wakes up when notified by [`CondVar::notify_one`] or > > + /// [`CondVar::notify_all`], or when the thread receives a signal. > > + /// > > + /// Returns whether there is a signal pending. > > + fn wait_internal_timeout<T, B>( > > + &self, > > + wait_state: u32, > > + guard: &mut Guard<'_, T, B>, > > + timeout: u64, > > + ) -> u64 > > + where > > + T: ?Sized, > > + B: Backend, > > + { > > + let wait = Opaque::<bindings::wait_queue_entry>::uninit(); > > + > > + // SAFETY: `wait` points to valid memory. > > + unsafe { bindings::init_wait(wait.get()) }; > > + > > + // SAFETY: Both `wait` and `wait_list` point to valid memory. > > + unsafe { > > + bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _) > > Does `.into()` work here? If for some reason the type here changes, we > probably want to know about it.
I think we may be able to eliminate this cast by using c_int for the integer type.
> > + }; > > + > > + // SAFETY: Switches to another thread. > > + let timeout = > > + guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout as _) as _ }); > > Ditto.
Here, we're casting u64->long and then long->u64. How about this?
u64->long - Use timeout.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT), since MAX_SCHEDULE_TIMEOUT is LONG_MAX.
long->u64 - This value is guaranteed to be less than the argument passed to schedule_timeout. Use .into() for infallible cast.
Alice
| |