Re: [PATCH v11 08/13] rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&mut T>`

[Benno Lossin <benno.lossin@proton.me>]

On Fri Mar 7, 2025 at 10:38 PM CET, Andreas Hindborg wrote:
> +impl<'a, T> RawHrTimerCallback for Pin<&'a mut T>
> +where
> +    T: HasHrTimer<T>,
> +    T: HrTimerCallback<Pointer<'a> = Self>,
> +{
> +    type CallbackTarget<'b> = Self;
> +
> +    unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart {
> +        // `HrTimer` is `repr(C)`
> +        let timer_ptr = ptr as *mut HrTimer<T>;
> +
> +        // SAFETY: By the safety requirement of this function, `timer_ptr`
> +        // points to a `HrTimer<T>` contained in an `T`.
> +        let receiver_ptr = unsafe { T::timer_container_of(timer_ptr) };
> +
> +        // SAFETY:
> +        //  - By the safety requirement of this function, `timer_ptr`
> +        //    points to a `HrTimer<T>` contained in an `T`.
> +        //  - As per he safety requirements of the trait `HrTimerHandle`, the
> +        //    `PinMutHrTimerHandle` associated with this timer is guaranteed to
> +        //    be alive until this method returns. As the handle borrows from
> +        //    `T`, `T` is also guaranteed to be alive for the duration of this
> +        //    function.

Ah one more thing, I don't think that the second part is needed (i.e.
that `T` is alive). How about:

        //  - As per the safety requirements of the trait `HrTimerHandle`, the `PinMutHrTimerHandle`
        //  associated with this timer is guaranteed to be alive until this method returns. That
        //  handle borrows the `T` behind `receiver_ptr` mutably thus guaranteeing the validity of
        //  the reference created below.

Can you also adjust the other instances of this in the other patches?
Thanks!

---
Cheers,
Benno

> +        //  - Because `PinMutTimerHandle` borrows mutably from `T`, the
> +        //    reference we create here is guaranteed to be unique.
> +        let receiver_ref = unsafe { &mut *receiver_ptr };
> +
> +        // SAFETY: `receiver_ref` only exists as pinned, so it is safe to pin it
> +        // here.
> +        let receiver_pin = unsafe { Pin::new_unchecked(receiver_ref) };
> +
> +        T::run(receiver_pin).into_c()
> +    }
> +}
>
> --
> 2.47.0