| Message ID | 20250319203112.131959-2-dakr@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Implement TryFrom<&Device> for bus specific devices | expand |
On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote: > Device::parent() returns a reference to the device' parent device, if > any. > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > --- > rust/kernel/device.rs | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > index 21b343a1dc4d..76b341441f3f 100644 > --- a/rust/kernel/device.rs > +++ b/rust/kernel/device.rs > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { > self.0.get() > } > > + /// Returns a reference to the parent device, if any. > + pub fn parent<'a>(&self) -> Option<&'a Self> { > + // SAFETY: By the type invariant `self.as_raw()` is always valid. > + let parent = unsafe { *self.as_raw() }.parent; This means: 1. Copy the entire `struct device` onto the stack. 2. Read the `parent` field of the copy. Please write this instead to only read the `parent` field: let parent = unsafe { *self.as_raw().parent }; > + if parent.is_null() { > + None > + } else { > + // SAFETY: Since `parent` is not NULL, it must be a valid pointer to a `struct device`. > + Some(unsafe { Self::as_ref(parent) }) > + } > + } > + > /// Convert a raw C `struct device` pointer to a `&'a Device`. > /// > /// # Safety > -- > 2.48.1 >
On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote: > > On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote: > > Device::parent() returns a reference to the device' parent device, if > > any. > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > --- > > rust/kernel/device.rs | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > > index 21b343a1dc4d..76b341441f3f 100644 > > --- a/rust/kernel/device.rs > > +++ b/rust/kernel/device.rs > > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { > > self.0.get() > > } > > > > + /// Returns a reference to the parent device, if any. > > + pub fn parent<'a>(&self) -> Option<&'a Self> { > > + // SAFETY: By the type invariant `self.as_raw()` is always valid. > > + let parent = unsafe { *self.as_raw() }.parent; > > This means: > 1. Copy the entire `struct device` onto the stack. > 2. Read the `parent` field of the copy. > > Please write this instead to only read the `parent` field: > let parent = unsafe { *self.as_raw().parent }; Sorry I meant (*self.as_raw()).parent
On Thu, Mar 20, 2025 at 09:40:45AM +0100, Alice Ryhl wrote: > On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote: > > > Device::parent() returns a reference to the device' parent device, if > > > any. > > > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > > --- > > > rust/kernel/device.rs | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > > > index 21b343a1dc4d..76b341441f3f 100644 > > > --- a/rust/kernel/device.rs > > > +++ b/rust/kernel/device.rs > > > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { > > > self.0.get() > > > } > > > > > > + /// Returns a reference to the parent device, if any. > > > + pub fn parent<'a>(&self) -> Option<&'a Self> { > > > + // SAFETY: By the type invariant `self.as_raw()` is always valid. > > > + let parent = unsafe { *self.as_raw() }.parent; > > > > This means: > > 1. Copy the entire `struct device` onto the stack. > > 2. Read the `parent` field of the copy. > > > > Please write this instead to only read the `parent` field: > > let parent = unsafe { *self.as_raw().parent }; > > Sorry I meant (*self.as_raw()).parent Good catch, thanks!
On Thu, Mar 20, 2025 at 01:19:11PM +0100, Danilo Krummrich wrote: > On Thu, Mar 20, 2025 at 09:40:45AM +0100, Alice Ryhl wrote: > > On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > > > On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote: > > > > Device::parent() returns a reference to the device' parent device, if > > > > any. > > > > > > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > > > > --- > > > > rust/kernel/device.rs | 13 +++++++++++++ > > > > 1 file changed, 13 insertions(+) > > > > > > > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > > > > index 21b343a1dc4d..76b341441f3f 100644 > > > > --- a/rust/kernel/device.rs > > > > +++ b/rust/kernel/device.rs > > > > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { > > > > self.0.get() > > > > } > > > > > > > > + /// Returns a reference to the parent device, if any. > > > > + pub fn parent<'a>(&self) -> Option<&'a Self> { > > > > + // SAFETY: By the type invariant `self.as_raw()` is always valid. > > > > + let parent = unsafe { *self.as_raw() }.parent; > > > > > > This means: > > > 1. Copy the entire `struct device` onto the stack. > > > 2. Read the `parent` field of the copy. > > > > > > Please write this instead to only read the `parent` field: > > > let parent = unsafe { *self.as_raw().parent }; > > > > Sorry I meant (*self.as_raw()).parent > > Good catch, thanks! With that fixed you may add Reviewed-by: Alice Ryhl <aliceryhl@google.com>
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 21b343a1dc4d..76b341441f3f 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { self.0.get() } + /// Returns a reference to the parent device, if any. + pub fn parent<'a>(&self) -> Option<&'a Self> { + // SAFETY: By the type invariant `self.as_raw()` is always valid. + let parent = unsafe { *self.as_raw() }.parent; + + if parent.is_null() { + None + } else { + // SAFETY: Since `parent` is not NULL, it must be a valid pointer to a `struct device`. + Some(unsafe { Self::as_ref(parent) }) + } + } + /// Convert a raw C `struct device` pointer to a `&'a Device`. /// /// # Safety
Device::parent() returns a reference to the device' parent device, if any. Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- rust/kernel/device.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+)