From: Benno Lossin <benno.lossin@proton.me>
To: Miguel Ojeda <ojeda@kernel.org>,
Wedson Almeida Filho <wedsonaf@gmail.com>,
Alex Gaynor <alex.gaynor@gmail.com>
Cc: "Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Alice Ryhl" <aliceryhl@google.com>,
"Andreas Hindborg" <nmi@metaspace.dk>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
"Asahi Lina" <lina@asahilina.net>,
"Martin Rodriguez Reboredo" <yakoyoku@gmail.com>
Subject: [PATCH v3 04/13] rust: init: make guards in the init macros hygienic
Date: Sat, 29 Jul 2023 09:09:40 +0000 [thread overview]
Message-ID: <20230729090838.225225-5-benno.lossin@proton.me> (raw)
In-Reply-To: <20230729090838.225225-1-benno.lossin@proton.me>
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>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
v2 -> v3:
- added Reviewed-by's from Martin and Alice.
v1 -> v2:
- use Gary's `paste!` macro to create the guard hygiene.
rust/kernel/init.rs | 1 -
rust/kernel/init/__internal.rs | 25 ++-----
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 d431d0b153a2..0120674b451e 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -216,7 +216,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 78091756dec0..454f31b8c614 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -994,7 +994,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 {
@@ -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)]
--
2.41.0
next prev parent reply other threads:[~2023-07-29 9:10 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-29 9:09 [PATCH v3 00/13] Quality of life improvements for pin-init Benno Lossin
2023-07-29 9:09 ` [PATCH v3 01/13] rust: init: consolidate init macros Benno Lossin
2023-07-29 9:09 ` [PATCH v3 02/13] rust: init: make `#[pin_data]` compatible with conditional compilation of fields Benno Lossin
2023-08-01 10:50 ` Alice Ryhl
2023-08-02 17:47 ` Gary Guo
2023-08-05 17:04 ` Martin Rodriguez Reboredo
2023-07-29 9:09 ` [PATCH v3 03/13] rust: add derive macro for `Zeroable` Benno Lossin
2023-07-31 2:51 ` Boqun Feng
2023-07-29 9:09 ` Benno Lossin [this message]
2023-08-02 17:52 ` [PATCH v3 04/13] rust: init: make guards in the init macros hygienic Gary Guo
2023-07-29 9:09 ` [PATCH v3 05/13] rust: init: wrap type checking struct initializers in a closure Benno Lossin
2023-08-02 17:52 ` Gary Guo
2023-07-29 9:09 ` [PATCH v3 06/13] rust: init: make initializer values inaccessible after initializing Benno Lossin
2023-08-02 17:59 ` Gary Guo
2023-07-29 9:09 ` [PATCH v3 07/13] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
2023-08-02 18:05 ` Gary Guo
2023-07-29 9:10 ` [PATCH v3 08/13] rust: init: Add functions to create array initializers Benno Lossin
2023-07-31 3:00 ` Boqun Feng
2023-08-05 17:11 ` Martin Rodriguez Reboredo
2023-08-06 16:07 ` Gary Guo
2023-07-29 9:10 ` [PATCH v3 09/13] rust: init: add support for arbitrary paths in init macros Benno Lossin
2023-08-06 16:07 ` Gary Guo
2023-07-29 9:10 ` [PATCH v3 10/13] rust: init: implement `Zeroable` for `UnsafeCell<T>` and `Opaque<T>` Benno Lossin
2023-08-05 17:12 ` Martin Rodriguez Reboredo
2023-08-06 16:08 ` Gary Guo
2023-07-29 9:10 ` [PATCH v3 11/13] rust: init: make `PinInit<T, E>` a supertrait of `Init<T, E>` Benno Lossin
2023-08-06 16:09 ` Gary Guo
2023-07-29 9:10 ` [PATCH v3 12/13] rust: init: add `{pin_}chain` functions to `{Pin}Init<T, E>` Benno Lossin
2023-08-05 17:15 ` Martin Rodriguez Reboredo
2023-07-29 9:10 ` [PATCH v3 13/13] rust: init: update expanded macro explanation Benno Lossin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230729090838.225225-5-benno.lossin@proton.me \
--to=benno.lossin@proton.me \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=gary@garyguo.net \
--cc=lina@asahilina.net \
--cc=linux-kernel@vger.kernel.org \
--cc=nmi@metaspace.dk \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=wedsonaf@gmail.com \
--cc=yakoyoku@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).