This allows us to run some code when the guard is dropped (e.g.,
implicitly when it goes out of scope). We can also prevent the
guard from running by calling its `dismiss()` method.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/types.rs | 127 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 126 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index e84e51ec9716..f0ad4472292d 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -2,7 +2,132 @@
//! Kernel types.
-use core::{cell::UnsafeCell, mem::MaybeUninit};
+use alloc::boxed::Box;
+use core::{
+ cell::UnsafeCell,
+ mem::MaybeUninit,
+ ops::{Deref, DerefMut},
+};
+
+/// Runs a cleanup function/closure when dropped.
+///
+/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
+///
+/// # Examples
+///
+/// In the example below, we have multiple exit paths and we want to log regardless of which one is
+/// taken:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example1(arg: bool) {
+/// let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
+///
+/// if arg {
+/// return;
+/// }
+///
+/// pr_info!("Do something...\n");
+/// }
+///
+/// # example1(false);
+/// # example1(true);
+/// ```
+///
+/// In the example below, we want to log the same message on all early exits but a different one on
+/// the main exit path:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example2(arg: bool) {
+/// let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
+///
+/// if arg {
+/// return;
+/// }
+///
+/// // (Other early returns...)
+///
+/// log.dismiss();
+/// pr_info!("example2 no early return\n");
+/// }
+///
+/// # example2(false);
+/// # example2(true);
+/// ```
+///
+/// In the example below, we need a mutable object (the vector) to be accessible within the log
+/// function, so we wrap it in the [`ScopeGuard`]:
+/// ```
+/// # use kernel::ScopeGuard;
+/// fn example3(arg: bool) -> Result {
+/// let mut vec =
+/// ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
+///
+/// vec.try_push(10u8)?;
+/// if arg {
+/// return Ok(());
+/// }
+/// vec.try_push(20u8)?;
+/// Ok(())
+/// }
+///
+/// # assert_eq!(example3(false), Ok(()));
+/// # assert_eq!(example3(true), Ok(()));
+/// ```
+///
+/// # Invariants
+///
+/// The value stored in the struct is nearly always `Some(_)`, except between
+/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
+/// will have been returned to the caller. Since [`ScopeGuard::dismiss`] consumes the guard,
+/// callers won't be able to use it anymore.
+pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
+
+impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
+ /// Creates a new guarded object wrapping the given data and with the given cleanup function.
+ pub fn new_with_data(data: T, cleanup_func: F) -> Self {
+ // INVARIANT: The struct is being initialised with `Some(_)`.
+ Self(Some((data, cleanup_func)))
+ }
+
+ /// Prevents the cleanup function from running and returns the guarded data.
+ pub fn dismiss(mut self) -> T {
+ // INVARIANT: This is the exception case in the invariant; it is not visible to callers
+ // because this function consumes `self`.
+ self.0.take().unwrap().0
+ }
+}
+
+impl ScopeGuard<(), Box<dyn FnOnce(())>> {
+ /// Creates a new guarded object with the given cleanup function.
+ pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
+ ScopeGuard::new_with_data((), move |_| cleanup())
+ }
+}
+
+impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
+ type Target = T;
+
+ fn deref(&self) -> &T {
+ // The type invariants guarantee that `unwrap` will succeed.
+ &self.0.as_ref().unwrap().0
+ }
+}
+
+impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
+ fn deref_mut(&mut self) -> &mut T {
+ // The type invariants guarantee that `unwrap` will succeed.
+ &mut self.0.as_mut().unwrap().0
+ }
+}
+
+impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
+ fn drop(&mut self) {
+ // Run the cleanup function if one is still present.
+ if let Some((data, cleanup)) = self.0.take() {
+ cleanup(data)
+ }
+ }
+}
/// Stores an opaque value.
///
--
2.34.1
This allows us to use the unit type `()` when we have no object whose
ownership must be managed but one implementing the `ForeignOwnable`
trait is needed.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/types.rs | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index e037c262f23e..8f80cffbff59 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -85,6 +85,19 @@ impl<T: 'static> ForeignOwnable for Box<T> {
}
}
+impl ForeignOwnable for () {
+ type Borrowed<'a> = ();
+
+ fn into_foreign(self) -> *const core::ffi::c_void {
+ // We use 1 to be different from a null pointer.
+ 1usize as _
+ }
+
+ unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
+
+ unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
+}
+
/// Runs a cleanup function/closure when dropped.
///
/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
--
2.34.1
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 <[email protected]>
---
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)> {
+ // 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.
--
2.34.1
This allows us to hand ownership of Rust dynamically allocated
objects to the C side of the kernel.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/types.rs | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 5475f6163002..e037c262f23e 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -63,6 +63,28 @@ pub trait ForeignOwnable {
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
}
+impl<T: 'static> ForeignOwnable for Box<T> {
+ type Borrowed<'a> = &'a T;
+
+ fn into_foreign(self) -> *const core::ffi::c_void {
+ Box::into_raw(self) as _
+ }
+
+ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
+ // SAFETY: The safety requirements for this function ensure that the object is still alive,
+ // so it is safe to dereference the raw pointer.
+ // The safety requirements of `from_foreign` also ensure that the object remains alive for
+ // the lifetime of the returned value.
+ unsafe { &*ptr.cast() }
+ }
+
+ 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`.
+ unsafe { Box::from_raw(ptr as _) }
+ }
+}
+
/// Runs a cleanup function/closure when dropped.
///
/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
--
2.34.1
This allows us to hand ownership of Rust ref-counted objects to
the C side of the kernel.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/sync/arc.rs | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index ff73f9240ca1..519a6ec43644 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -15,7 +15,11 @@
//!
//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
-use crate::{bindings, error::Result, types::Opaque};
+use crate::{
+ bindings,
+ error::Result,
+ types::{ForeignOwnable, Opaque},
+};
use alloc::boxed::Box;
use core::{
marker::{PhantomData, Unsize},
@@ -189,6 +193,32 @@ impl<T: ?Sized> Arc<T> {
}
}
+impl<T: 'static> ForeignOwnable for Arc<T> {
+ type Borrowed<'a> = ArcBorrow<'a, T>;
+
+ fn into_foreign(self) -> *const core::ffi::c_void {
+ ManuallyDrop::new(self).ptr.as_ptr() as _
+ }
+
+ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
+ // SAFETY: 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 `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.
+ unsafe { ArcBorrow::new(inner) }
+ }
+
+ unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
+ // SAFETY: By the safety requirement of this function, we know that `ptr` came from
+ // a previous call to `Arc::into_foreign`, which owned guarantees that `ptr` is valid and
+ // owns a reference.
+ unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
+ }
+}
+
impl<T: ?Sized> Deref for Arc<T> {
type Target = T;
--
2.34.1
On Thu, Jan 19, 2023 at 02:40:32PM -0300, Wedson Almeida Filho wrote:
> This allows us to run some code when the guard is dropped (e.g.,
> implicitly when it goes out of scope). We can also prevent the
> guard from running by calling its `dismiss()` method.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/types.rs | 127 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e84e51ec9716..f0ad4472292d 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,7 +2,132 @@
>
> //! Kernel types.
>
> -use core::{cell::UnsafeCell, mem::MaybeUninit};
> +use alloc::boxed::Box;
> +use core::{
> + cell::UnsafeCell,
> + mem::MaybeUninit,
> + ops::{Deref, DerefMut},
> +};
> +
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +/// # Examples
> +///
> +/// In the example below, we have multiple exit paths and we want to log regardless of which one is
> +/// taken:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example1(arg: bool) {
> +/// let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
> +///
> +/// if arg {
> +/// return;
> +/// }
> +///
> +/// pr_info!("Do something...\n");
> +/// }
> +///
> +/// # example1(false);
> +/// # example1(true);
> +/// ```
> +///
> +/// In the example below, we want to log the same message on all early exits but a different one on
> +/// the main exit path:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example2(arg: bool) {
> +/// let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
> +///
> +/// if arg {
> +/// return;
> +/// }
> +///
> +/// // (Other early returns...)
> +///
> +/// log.dismiss();
> +/// pr_info!("example2 no early return\n");
> +/// }
> +///
> +/// # example2(false);
> +/// # example2(true);
> +/// ```
> +///
> +/// In the example below, we need a mutable object (the vector) to be accessible within the log
> +/// function, so we wrap it in the [`ScopeGuard`]:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example3(arg: bool) -> Result {
> +/// let mut vec =
> +/// ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
> +///
> +/// vec.try_push(10u8)?;
> +/// if arg {
> +/// return Ok(());
> +/// }
> +/// vec.try_push(20u8)?;
> +/// Ok(())
> +/// }
> +///
> +/// # assert_eq!(example3(false), Ok(()));
> +/// # assert_eq!(example3(true), Ok(()));
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// The value stored in the struct is nearly always `Some(_)`, except between
> +/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
> +/// will have been returned to the caller. Since [`ScopeGuard::dismiss`] consumes the guard,
> +/// callers won't be able to use it anymore.
> +pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
> +
> +impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
> + /// Creates a new guarded object wrapping the given data and with the given cleanup function.
> + pub fn new_with_data(data: T, cleanup_func: F) -> Self {
> + // INVARIANT: The struct is being initialised with `Some(_)`.
> + Self(Some((data, cleanup_func)))
> + }
> +
> + /// Prevents the cleanup function from running and returns the guarded data.
> + pub fn dismiss(mut self) -> T {
> + // INVARIANT: This is the exception case in the invariant; it is not visible to callers
> + // because this function consumes `self`.
> + self.0.take().unwrap().0
> + }
> +}
> +
> +impl ScopeGuard<(), Box<dyn FnOnce(())>> {
How about `fn(())` here as a placeholder? I.e
impl ScopeGuard<(), fn(())>
Regards,
Boqun
> + /// Creates a new guarded object with the given cleanup function.
> + pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
> + ScopeGuard::new_with_data((), move |_| cleanup())
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
> + type Target = T;
> +
> + fn deref(&self) -> &T {
> + // The type invariants guarantee that `unwrap` will succeed.
> + &self.0.as_ref().unwrap().0
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
> + fn deref_mut(&mut self) -> &mut T {
> + // The type invariants guarantee that `unwrap` will succeed.
> + &mut self.0.as_mut().unwrap().0
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
> + fn drop(&mut self) {
> + // Run the cleanup function if one is still present.
> + if let Some((data, cleanup)) = self.0.take() {
> + cleanup(data)
> + }
> + }
> +}
>
> /// Stores an opaque value.
> ///
> --
> 2.34.1
>
On Thu, Jan 19, 2023 at 02:40:33PM -0300, Wedson Almeida Filho 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 <[email protected]>
> ---
> 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)> {
> + // 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();
> + })
I kinda want to suggest borrow_mut() to be implemented as:
pub trait ForeignOwnable {
...
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();
})
}
to avoid funny code as follow:
let a = Box::new(0).into_foreign();
// Using an irrelevant `impl ForeignOwnable` to `borrow_mut`
let borrowed_a: ScopeGuard<Box<i32>, ...> = unsafe { Arc::<u64>::borrow_mut(a) };
but that requires `Self: Sized`. Is it too restrictive?
Regards,
Boqun
> + }
> +
> + /// 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.
> --
> 2.34.1
>
On Fri, 20 Jan 2023 at 16:59, Boqun Feng <[email protected]> wrote:
>
> On Thu, Jan 19, 2023 at 02:40:33PM -0300, Wedson Almeida Filho 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 <[email protected]>
> > ---
> > 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)> {
> > + // 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();
> > + })
>
> I kinda want to suggest borrow_mut() to be implemented as:
>
> pub trait ForeignOwnable {
> ...
> 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();
> })
> }
Oh, eliminate the generic type `T` and just use `Self`. Yes, I think
that's a good simplification.
>
> to avoid funny code as follow:
>
> let a = Box::new(0).into_foreign();
> // Using an irrelevant `impl ForeignOwnable` to `borrow_mut`
> let borrowed_a: ScopeGuard<Box<i32>, ...> = unsafe { Arc::<u64>::borrow_mut(a) };
>
> but that requires `Self: Sized`. Is it too restrictive?
It isn't. Since we want the raw pointer to fit in a single pointer
(from C's perspective), we already require `Self: Sized`.
I'll make this change in v2.
>
> Regards,
> Boqun
>
> > + }
> > +
> > + /// 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.
> > --
> > 2.34.1
> >
On Fri, 20 Jan 2023 at 03:24, Boqun Feng <[email protected]> wrote:
>
> On Thu, Jan 19, 2023 at 02:40:32PM -0300, Wedson Almeida Filho wrote:
> > This allows us to run some code when the guard is dropped (e.g.,
> > implicitly when it goes out of scope). We can also prevent the
> > guard from running by calling its `dismiss()` method.
> >
> > Signed-off-by: Wedson Almeida Filho <[email protected]>
> > ---
> > rust/kernel/types.rs | 127 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 126 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index e84e51ec9716..f0ad4472292d 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -2,7 +2,132 @@
> >
> > //! Kernel types.
> >
> > -use core::{cell::UnsafeCell, mem::MaybeUninit};
> > +use alloc::boxed::Box;
> > +use core::{
> > + cell::UnsafeCell,
> > + mem::MaybeUninit,
> > + ops::{Deref, DerefMut},
> > +};
> > +
> > +/// Runs a cleanup function/closure when dropped.
> > +///
> > +/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> > +///
> > +/// # Examples
> > +///
> > +/// In the example below, we have multiple exit paths and we want to log regardless of which one is
> > +/// taken:
> > +/// ```
> > +/// # use kernel::ScopeGuard;
> > +/// fn example1(arg: bool) {
> > +/// let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
> > +///
> > +/// if arg {
> > +/// return;
> > +/// }
> > +///
> > +/// pr_info!("Do something...\n");
> > +/// }
> > +///
> > +/// # example1(false);
> > +/// # example1(true);
> > +/// ```
> > +///
> > +/// In the example below, we want to log the same message on all early exits but a different one on
> > +/// the main exit path:
> > +/// ```
> > +/// # use kernel::ScopeGuard;
> > +/// fn example2(arg: bool) {
> > +/// let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
> > +///
> > +/// if arg {
> > +/// return;
> > +/// }
> > +///
> > +/// // (Other early returns...)
> > +///
> > +/// log.dismiss();
> > +/// pr_info!("example2 no early return\n");
> > +/// }
> > +///
> > +/// # example2(false);
> > +/// # example2(true);
> > +/// ```
> > +///
> > +/// In the example below, we need a mutable object (the vector) to be accessible within the log
> > +/// function, so we wrap it in the [`ScopeGuard`]:
> > +/// ```
> > +/// # use kernel::ScopeGuard;
> > +/// fn example3(arg: bool) -> Result {
> > +/// let mut vec =
> > +/// ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
> > +///
> > +/// vec.try_push(10u8)?;
> > +/// if arg {
> > +/// return Ok(());
> > +/// }
> > +/// vec.try_push(20u8)?;
> > +/// Ok(())
> > +/// }
> > +///
> > +/// # assert_eq!(example3(false), Ok(()));
> > +/// # assert_eq!(example3(true), Ok(()));
> > +/// ```
> > +///
> > +/// # Invariants
> > +///
> > +/// The value stored in the struct is nearly always `Some(_)`, except between
> > +/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
> > +/// will have been returned to the caller. Since [`ScopeGuard::dismiss`] consumes the guard,
> > +/// callers won't be able to use it anymore.
> > +pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
> > +
> > +impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
> > + /// Creates a new guarded object wrapping the given data and with the given cleanup function.
> > + pub fn new_with_data(data: T, cleanup_func: F) -> Self {
> > + // INVARIANT: The struct is being initialised with `Some(_)`.
> > + Self(Some((data, cleanup_func)))
> > + }
> > +
> > + /// Prevents the cleanup function from running and returns the guarded data.
> > + pub fn dismiss(mut self) -> T {
> > + // INVARIANT: This is the exception case in the invariant; it is not visible to callers
> > + // because this function consumes `self`.
> > + self.0.take().unwrap().0
> > + }
> > +}
> > +
> > +impl ScopeGuard<(), Box<dyn FnOnce(())>> {
>
> How about `fn(())` here as a placeholder? I.e
>
> impl ScopeGuard<(), fn(())>
>
That's simpler, I like it. I'll change this for v2. Thanks!
> Regards,
> Boqun
>
> > + /// Creates a new guarded object with the given cleanup function.
> > + pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
> > + ScopeGuard::new_with_data((), move |_| cleanup())
> > + }
> > +}
> > +
> > +impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &T {
> > + // The type invariants guarantee that `unwrap` will succeed.
> > + &self.0.as_ref().unwrap().0
> > + }
> > +}
> > +
> > +impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
> > + fn deref_mut(&mut self) -> &mut T {
> > + // The type invariants guarantee that `unwrap` will succeed.
> > + &mut self.0.as_mut().unwrap().0
> > + }
> > +}
> > +
> > +impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
> > + fn drop(&mut self) {
> > + // Run the cleanup function if one is still present.
> > + if let Some((data, cleanup)) = self.0.take() {
> > + cleanup(data)
> > + }
> > + }
> > +}
> >
> > /// Stores an opaque value.
> > ///
> > --
> > 2.34.1
> >
On Thu, 19 Jan 2023 14:40:33 -0300
Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> ---
> 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`.
> + // 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.
On Thu, 19 Jan 2023 14:40:34 -0300
Wedson Almeida Filho <[email protected]> wrote:
> This allows us to hand ownership of Rust dynamically allocated
> objects to the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/types.rs | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 5475f6163002..e037c262f23e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -63,6 +63,28 @@ pub trait ForeignOwnable {
> unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
> }
>
> +impl<T: 'static> ForeignOwnable for Box<T> {
> + type Borrowed<'a> = &'a T;
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + Box::into_raw(self) as _
> + }
> +
> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
> + // SAFETY: The safety requirements for this function ensure that the object is still alive,
> + // so it is safe to dereference the raw pointer.
> + // The safety requirements of `from_foreign` also ensure that the object remains alive for
> + // the lifetime of the returned value.
> + unsafe { &*ptr.cast() }
> + }
> +
> + 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`.
> + unsafe { Box::from_raw(ptr as _) }
> + }
> +}
> +
> /// Runs a cleanup function/closure when dropped.
> ///
> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
On Thu, 19 Jan 2023 14:40:35 -0300
Wedson Almeida Filho <[email protected]> wrote:
> This allows us to use the unit type `()` when we have no object whose
> ownership must be managed but one implementing the `ForeignOwnable`
> trait is needed.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/types.rs | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e037c262f23e..8f80cffbff59 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -85,6 +85,19 @@ impl<T: 'static> ForeignOwnable for Box<T> {
> }
> }
>
> +impl ForeignOwnable for () {
> + type Borrowed<'a> = ();
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + // We use 1 to be different from a null pointer.
> + 1usize as _
this should really be `core::ptr::invalid(1)`. That's currently
unstable, but can be equivalently written as
`NonNull::<()>::dangling().as_ptr()`.
This has a different semantic meaning from `as` since it explicitly
suggests an invalid provenance and thus will not alias with other
pointers. (Although I don't think compiler currently can take advantage
of this fact yet)
> + }
> +
> + unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
> +
> + unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
> +}
> +
> /// Runs a cleanup function/closure when dropped.
> ///
> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
On Thu, 19 Jan 2023 14:40:36 -0300
Wedson Almeida Filho <[email protected]> wrote:
> This allows us to hand ownership of Rust ref-counted objects to
> the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/sync/arc.rs | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index ff73f9240ca1..519a6ec43644 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -15,7 +15,11 @@
> //!
> //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
>
> -use crate::{bindings, error::Result, types::Opaque};
> +use crate::{
> + bindings,
> + error::Result,
> + types::{ForeignOwnable, Opaque},
> +};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> @@ -189,6 +193,32 @@ impl<T: ?Sized> Arc<T> {
> }
> }
>
> +impl<T: 'static> ForeignOwnable for Arc<T> {
> + type Borrowed<'a> = ArcBorrow<'a, T>;
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + ManuallyDrop::new(self).ptr.as_ptr() as _
> + }
> +
> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
> + // SAFETY: 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 `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.
> + unsafe { ArcBorrow::new(inner) }
> + }
> +
> + unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
> + // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> + // a previous call to `Arc::into_foreign`, which owned guarantees that `ptr` is valid and
> + // owns a reference.
> + unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
> + }
> +}
> +
> impl<T: ?Sized> Deref for Arc<T> {
> type Target = T;
>
On Fri, Jan 27, 2023 at 3:03 PM Gary Guo <[email protected]> wrote:
>
> this should really be `core::ptr::invalid(1)`. That's currently
> unstable, but can be equivalently written as
> `NonNull::<()>::dangling().as_ptr()`.
>
> This has a different semantic meaning from `as` since it explicitly
> suggests an invalid provenance and thus will not alias with other
> pointers. (Although I don't think compiler currently can take advantage
> of this fact yet)
We talked about starting to use `strict_provenance` when it came out
-- what is the latest status? i.e. do you know if it is expected that
it will pass FCP etc.? (my understanding originally was that it was an
experiment).
If it is likely to become stable, then I agree it could be nice to
start using it before a lot of kernel code gets written without it.
Thanks Gary!
Cheers,
Miguel
On Thu Jan 19, 2023 at 6:40 PM CET, Wedson Almeida Filho wrote:
> This allows us to run some code when the guard is dropped (e.g.,
> implicitly when it goes out of scope). We can also prevent the
> guard from running by calling its `dismiss()` method.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
Reviewed-by: Vincenzo Palazzo <[email protected]>
> rust/kernel/types.rs | 127 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e84e51ec9716..f0ad4472292d 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -2,7 +2,132 @@
>
> //! Kernel types.
>
> -use core::{cell::UnsafeCell, mem::MaybeUninit};
> +use alloc::boxed::Box;
> +use core::{
> + cell::UnsafeCell,
> + mem::MaybeUninit,
> + ops::{Deref, DerefMut},
> +};
> +
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +/// # Examples
> +///
> +/// In the example below, we have multiple exit paths and we want to log regardless of which one is
> +/// taken:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example1(arg: bool) {
> +/// let _log = ScopeGuard::new(|| pr_info!("example1 completed\n"));
> +///
> +/// if arg {
> +/// return;
> +/// }
> +///
> +/// pr_info!("Do something...\n");
> +/// }
> +///
> +/// # example1(false);
> +/// # example1(true);
> +/// ```
> +///
> +/// In the example below, we want to log the same message on all early exits but a different one on
> +/// the main exit path:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example2(arg: bool) {
> +/// let log = ScopeGuard::new(|| pr_info!("example2 returned early\n"));
> +///
> +/// if arg {
> +/// return;
> +/// }
> +///
> +/// // (Other early returns...)
> +///
> +/// log.dismiss();
> +/// pr_info!("example2 no early return\n");
> +/// }
> +///
> +/// # example2(false);
> +/// # example2(true);
> +/// ```
> +///
> +/// In the example below, we need a mutable object (the vector) to be accessible within the log
> +/// function, so we wrap it in the [`ScopeGuard`]:
> +/// ```
> +/// # use kernel::ScopeGuard;
> +/// fn example3(arg: bool) -> Result {
> +/// let mut vec =
> +/// ScopeGuard::new_with_data(Vec::new(), |v| pr_info!("vec had {} elements\n", v.len()));
> +///
> +/// vec.try_push(10u8)?;
> +/// if arg {
> +/// return Ok(());
> +/// }
> +/// vec.try_push(20u8)?;
> +/// Ok(())
> +/// }
> +///
> +/// # assert_eq!(example3(false), Ok(()));
> +/// # assert_eq!(example3(true), Ok(()));
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// The value stored in the struct is nearly always `Some(_)`, except between
> +/// [`ScopeGuard::dismiss`] and [`ScopeGuard::drop`]: in this case, it will be `None` as the value
> +/// will have been returned to the caller. Since [`ScopeGuard::dismiss`] consumes the guard,
> +/// callers won't be able to use it anymore.
> +pub struct ScopeGuard<T, F: FnOnce(T)>(Option<(T, F)>);
> +
> +impl<T, F: FnOnce(T)> ScopeGuard<T, F> {
> + /// Creates a new guarded object wrapping the given data and with the given cleanup function.
> + pub fn new_with_data(data: T, cleanup_func: F) -> Self {
> + // INVARIANT: The struct is being initialised with `Some(_)`.
> + Self(Some((data, cleanup_func)))
> + }
> +
> + /// Prevents the cleanup function from running and returns the guarded data.
> + pub fn dismiss(mut self) -> T {
> + // INVARIANT: This is the exception case in the invariant; it is not visible to callers
> + // because this function consumes `self`.
> + self.0.take().unwrap().0
> + }
> +}
> +
> +impl ScopeGuard<(), Box<dyn FnOnce(())>> {
> + /// Creates a new guarded object with the given cleanup function.
> + pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
> + ScopeGuard::new_with_data((), move |_| cleanup())
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> Deref for ScopeGuard<T, F> {
> + type Target = T;
> +
> + fn deref(&self) -> &T {
> + // The type invariants guarantee that `unwrap` will succeed.
> + &self.0.as_ref().unwrap().0
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> DerefMut for ScopeGuard<T, F> {
> + fn deref_mut(&mut self) -> &mut T {
> + // The type invariants guarantee that `unwrap` will succeed.
> + &mut self.0.as_mut().unwrap().0
> + }
> +}
> +
> +impl<T, F: FnOnce(T)> Drop for ScopeGuard<T, F> {
> + fn drop(&mut self) {
> + // Run the cleanup function if one is still present.
> + if let Some((data, cleanup)) = self.0.take() {
> + cleanup(data)
> + }
> + }
> +}
>
> /// Stores an opaque value.
> ///
> --
> 2.34.1
On Thu Jan 19, 2023 at 6:40 PM CET, Wedson Almeida Filho 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 <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
> ---
> 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)> {
> + // 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.
> --
> 2.34.1
On Thu Jan 19, 2023 at 6:40 PM CET, Wedson Almeida Filho wrote:
> This allows us to hand ownership of Rust dynamically allocated
> objects to the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
Reviewed-by: Vincenzo Palazzo <[email protected]>
> 1 file changed, 22 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 5475f6163002..e037c262f23e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -63,6 +63,28 @@ pub trait ForeignOwnable {
> unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
> }
>
> +impl<T: 'static> ForeignOwnable for Box<T> {
> + type Borrowed<'a> = &'a T;
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + Box::into_raw(self) as _
> + }
> +
> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
> + // SAFETY: The safety requirements for this function ensure that the object is still alive,
> + // so it is safe to dereference the raw pointer.
> + // The safety requirements of `from_foreign` also ensure that the object remains alive for
> + // the lifetime of the returned value.
> + unsafe { &*ptr.cast() }
> + }
> +
> + 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`.
> + unsafe { Box::from_raw(ptr as _) }
> + }
> +}
> +
> /// Runs a cleanup function/closure when dropped.
> ///
> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> --
> 2.34.1
On Fri Jan 27, 2023 at 3:11 PM CET, Miguel Ojeda wrote:
> On Fri, Jan 27, 2023 at 3:03 PM Gary Guo <[email protected]> wrote:
> >
> > this should really be `core::ptr::invalid(1)`. That's currently
> > unstable, but can be equivalently written as
> > `NonNull::<()>::dangling().as_ptr()`.
> >
> > This has a different semantic meaning from `as` since it explicitly
> > suggests an invalid provenance and thus will not alias with other
> > pointers. (Although I don't think compiler currently can take advantage
> > of this fact yet)
>
> We talked about starting to use `strict_provenance` when it came out
> -- what is the latest status? i.e. do you know if it is expected that
> it will pass FCP etc.? (my understanding originally was that it was an
> experiment).
From what I remember the feeling was positing into hace `strict_provenance`
Here is the last meeting that was back in August
https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Stabilizing.20strict.20provenance.20APIs.3F
I guess, we could just put a fix me around the actual code, I feel that the Gary change deserve a
own patch with the own description.
Cheers!
Vincent.
On Thu Jan 19, 2023 at 6:40 PM CET, Wedson Almeida Filho wrote:
> This allows us to use the unit type `()` when we have no object whose
> ownership must be managed but one implementing the `ForeignOwnable`
> trait is needed.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
> ---
> rust/kernel/types.rs | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index e037c262f23e..8f80cffbff59 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -85,6 +85,19 @@ impl<T: 'static> ForeignOwnable for Box<T> {
> }
> }
>
> +impl ForeignOwnable for () {
> + type Borrowed<'a> = ();
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + // We use 1 to be different from a null pointer.
> + 1usize as _
> + }
> +
> + unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
> +
> + unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
> +}
> +
> /// Runs a cleanup function/closure when dropped.
> ///
> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
> --
> 2.34.1
On Thu Jan 19, 2023 at 6:40 PM CET, Wedson Almeida Filho wrote:
> This allows us to hand ownership of Rust ref-counted objects to
> the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
Reviewed-by: Vincenzo Palazzo <[email protected]>
> rust/kernel/sync/arc.rs | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index ff73f9240ca1..519a6ec43644 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -15,7 +15,11 @@
> //!
> //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
>
> -use crate::{bindings, error::Result, types::Opaque};
> +use crate::{
> + bindings,
> + error::Result,
> + types::{ForeignOwnable, Opaque},
> +};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> @@ -189,6 +193,32 @@ impl<T: ?Sized> Arc<T> {
> }
> }
>
> +impl<T: 'static> ForeignOwnable for Arc<T> {
> + type Borrowed<'a> = ArcBorrow<'a, T>;
> +
> + fn into_foreign(self) -> *const core::ffi::c_void {
> + ManuallyDrop::new(self).ptr.as_ptr() as _
> + }
> +
> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
> + // SAFETY: 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 `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.
> + unsafe { ArcBorrow::new(inner) }
> + }
> +
> + unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
> + // SAFETY: By the safety requirement of this function, we know that `ptr` came from
> + // a previous call to `Arc::into_foreign`, which owned guarantees that `ptr` is valid and
> + // owns a reference.
> + unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
> + }
> +}
> +
> impl<T: ?Sized> Deref for Arc<T> {
> type Target = T;
>
> --
> 2.34.1
On Thu, Jan 19, 2023 at 02:40:33PM -0300, Wedson Almeida Filho 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 <[email protected]>
> ---
> 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>;
Is it there a possibility that this could make use of borrowing features
like AsRef/Borrowed/Deref?
> +
> + /// 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)> {
> + // 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();
> + })
> + }
Could these three methods have a borrowing equivalent? When I was
working on some features for the USB module I've stumbled upon the case
of having to encode a pointer (with a pivot) and I cannot do it without
taking ownership of the pointer.
> +
> + /// 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.
> --
> 2.34.1
Aside from these comments I observe that there's a possibility to make
ForeignOwnable a const trait and have non const implementors. Otherwise
if these things are out of scope, no problem whatsoever and this has my
OK.
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote:
[...]
> > + /// 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)> {
> > + // 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();
> > + })
> > + }
>
> Could these three methods have a borrowing equivalent? When I was
> working on some features for the USB module I've stumbled upon the case
> of having to encode a pointer (with a pivot) and I cannot do it without
> taking ownership of the pointer.
>
*const T is Copy, so you can still use it after pass it to a function or
a new binding, e.g.
pub fn use_ptr(ptr: *const i32) { .. }
let p: *const i32 = some_func();
let q = p;
// q is just a copy of p
use_ptr(p);
// passing to a function parameter is just copying
use_ptr(p);
maybe I'm missing something subtle, but if you have an example I can
help take a look.
Regards,
Boqun
> > +
> > + /// 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.
> > --
> > 2.34.1
>
> Aside from these comments I observe that there's a possibility to make
> ForeignOwnable a const trait and have non const implementors. Otherwise
> if these things are out of scope, no problem whatsoever and this has my
> OK.
>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On 1/28/23 14:05, Boqun Feng wrote:
> On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote:
> [...]
>>> + /// 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)> {
>>> + // 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();
>>> + })
>>> + }
>>
>> Could these three methods have a borrowing equivalent? When I was
>> working on some features for the USB module I've stumbled upon the case
>> of having to encode a pointer (with a pivot) and I cannot do it without
>> taking ownership of the pointer.
>>
>
> *const T is Copy, so you can still use it after pass it to a function or
> a new binding, e.g.
>
> pub fn use_ptr(ptr: *const i32) { .. }
>
> let p: *const i32 = some_func();
>
> let q = p;
>
> // q is just a copy of p
> use_ptr(p);
> // passing to a function parameter is just copying
> use_ptr(p);
>
> maybe I'm missing something subtle, but if you have an example I can
> help take a look.
>
> Regards,
> Boqun
>
I'll use a much more simple example. If I want to take the byte offset
between two `ForeignWrapper`s I'd have to take ownership of them, but I
don't see it desirable in some cases.
fn byte_offset<P: PointerWrapper>(ptr: P, pivot: P) -> isize {
unsafe {
ptr.into_pointer().cast::<u8>()
.byte_offset(pivot.into_pointer().cast())
}
}
But if there was an `as_pointer(&self) -> *const c_void` method then the
above function will be able to borrow both `ForeignWrapper`s.
fn byte_offset<P: PointerWrapper>(ptr: &P, pivot: &P) -> isize {
unsafe {
ptr.as_pointer().cast::<u8>()
.byte_offset(pivot.as_pointer().cast())
}
}
Obviously those methods that borrow will announce invariancies in their
doc comments. If these can exist then great and if not then another
solution could be explored.
>>> +
>>> + /// 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.
>>> --
>>> 2.34.1
>>
>> Aside from these comments I observe that there's a possibility to make
>> ForeignOwnable a const trait and have non const implementors. Otherwise
>> if these things are out of scope, no problem whatsoever and this has my
>> OK.
>>
>> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On Sat, Jan 28, 2023 at 05:46:50PM -0300, Martin Rodriguez Reboredo wrote:
> On 1/28/23 14:05, Boqun Feng wrote:
> > On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote:
> > [...]
> >>> + /// 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)> {
> >>> + // 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();
> >>> + })
> >>> + }
> >>
> >> Could these three methods have a borrowing equivalent? When I was
> >> working on some features for the USB module I've stumbled upon the case
> >> of having to encode a pointer (with a pivot) and I cannot do it without
> >> taking ownership of the pointer.
> >>
> >
> > *const T is Copy, so you can still use it after pass it to a function or
> > a new binding, e.g.
> >
> > pub fn use_ptr(ptr: *const i32) { .. }
> >
> > let p: *const i32 = some_func();
> >
> > let q = p;
> >
> > // q is just a copy of p
> > use_ptr(p);
> > // passing to a function parameter is just copying
> > use_ptr(p);
> >
> > maybe I'm missing something subtle, but if you have an example I can
> > help take a look.
> >
> > Regards,
> > Boqun
> >
>
> I'll use a much more simple example. If I want to take the byte offset
> between two `ForeignWrapper`s I'd have to take ownership of them, but I
> don't see it desirable in some cases.
>
> fn byte_offset<P: PointerWrapper>(ptr: P, pivot: P) -> isize {
> unsafe {
> ptr.into_pointer().cast::<u8>()
> .byte_offset(pivot.into_pointer().cast())
> }
> }
>
> But if there was an `as_pointer(&self) -> *const c_void` method then the
> above function will be able to borrow both `ForeignWrapper`s.
>
> fn byte_offset<P: PointerWrapper>(ptr: &P, pivot: &P) -> isize {
> unsafe {
> ptr.as_pointer().cast::<u8>()
> .byte_offset(pivot.as_pointer().cast())
> }
> }
>
> Obviously those methods that borrow will announce invariancies in their
> doc comments. If these can exist then great and if not then another
> solution could be explored.
>
For this particular use case, IIUC, what you need is exactly `AsRef`:
fn byte_offset<T, P: AsRef<T>>(ptr: &P, pivot: &P) -> isize {
unsafe {
(ptr.as_ref() as *const _).cast::<u8>()
.byte_offset((pivot.as_ref() *const _).cast())
}
}
or you can implement a as_pointer() helper function to avoid the
duplicate `.as_ref() as *const _`:
fn as_pointer<T, P: AsRef<T>>(ptr: &P) -> *const T {
ptr.as_ref() as *const T
}
Although, we need to `impl AsRef<T> for Arc<T>` to make it work for
`Arc<T>`, which is currently missing.
Regards,
Boqun
> >>> +
> >>> + /// 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.
> >>> --
> >>> 2.34.1
> >>
> >> Aside from these comments I observe that there's a possibility to make
> >> ForeignOwnable a const trait and have non const implementors. Otherwise
> >> if these things are out of scope, no problem whatsoever and this has my
> >> OK.
> >>
> >> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On 1/28/23 19:07, Boqun Feng wrote:
> On Sat, Jan 28, 2023 at 05:46:50PM -0300, Martin Rodriguez Reboredo wrote:
>> On 1/28/23 14:05, Boqun Feng wrote:
>>> On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote:
>>> [...]
>>>>> + /// 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)> {
>>>>> + // 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();
>>>>> + })
>>>>> + }
>>>>
>>>> Could these three methods have a borrowing equivalent? When I was
>>>> working on some features for the USB module I've stumbled upon the case
>>>> of having to encode a pointer (with a pivot) and I cannot do it without
>>>> taking ownership of the pointer.
>>>>
>>>
>>> *const T is Copy, so you can still use it after pass it to a function or
>>> a new binding, e.g.
>>>
>>> pub fn use_ptr(ptr: *const i32) { .. }
>>>
>>> let p: *const i32 = some_func();
>>>
>>> let q = p;
>>>
>>> // q is just a copy of p
>>> use_ptr(p);
>>> // passing to a function parameter is just copying
>>> use_ptr(p);
>>>
>>> maybe I'm missing something subtle, but if you have an example I can
>>> help take a look.
>>>
>>> Regards,
>>> Boqun
>>>
>>
>> I'll use a much more simple example. If I want to take the byte offset
>> between two `ForeignWrapper`s I'd have to take ownership of them, but I
>> don't see it desirable in some cases.
>>
>> fn byte_offset<P: PointerWrapper>(ptr: P, pivot: P) -> isize {
>> unsafe {
>> ptr.into_pointer().cast::<u8>()
>> .byte_offset(pivot.into_pointer().cast())
>> }
>> }
>>
>> But if there was an `as_pointer(&self) -> *const c_void` method then the
>> above function will be able to borrow both `ForeignWrapper`s.
>>
>> fn byte_offset<P: PointerWrapper>(ptr: &P, pivot: &P) -> isize {
>> unsafe {
>> ptr.as_pointer().cast::<u8>()
>> .byte_offset(pivot.as_pointer().cast())
>> }
>> }
>>
>> Obviously those methods that borrow will announce invariancies in their
>> doc comments. If these can exist then great and if not then another
>> solution could be explored.
>>
>
> For this particular use case, IIUC, what you need is exactly `AsRef`:
>
> fn byte_offset<T, P: AsRef<T>>(ptr: &P, pivot: &P) -> isize {
> unsafe {
> (ptr.as_ref() as *const _).cast::<u8>()
> .byte_offset((pivot.as_ref() *const _).cast())
> }
> }
>
> or you can implement a as_pointer() helper function to avoid the
> duplicate `.as_ref() as *const _`:
>
> fn as_pointer<T, P: AsRef<T>>(ptr: &P) -> *const T {
> ptr.as_ref() as *const T
> }
>
> Although, we need to `impl AsRef<T> for Arc<T>` to make it work for
> `Arc<T>`, which is currently missing.
>
> Regards,
> Boqun
>
Now that you say it I think that I can do what I wanted by further
constraining it to `ForeignOwnable` plus `AsRef`, so thanks for the tip
Boqun.
>>>>> +
>>>>> + /// 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.
>>>>> --
>>>>> 2.34.1
>>>>
>>>> Aside from these comments I observe that there's a possibility to make
>>>> ForeignOwnable a const trait and have non const implementors. Otherwise
>>>> if these things are out of scope, no problem whatsoever and this has my
>>>> OK.
>>>>
>>>> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On Fri, Jan 20, 2023 at 2:41 AM Wedson Almeida Filho <[email protected]> wrote:
>
> This allows us to hand ownership of Rust dynamically allocated
> objects to the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
Reviewed-by: Alice Ferrazzi <[email protected]>
--
======================================
Cybertrust Japan Co.,Ltd.
Alice Ferrazzi
[email protected]
======================================
On Fri, Jan 20, 2023 at 2:41 AM Wedson Almeida Filho <[email protected]> wrote:
>
> This allows us to hand ownership of Rust ref-counted objects to
> the C side of the kernel.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Alice Ferrazzi <[email protected]>
--
======================================
Cybertrust Japan Co.,Ltd.
Alice Ferrazzi
[email protected]
======================================
On Fri, 27 Jan 2023 at 11:03, Gary Guo <[email protected]> wrote:
>
> On Thu, 19 Jan 2023 14:40:35 -0300
> Wedson Almeida Filho <[email protected]> wrote:
>
> > This allows us to use the unit type `()` when we have no object whose
> > ownership must be managed but one implementing the `ForeignOwnable`
> > trait is needed.
> >
> > Signed-off-by: Wedson Almeida Filho <[email protected]>
> > ---
> > rust/kernel/types.rs | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index e037c262f23e..8f80cffbff59 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -85,6 +85,19 @@ impl<T: 'static> ForeignOwnable for Box<T> {
> > }
> > }
> >
> > +impl ForeignOwnable for () {
> > + type Borrowed<'a> = ();
> > +
> > + fn into_foreign(self) -> *const core::ffi::c_void {
> > + // We use 1 to be different from a null pointer.
> > + 1usize as _
>
> this should really be `core::ptr::invalid(1)`. That's currently
> unstable, but can be equivalently written as
> `NonNull::<()>::dangling().as_ptr()`.
This has the wrong type, but I agree with the spirit of the
suggestion. I'll add it to V2.
> This has a different semantic meaning from `as` since it explicitly
> suggests an invalid provenance and thus will not alias with other
> pointers. (Although I don't think compiler currently can take advantage
> of this fact yet)
>
> > + }
> > +
> > + unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
> > +
> > + unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
> > +}
> > +
> > /// Runs a cleanup function/closure when dropped.
> > ///
> > /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
>
On Fri, 27 Jan 2023 at 10:55, Gary Guo <[email protected]> wrote:
>
> On Thu, 19 Jan 2023 14:40:33 -0300
> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> > ---
> > 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.
>
On Sat, Jan 28, 2023 at 12:13 PM Vincenzo Palazzo
<[email protected]> wrote:
>
> From what I remember the feeling was positing into hace `strict_provenance`
>
> Here is the last meeting that was back in August
> https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Stabilizing.20strict.20provenance.20APIs.3F
Thanks Vincent! Yeah, as far as I understand Gary was in that meeting
(in October).
If there has not been anything after that, then I guess it is still
the case that provenance will probably happen (for better or worse,
depending on the perspective :) and that a subset of the APIs may get
stabilized first.
Cheers,
Miguel