lkml.org 
[lkml]   [2023]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2023-03-27 00:57    [W:2.465 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site