Messages in this thread | | | Date | Sat, 24 Jun 2023 14:58:50 +0000 | From | Björn Roy Baron <> | Subject | Re: [PATCH 3/7] rust: init: make guards in the init macros hygienic |
| |
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Use hygienic identifiers for the guards instead of the field names. This > makes the init macros feel more like normal struct initializers, since > assigning identifiers with the name of a field does not create > conflicts. > Also change the internals of the guards, no need to make the `forget` > function `unsafe`, since users cannot access the guards anyways. Now the > guards are carried directly on the stack and have no extra `Cell<bool>` > field that marks if they have been forgotten or not, instead they are > just forgotten via `mem::forget`. > > Suggested-by: Asahi Lina <lina@asahilina.net> > Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> --- > rust/kernel/init.rs | 1 - > rust/kernel/init/__internal.rs | 25 +++------------ > rust/kernel/init/macros.rs | 56 ++++++++++++---------------------- > 3 files changed, 23 insertions(+), 59 deletions(-) > > diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs > index d9a91950cba2..ecf6a4bd0ce4 100644 > --- a/rust/kernel/init.rs > +++ b/rust/kernel/init.rs > @@ -206,7 +206,6 @@ > use alloc::boxed::Box; > use core::{ > alloc::AllocError, > - cell::Cell, > convert::Infallible, > marker::PhantomData, > mem::MaybeUninit, > diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs > index 44751fb62b51..7abd1fb65e41 100644 > --- a/rust/kernel/init/__internal.rs > +++ b/rust/kernel/init/__internal.rs > @@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu > /// Can be forgotten to prevent the drop. > pub struct DropGuard<T: ?Sized> { > ptr: *mut T, > - do_drop: Cell<bool>, > } > > impl<T: ?Sized> DropGuard<T> { > @@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> { > /// - will not be dropped by any other means. > #[inline] > pub unsafe fn new(ptr: *mut T) -> Self { > - Self { > - ptr, > - do_drop: Cell::new(true), > - } > - } > - > - /// Prevents this guard from dropping the supplied pointer. > - /// > - /// # Safety > - /// > - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should > - /// only be called by the macros in this module. > - #[inline] > - pub unsafe fn forget(&self) { > - self.do_drop.set(false); > + Self { ptr } > } > } > > impl<T: ?Sized> Drop for DropGuard<T> { > #[inline] > fn drop(&mut self) { > - if self.do_drop.get() { > - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function > - // ensuring that this operation is safe. > - unsafe { ptr::drop_in_place(self.ptr) } > - } > + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function > + // ensuring that this operation is safe. > + unsafe { ptr::drop_in_place(self.ptr) } > } > } > > diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs > index e8165ff53a94..df4281743175 100644 > --- a/rust/kernel/init/macros.rs > +++ b/rust/kernel/init/macros.rs > @@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*> > /// - `init_slot`: recursively creates the code that initializes all fields in `slot`. > /// - `make_initializer`: recursively create the struct initializer that guarantees that every > /// field has been initialized exactly once. > -/// - `forget_guards`: recursively forget the drop guards for every field. > #[doc(hidden)] > #[macro_export] > macro_rules! __init_internal { > @@ -1032,6 +1031,7 @@ macro_rules! __init_internal { > $crate::__init_internal!(init_slot($($use_data)?): > @data(data), > @slot(slot), > + @guards(), > @munch_fields($($fields)*,), > ); > // We use unreachable code to ensure that all fields have been mentioned exactly > @@ -1046,10 +1046,6 @@ macro_rules! __init_internal { > @acc(), > ); > } > - // Forget all guards, since initialization was a success. > - $crate::__init_internal!(forget_guards: > - @munch_fields($($fields)*,), > - ); > } > Ok(__InitOk) > } > @@ -1063,13 +1059,17 @@ macro_rules! __init_internal { > (init_slot($($use_data:ident)?): > @data($data:ident), > @slot($slot:ident), > + @guards($($guards:ident,)*), > @munch_fields($(,)?), > ) => { > - // Endpoint of munching, no fields are left. > + // Endpoint of munching, no fields are left. If execution reaches this point, all fields > + // have been initialized. Therefore we can now dismiss the guards by forgetting them. > + $(::core::mem::forget($guards);)* > }; > (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields. > @data($data:ident), > @slot($slot:ident), > + @guards($($guards:ident,)*), > // In-place initialization syntax. > @munch_fields($field:ident <- $val:expr, $($rest:tt)*), > ) => { > @@ -1080,24 +1080,26 @@ macro_rules! __init_internal { > // return when an error/panic occurs. > // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`. > unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? }; > - // Create the drop guard. > + // Create the drop guard: > // > - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. > + // We rely on macro hygiene to make it impossible for users to access this local variable. > // > // SAFETY: We forget the guard later when initialization has succeeded. > - let $field = &unsafe { > + let guard = unsafe { > $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) > }; > > $crate::__init_internal!(init_slot($use_data): > @data($data), > @slot($slot), > + @guards(guard, $($guards,)*), > @munch_fields($($rest)*), > ); > }; > (init_slot(): // no use_data, so we use `Init::__init` directly. > @data($data:ident), > @slot($slot:ident), > + @guards($($guards:ident,)*), > // In-place initialization syntax. > @munch_fields($field:ident <- $val:expr, $($rest:tt)*), > ) => { > @@ -1107,24 +1109,26 @@ macro_rules! __init_internal { > // SAFETY: `slot` is valid, because we are inside of an initializer closure, we > // return when an error/panic occurs. > unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? }; > - // Create the drop guard. > + // Create the drop guard: > // > - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. > + // We rely on macro hygiene to make it impossible for users to access this local variable. > // > // SAFETY: We forget the guard later when initialization has succeeded. > - let $field = &unsafe { > + let guard = unsafe { > $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) > }; > > $crate::__init_internal!(init_slot(): > @data($data), > @slot($slot), > + @guards(guard, $($guards,)*), > @munch_fields($($rest)*), > ); > }; > (init_slot($($use_data:ident)?): > @data($data:ident), > @slot($slot:ident), > + @guards($($guards:ident,)*), > // Init by-value. > @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), > ) => { > @@ -1135,16 +1139,17 @@ macro_rules! __init_internal { > unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) }; > // Create the drop guard: > // > - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten. > + // We rely on macro hygiene to make it impossible for users to access this local variable. > // > // SAFETY: We forget the guard later when initialization has succeeded. > - let $field = &unsafe { > + let guard = unsafe { > $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) > }; > > $crate::__init_internal!(init_slot($($use_data)?): > @data($data), > @slot($slot), > + @guards(guard, $($guards,)*), > @munch_fields($($rest)*), > ); > }; > @@ -1189,29 +1194,6 @@ macro_rules! __init_internal { > @acc($($acc)* $field: ::core::panic!(),), > ); > }; > - (forget_guards: > - @munch_fields($(,)?), > - ) => { > - // Munching finished. > - }; > - (forget_guards: > - @munch_fields($field:ident <- $val:expr, $($rest:tt)*), > - ) => { > - unsafe { $crate::init::__internal::DropGuard::forget($field) }; > - > - $crate::__init_internal!(forget_guards: > - @munch_fields($($rest)*), > - ); > - }; > - (forget_guards: > - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), > - ) => { > - unsafe { $crate::init::__internal::DropGuard::forget($field) }; > - > - $crate::__init_internal!(forget_guards: > - @munch_fields($($rest)*), > - ); > - }; > } > > #[doc(hidden)] > -- > 2.41.0
| |