[PATCH v3] rust: device: change the from_raw() function
From: Guilherme Giacomo Simoes
Date: Tue Oct 01 2024 - 16:56:35 EST
The function Device::from_raw() increments a refcount by a call to
bindings::get_device(ptr). This can be confused because usually
from_raw() functions don't increment a refcount.
Hence, rename Device::from_raw() to avoid confuion with other "from_raw"
semantics.
The new name of function should be "get_device" to be consistent with
the function get_device() already exist in .c files.
This function body also changed, because the `into()` will convert the
`&'a Device` into `ARef<Device>` and also call `inc_ref` from the
`AlwaysRefCounted` trait implemented for Device.
Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@xxxxxxxxx>
---
differences from v1 to v2:
- remove the 0/1 patch
- refactor get_device() function
differences from v2:
- fix the place of changelog.
The motivation from this change was will discussion in:
https://rust-for-linux.zulipchat.com/#narrow/stream/291566-Library/topic/Inconsistency.20of.20.60from_raw.60.2E
I would like to thanks for Greg <gregkh@xxxxxxxxxxxxxxxxxxx>, Danilo
<dakr@xxxxxxxxxx> and Alice <aliceryhl@xxxxxxxxxx> for help me with this
patch.
---
rust/kernel/device.rs | 15 +++------------
rust/kernel/firmware.rs | 2 +-
2 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 851018eef885..c8199ee079ef 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -51,18 +51,9 @@ impl Device {
///
/// It must also be ensured that `bindings::device::release` can be called from any thread.
/// While not officially documented, this should be the case for any `struct device`.
- pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
- // SAFETY: By the safety requirements, ptr is valid.
- // Initially increase the reference count by one to compensate for the final decrement once
- // this newly created `ARef<Device>` instance is dropped.
- unsafe { bindings::get_device(ptr) };
-
- // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::device`.
- let ptr = ptr.cast::<Self>();
-
- // SAFETY: `ptr` is valid by the safety requirements of this function. By the above call to
- // `bindings::get_device` we also own a reference to the underlying `struct device`.
- unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(ptr)) }
+ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
+ // SAFETY: By the safety requirements ptr is valid
+ unsafe { Self::as_ref(ptr) }.into()
}
/// Obtain the raw `struct device *`.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index dee5b4b18aec..13a374a5cdb7 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -44,7 +44,7 @@ fn request_nowarn() -> Self {
///
/// # fn no_run() -> Result<(), Error> {
/// # // SAFETY: *NOT* safe, just for the example to get an `ARef<Device>` instance
-/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) };
+/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) };
///
/// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?;
/// let blob = fw.data();
--
2.46.2