MIME-Version: 1.0
References: <20230119174036.64046-1-wedsonaf@gmail.com> <20230119174036.64046-2-wedsonaf@gmail.com>
 <20230127135533.3dfc7440.gary@garyguo.net>
In-Reply-To: <20230127135533.3dfc7440.gary@garyguo.net>
From:   Wedson Almeida Filho <wedsonaf@gmail.com>
Date:   Mon, 30 Jan 2023 02:59:27 -0300
Message-ID: <CANeycqrWd4Tn8KpfY0Jga-7nrWN9pM23Y3B+j2HAphPbdEOM-Q@mail.gmail.com>
Subject: Re: [PATCH 2/5] rust: types: introduce `ForeignOwnable`
To:     Gary Guo <gary@garyguo.net>
Cc:     rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 27 Jan 2023 at 10:55, Gary Guo <gary@garyguo.net> wrote:
>
> On Thu, 19 Jan 2023 14:40:33 -0300
> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> > It was originally called `PointerWrapper`. It is used to convert
> > a Rust object to a pointer representation (void *) that can be
> > stored on the C side, used, and eventually returned to Rust.
> >
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> > ---
> >  rust/kernel/lib.rs   |  1 +
> >  rust/kernel/types.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 55 insertions(+)
> >
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index e0b0e953907d..223564f9f0cc 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -16,6 +16,7 @@
> >  #![feature(coerce_unsized)]
> >  #![feature(core_ffi_c)]
> >  #![feature(dispatch_from_dyn)]
> > +#![feature(generic_associated_types)]
> >  #![feature(receiver_trait)]
> >  #![feature(unsize)]
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index f0ad4472292d..5475f6163002 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -9,6 +9,60 @@ use core::{
> >      ops::{Deref, DerefMut},
> >  };
> >
> > +/// Used to transfer ownership to and from foreign (non-Rust) languages.
> > +///
> > +/// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> > +/// later may be transferred back to Rust by calling [`Self::from_foreign`].
> > +///
> > +/// This trait is meant to be used in cases when Rust objects are stored in C objects and
> > +/// eventually "freed" back to Rust.
> > +pub trait ForeignOwnable {
> > +    /// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and
> > +    /// [`ForeignOwnable::from_foreign`].
> > +    type Borrowed<'a>;
> > +
> > +    /// Converts a Rust-owned object to a foreign-owned one.
> > +    ///
> > +    /// The foreign representation is a pointer to void.
> > +    fn into_foreign(self) -> *const core::ffi::c_void;
> > +
> > +    /// 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_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<T: ForeignOwnable>(ptr: *const core::ffi::c_void) -> ScopeGuard<T, fn(T)> {
>
> I feel that this should could its own guard (maybe `PointerGuard`?) to
> be more semantically meaningful than a `ScopeGuard`.

I prefer not to add yet another type just for this internal type. It's
only used in the implementation of abstractions, and is exported only
to make it simpler for users to refer to types indirectly (e.g., `<T
as ForeignOwnable>::Borrowed<'_>`).

>
> > +        // SAFETY: The safety requirements ensure that `ptr` came from a previous call to
> > +        // `into_foreign`.
> > +        ScopeGuard::new_with_data(unsafe { T::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.
> > +    unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
> > +}
> > +
> >  /// Runs a cleanup function/closure when dropped.
> >  ///
> >  /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
>