rust: device: change the from_raw() function

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 <[email protected]>

---
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 <[email protected]>, Danilo <[email protected]> and Alice <[email protected]> for help me with this patch.

---
 rust/kernel/device.rs   | 15 +++------------
 rust/kernel/firmware.rs |  2 +-
 2 files changed, 4 insertions(+), 13 deletions(-)

rust/kernel/device.rs +3 -12

index 851018eef885..c8199ee079ef 100644
@@ -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 *`.

rust/kernel/firmware.rs +1 -1

index dee5b4b18aec..13a374a5cdb7 100644
@@ -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();

Signature · 1 line

2.46.2

Quoted 9 lines · Guilherme Giacomo Simoes

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.

Quoted 5 lines

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 <[email protected]>

Acked-by Danilo Krummrich <[email protected]>

Quoted 7 lines · Danilo Krummrich

Quoted 6 lines · Guilherme Giacomo Simoes

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.

Quoted 7 lines

Quoted 7 lines

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.

Quoted 3 lines

Quoted 1 line

Signed-off-by Guilherme Giacomo Simoes <[email protected]>

Acked-by Danilo Krummrich <[email protected]>

Thanks for the review, and Guilherme, for all of the revisions. I'll queue this up for this release soon so we don't get any code building on the old api.

thanks,

greg k-h

This work is because we have an issue, so this should really have a:

Closes https://github.com/Rust-for-Linux/linux/issues/1088

tag.

Greg, could you add this when you queue this?

Guilherme, please add such a tag next time, so we can have things tracked. Thanks!

Regards,
Boqun