Re: [PATCH v10 7/8] rust: Add read_poll_timeout functions

[Daniel Almeida]

Hi,

> +
> +/// Polls periodically until a condition is met or a timeout is reached.
> +///
> +/// ```rust
> +/// use kernel::io::poll::read_poll_timeout;
> +/// use kernel::time::Delta;
> +/// use kernel::sync::{SpinLock, new_spinlock};
> +///
> +/// let lock = KBox::pin_init(new_spinlock!(()), kernel::alloc::flags::GFP_KERNEL)?;
> +/// let g = lock.lock();
> +/// read_poll_timeout(|| Ok(()), |()| true, Delta::from_micros(42), Some(Delta::from_micros(42)));
> +/// drop(g);
> +///
> +/// # Ok::<(), Error>(())

IMHO, the example section here needs to be improved.

> +/// ```
> +#[track_caller]
> +pub fn read_poll_timeout<Op, Cond, T>(
> +    mut op: Op,
> +    mut cond: Cond,
> +    sleep_delta: Delta,
> +    timeout_delta: Option<Delta>,
> +) -> Result<T>
> +where
> +    Op: FnMut() -> Result<T>,
> +    Cond: FnMut(&T) -> bool,
> +{
> +    let start = Instant::now();
> +    let sleep = !sleep_delta.is_zero();
> +
> +    if sleep {
> +        might_sleep(Location::caller());
> +    }
> +
> +    loop {
> +        let val = op()?;

I.e.: it’s not clear that `op` computes `val` until you read this.

> +        if cond(&val) {

It’s a bit unclear how `cond` works, until you see this line here.

It’s even more important to explain this a tad better, since it differs slightly from the C API.

Also, it doesn’t help that `Op` and `Cond` take and return the unit type in the
only example provided.

— Daniel