linux-kernel - Re: [PATCH v3 1/1] rust: device: rename "Device::from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZvrAHnN0D7KPlLDc@pollux>
Date: Mon, 30 Sep 2024 17:13:34 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, ojeda@...nel.org,
	alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net,
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	aliceryhl@...gle.com, mcgrof@...nel.org, russ.weight@...ux.dev,
	dakr@...hat.com, a.hindborg@...nel.org,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] rust: device: rename "Device::from_raw()"

On Mon, Sep 30, 2024 at 11:43:27AM -0300, Guilherme Giacomo Simoes wrote:
> This function increments the refcount by a call to
> "bindings::get_device(ptr)". This can be confused because, the function
> Arch::from_raw() from standard library, don't increments the refcount.

`Arc::from_raw`

Again the note, it's both the semantics used in the Rust stdlib and in the
kernel.

Let's wait for what Greg says, if he wants to fix the small typo when
applying the patch.

Acked-by: Danilo Krummrich <dakr@...nel.org>

> Hence, rename "Device::from_raw()" to avoid confusion with other
> "from_raw" semantics.
> 
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>
> ---
>  rust/kernel/device.rs   | 2 +-
>  rust/kernel/firmware.rs | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 851018eef885..ecffaff041e0 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -51,7 +51,7 @@ 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> {
> +    pub unsafe fn get_device(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.
> 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
>