[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87jzdopthc.fsf@kernel.org>
Date: Thu, 31 Oct 2024 09:27:43 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Tamir Duberstein" <tamird@...il.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
<alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
<gary@...yguo.net>, =?utf-8?Q?Bj=C3=B6rn?=
Roy Baron
<bjorn3_gh@...tonmail.com>, "Benno
Lossin" <benno.lossin@...ton.me>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich"
<dakr@...nel.org>, <rust-for-linux@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/5] rust: arc: use `NonNull::new_unchecked`
"Tamir Duberstein" <tamird@...il.com> writes:
> There is no need to check (and panic on violations of) the safety
> requirements on `ForeignOwnable` functions. Avoiding the check is
> consistent with the implementation of `ForeignOwnable` for `Box`.
>
> Signed-off-by: Tamir Duberstein <tamird@...il.com>
> ---
> rust/kernel/sync/arc.rs | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index db9da352d588f65348aa7a5204abbb165b70197f..4857230bd8d410bcca97b2081c3ce2f617ee7921 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -337,9 +337,9 @@ fn into_foreign(self) -> *const core::ffi::c_void {
> }
>
> unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
> - // By the safety requirement of this function, we know that `ptr` came from
> - // a previous call to `Arc::into_foreign`.
> - let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap();
> + // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
> + // call to `Self::into_foreign`.
> + let inner = unsafe { NonNull::new_unchecked(ptr as _) };
Please use an explicit cast.
>
> // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive
> // for the lifetime of the returned value.
> @@ -347,10 +347,14 @@ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
> }
>
> unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
> + // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
> + // call to `Self::into_foreign`.
> + let inner = unsafe { NonNull::new_unchecked(ptr as _) };
Please use an explicit cast.
> +
> // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
> // holds a reference count increment that is transferrable to us.
> - unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
> + unsafe { Self::from_inner(inner) }
> }
> }
Otherwise lgtm.
Best regards,
Andreas
Powered by blists - more mailing lists