Messages in this thread | | | Date | Thu, 06 Jul 2023 17:33:20 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH] rust: delete `ForeignOwnable::borrow_mut` |
| |
On 06.07.23 11:46, Alice Ryhl wrote: > We discovered that the current design of `borrow_mut` is problematic. > This patch removes it until a better solution can be found. > > Specifically, the current design gives you access to a `&mut T`, which > lets you change where the `ForeignOwnable` points (e.g., with > `core::mem::swap`). No upcoming user of this API intended to make that > possible, making all of them unsound. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> --- > rust/kernel/sync/arc.rs | 3 +-- > rust/kernel/types.rs | 22 ++-------------------- > 2 files changed, 3 insertions(+), 22 deletions(-) > > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs > index a89843cacaad..172f563976a9 100644 > --- a/rust/kernel/sync/arc.rs > +++ b/rust/kernel/sync/arc.rs > @@ -243,8 +243,7 @@ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> { > let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap(); > > // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive > - // for the lifetime of the returned value. Additionally, the safety requirements of > - // `ForeignOwnable::borrow_mut` ensure that no new mutable references are created. > + // for the lifetime of the returned value. > unsafe { ArcBorrow::new(inner) } > } > > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs > index 1e5380b16ed5..d479f8da8f38 100644 > --- a/rust/kernel/types.rs > +++ b/rust/kernel/types.rs > @@ -35,34 +35,16 @@ pub trait ForeignOwnable: Sized { > /// > /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > - /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`] > - /// for this object must have been dropped. > unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>; > > - /// Mutably borrows a foreign-owned object. > - /// > - /// # Safety > - /// > - /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > - /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > - /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and > - /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped. > - unsafe fn borrow_mut(ptr: *const core::ffi::c_void) -> ScopeGuard<Self, fn(Self)> { > - // SAFETY: The safety requirements ensure that `ptr` came from a previous call to > - // `into_foreign`. > - ScopeGuard::new_with_data(unsafe { Self::from_foreign(ptr) }, |d| { > - d.into_foreign(); > - }) > - } > - > /// Converts a foreign-owned object back to a Rust-owned one. > /// > /// # Safety > /// > /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > - /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and > - /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped. > + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] for > + /// this object must have been dropped. > unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self; > } > > > base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3 > -- > 2.41.0.255.g8b1d071c50-goog >
| |