Messages in this thread Patch in this message | | | Date | Mon, 13 Mar 2023 11:05:04 -0700 | From | Boqun Feng <> | Subject | Re: [PATCH 5/5] rust: device: Add a stub abstraction for devices |
| |
On Mon, Mar 13, 2023 at 05:52:02PM +0000, Gary Guo wrote: [...] > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > > index 9be021e393ca..e57da622d817 100644 > > --- a/rust/kernel/device.rs > > +++ b/rust/kernel/device.rs > > @@ -4,7 +4,7 @@ > > //! > > //! C header: [`include/linux/device.h`](../../../../include/linux/device.h) > > > > -use crate::bindings; > > +use crate::{bindings, str::CStr}; > > > > /// A raw device. > > /// > > @@ -20,4 +20,78 @@ use crate::bindings; > > pub unsafe trait RawDevice { > > /// Returns the raw `struct device` related to `self`. > > fn raw_device(&self) -> *mut bindings::device; > > + > > + /// Returns the name of the device. > > + fn name(&self) -> &CStr { > > + let ptr = self.raw_device(); > > + > > + // SAFETY: `ptr` is valid because `self` keeps it alive. > > + let name = unsafe { bindings::dev_name(ptr) }; > > + > > + // SAFETY: The name of the device remains valid while it is alive (because the device is > > + // never renamed, per the safety requirement of this trait). This is guaranteed to be the > > + // case because the reference to `self` outlives the one of the returned `CStr` (enforced > > + // by the compiler because of their lifetimes). > > + unsafe { CStr::from_char_ptr(name) } > > + } > > +} > > + > > +/// A ref-counted device. > > +/// > > +/// # Invariants > > +/// > > +/// `ptr` is valid, non-null, and has a non-zero reference count. One of the references is owned by > > +/// `self`, and will be decremented when `self` is dropped. > > +pub struct Device { > > + pub(crate) ptr: *mut bindings::device, > > +} > > + > > Shouldn't this be > > #[repr(transparent)] > pub struct Device(Opaque<bindings::device>); > > ?
I have the same feeling, for `task_struct`, we have
#[repr(transparent)] pub struct Task(pub(crate) UnsafeCell<bindings::task_struct>);
and pub struct TaskRef<'a> { task: &'a Task, _not_send: PhantomData<*mut ()>, }
I wonder whether we should do the similar for `Device`, meaning `Device` is just a tranparent representation for `struct device` and another type (say, `DeviceRef`) is introduced as the reference type, or we can just use `ARef`. Regards, Boqun
> > Best, > Gary
| |