rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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,
	patches@lists.linux.dev, "Asahi Lina" <lina@asahilina.net>
Subject: [PATCH 3/7] rust: init: make guards in the init macros hygienic
Date: Sat, 24 Jun 2023 09:25:10 +0000	[thread overview]
Message-ID: <20230624092330.157338-3-benno.lossin@proton.me> (raw)
In-Reply-To: <20230624092330.157338-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>
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



  parent reply	other threads:[~2023-06-24  9:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-24  9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
2023-06-24  9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
2023-06-24 14:55   ` Björn Roy Baron
2023-06-25 20:46   ` Gary Guo
2023-07-03 11:50   ` Alice Ryhl
2023-06-24  9:25 ` Benno Lossin [this message]
2023-06-24 14:58   ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Björn Roy Baron
2023-06-25 20:54   ` Gary Guo
2023-06-28 11:41     ` Benno Lossin
2023-06-28 16:48       ` Gary Guo
2023-06-24  9:25 ` [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure Benno Lossin
2023-06-24 15:03   ` Björn Roy Baron
2023-06-24 21:05     ` Benno Lossin
2023-06-24  9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
2023-06-24 15:11   ` Björn Roy Baron
2023-06-24 21:14     ` Benno Lossin
2023-06-25 12:56       ` Björn Roy Baron
2023-06-25 13:07         ` Benno Lossin
2023-06-25 14:17           ` Björn Roy Baron
2023-06-25 16:46             ` Benno Lossin
2023-07-03 11:58   ` Alice Ryhl
2023-07-03 18:15   ` Boqun Feng
2023-07-05 17:48     ` Gary Guo
2023-07-05 21:44       ` Benno Lossin
2023-06-24  9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
2023-06-24 15:17   ` Björn Roy Baron
2023-07-03 12:03   ` Alice Ryhl
2023-06-24  9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
2023-06-24 15:20   ` Björn Roy Baron
2023-06-25 21:01   ` Gary Guo
2023-06-28 11:26     ` Benno Lossin
2023-06-28 17:13       ` Gary Guo
2023-06-24 14:49 ` [PATCH 1/7] rust: init: consolidate " Björn Roy Baron

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=20230624092330.157338-3-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=patches@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@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).