diff options
author | Benno Lossin <benno.lossin@proton.me> | 2023-08-14 08:46:48 +0000 |
---|---|---|
committer | Miguel Ojeda <ojeda@kernel.org> | 2023-08-21 14:31:48 +0200 |
commit | 97de919d574e6a22f0d43a169b96274f1862e770 (patch) | |
tree | ab4443461c6e42b890802f2b5e13c184c7f69406 | |
parent | 071cedc84e907f6984b3de3285ec2b077d3c3cdb (diff) |
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>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20230814084602.25699-5-benno.lossin@proton.me
[ Cleaned a few trivial nits. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
-rw-r--r-- | rust/kernel/init.rs | 1 | ||||
-rw-r--r-- | rust/kernel/init/__internal.rs | 25 | ||||
-rw-r--r-- | rust/kernel/init/macros.rs | 116 |
3 files changed, 56 insertions, 86 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 crate::{ 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 @@ impl<T> StackInit<T> { /// 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 0b0ffbc901a7..5063b45970f3 100644 --- a/rust/kernel/init/macros.rs +++ b/rust/kernel/init/macros.rs @@ -994,7 +994,6 @@ macro_rules! __pin_data { /// - `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 { @@ -1034,6 +1033,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 @@ -1048,10 +1048,6 @@ macro_rules! __init_internal { @acc(), ); } - // Forget all guards, since initialization was a success. - $crate::__init_internal!(forget_guards: - @munch_fields($($fields)*,), - ); } Ok(__InitOk) } @@ -1065,13 +1061,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)*), ) => { @@ -1082,24 +1082,28 @@ 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. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for `$field`. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot($use_data): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot($use_data): + @data($data), + @slot($slot), + @guards([<$field>], $($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)*), ) => { @@ -1109,24 +1113,28 @@ 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. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for `$field`. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot(): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot(): + @data($data), + @slot($slot), + @guards([<$field>], $($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)*), ) => { @@ -1137,18 +1145,21 @@ 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. - // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for `$field`. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot($($use_data)?): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot($($use_data)?): + @data($data), + @slot($slot), + @guards([<$field>], $($guards,)*), + @munch_fields($($rest)*), + ); + } }; (make_initializer: @slot($slot:ident), @@ -1191,29 +1202,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)] |