Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers

FUJITA Tomonori
2023-11-21T12:47:34Z

On Fri, 17 Nov 2023 15:42:46 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> Anyway. If you don't want to write down the tribal knowledge here, then
> I suggest this simpler wording:
>
> /// # Invariants
> ///
> /// Referencing a `phy_device` using this struct asserts that you are in
> /// a context where all methods defined on this struct are safe to call.
> #[repr(transparent)]
> pub struct Device(Opaque<bindings::phy_device>);
>
> This invariant is much less precise, but at least it is correct.
>
> Other safety comments may then be:
>
> /// Gets the id of the PHY.
> pub fn phy_id(&self) -> u32 {
> let phydev = self.0.get();
> // SAFETY: The struct invariant ensures that we may access
> // this field without additional synchronization.
> unsafe { (*phydev).phy_id }
> }
>
> And:
>
> unsafe extern "C" fn soft_reset_callback(
> phydev: *mut bindings::phy_device,
> ) -> core::ffi::c_int {
> from_result(|| {
> // SAFETY: This callback is called only in contexts
> // where we hold `phy_device->lock`, so the accessors on
> // `Device` are okay to call.
> let dev = unsafe { Device::from_raw(phydev) };
> T::soft_reset(dev)?;
> Ok(0)
> })
> }
>
> And:
>
> unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int {
> from_result(|| {
> // SAFETY: The C core code ensures that the accessors on
> // `Device` are okay to call even though `phy_device->lock`
> // might not be held.
> let dev = unsafe { Device::from_raw(phydev) };
> T::resume(dev)?;
> Ok(0)
> })
> }

With these comments, What I should write on from_raw() function as a
safety comment?

/// # Safety
///
/// For the duration of 'a, the pointer must point at a valid
/// `phy_device`, and the caller must ensure that an user of this struct
/// in a context where all methods defined on this struct are safe to
/// call.
unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self {