Messages in this thread Patch in this message | | | Date | Sat, 24 Jun 2023 09:25:10 +0000 | From | Benno Lossin <> | Subject | [PATCH 3/7] rust: init: make guards in the init macros hygienic |
| |
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> --- 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
| |