Skip to content

Commit 97de919

Browse files
y86-devojeda
authored andcommitted
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 <[email protected]> Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Gary Guo <[email protected]> Signed-off-by: Benno Lossin <[email protected]> Link: https://lore.kernel.org/r/[email protected] [ Cleaned a few trivial nits. ] Signed-off-by: Miguel Ojeda <[email protected]>
1 parent 071cedc commit 97de919

File tree

3 files changed

+56
-86
lines changed

3 files changed

+56
-86
lines changed

rust/kernel/init.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ use crate::{
206206
use alloc::boxed::Box;
207207
use core::{
208208
alloc::AllocError,
209-
cell::Cell,
210209
convert::Infallible,
211210
marker::PhantomData,
212211
mem::MaybeUninit,

rust/kernel/init/__internal.rs

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@ impl<T> StackInit<T> {
174174
/// Can be forgotten to prevent the drop.
175175
pub struct DropGuard<T: ?Sized> {
176176
ptr: *mut T,
177-
do_drop: Cell<bool>,
178177
}
179178

180179
impl<T: ?Sized> DropGuard<T> {
@@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
190189
/// - will not be dropped by any other means.
191190
#[inline]
192191
pub unsafe fn new(ptr: *mut T) -> Self {
193-
Self {
194-
ptr,
195-
do_drop: Cell::new(true),
196-
}
197-
}
198-
199-
/// Prevents this guard from dropping the supplied pointer.
200-
///
201-
/// # Safety
202-
///
203-
/// This function is unsafe in order to prevent safe code from forgetting this guard. It should
204-
/// only be called by the macros in this module.
205-
#[inline]
206-
pub unsafe fn forget(&self) {
207-
self.do_drop.set(false);
192+
Self { ptr }
208193
}
209194
}
210195

211196
impl<T: ?Sized> Drop for DropGuard<T> {
212197
#[inline]
213198
fn drop(&mut self) {
214-
if self.do_drop.get() {
215-
// SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
216-
// ensuring that this operation is safe.
217-
unsafe { ptr::drop_in_place(self.ptr) }
218-
}
199+
// SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
200+
// ensuring that this operation is safe.
201+
unsafe { ptr::drop_in_place(self.ptr) }
219202
}
220203
}
221204

rust/kernel/init/macros.rs

Lines changed: 52 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,6 @@ macro_rules! __pin_data {
994994
/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
995995
/// - `make_initializer`: recursively create the struct initializer that guarantees that every
996996
/// field has been initialized exactly once.
997-
/// - `forget_guards`: recursively forget the drop guards for every field.
998997
#[doc(hidden)]
999998
#[macro_export]
1000999
macro_rules! __init_internal {
@@ -1034,6 +1033,7 @@ macro_rules! __init_internal {
10341033
$crate::__init_internal!(init_slot($($use_data)?):
10351034
@data(data),
10361035
@slot(slot),
1036+
@guards(),
10371037
@munch_fields($($fields)*,),
10381038
);
10391039
// We use unreachable code to ensure that all fields have been mentioned exactly
@@ -1048,10 +1048,6 @@ macro_rules! __init_internal {
10481048
@acc(),
10491049
);
10501050
}
1051-
// Forget all guards, since initialization was a success.
1052-
$crate::__init_internal!(forget_guards:
1053-
@munch_fields($($fields)*,),
1054-
);
10551051
}
10561052
Ok(__InitOk)
10571053
}
@@ -1065,13 +1061,17 @@ macro_rules! __init_internal {
10651061
(init_slot($($use_data:ident)?):
10661062
@data($data:ident),
10671063
@slot($slot:ident),
1064+
@guards($($guards:ident,)*),
10681065
@munch_fields($(,)?),
10691066
) => {
1070-
// Endpoint of munching, no fields are left.
1067+
// Endpoint of munching, no fields are left. If execution reaches this point, all fields
1068+
// have been initialized. Therefore we can now dismiss the guards by forgetting them.
1069+
$(::core::mem::forget($guards);)*
10711070
};
10721071
(init_slot($use_data:ident): // `use_data` is present, so we use the `data` to init fields.
10731072
@data($data:ident),
10741073
@slot($slot:ident),
1074+
@guards($($guards:ident,)*),
10751075
// In-place initialization syntax.
10761076
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
10771077
) => {
@@ -1082,24 +1082,28 @@ macro_rules! __init_internal {
10821082
// return when an error/panic occurs.
10831083
// We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
10841084
unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
1085-
// Create the drop guard.
1086-
//
1087-
// We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
1085+
// Create the drop guard:
10881086
//
1089-
// SAFETY: We forget the guard later when initialization has succeeded.
1090-
let $field = &unsafe {
1091-
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1092-
};
1087+
// We rely on macro hygiene to make it impossible for users to access this local variable.
1088+
// We use `paste!` to create new hygiene for `$field`.
1089+
::kernel::macros::paste! {
1090+
// SAFETY: We forget the guard later when initialization has succeeded.
1091+
let [<$field>] = unsafe {
1092+
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1093+
};
10931094

1094-
$crate::__init_internal!(init_slot($use_data):
1095-
@data($data),
1096-
@slot($slot),
1097-
@munch_fields($($rest)*),
1098-
);
1095+
$crate::__init_internal!(init_slot($use_data):
1096+
@data($data),
1097+
@slot($slot),
1098+
@guards([<$field>], $($guards,)*),
1099+
@munch_fields($($rest)*),
1100+
);
1101+
}
10991102
};
11001103
(init_slot(): // No `use_data`, so we use `Init::__init` directly.
11011104
@data($data:ident),
11021105
@slot($slot:ident),
1106+
@guards($($guards:ident,)*),
11031107
// In-place initialization syntax.
11041108
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
11051109
) => {
@@ -1109,24 +1113,28 @@ macro_rules! __init_internal {
11091113
// SAFETY: `slot` is valid, because we are inside of an initializer closure, we
11101114
// return when an error/panic occurs.
11111115
unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
1112-
// Create the drop guard.
1113-
//
1114-
// We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
1116+
// Create the drop guard:
11151117
//
1116-
// SAFETY: We forget the guard later when initialization has succeeded.
1117-
let $field = &unsafe {
1118-
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1119-
};
1118+
// We rely on macro hygiene to make it impossible for users to access this local variable.
1119+
// We use `paste!` to create new hygiene for `$field`.
1120+
::kernel::macros::paste! {
1121+
// SAFETY: We forget the guard later when initialization has succeeded.
1122+
let [<$field>] = unsafe {
1123+
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1124+
};
11201125

1121-
$crate::__init_internal!(init_slot():
1122-
@data($data),
1123-
@slot($slot),
1124-
@munch_fields($($rest)*),
1125-
);
1126+
$crate::__init_internal!(init_slot():
1127+
@data($data),
1128+
@slot($slot),
1129+
@guards([<$field>], $($guards,)*),
1130+
@munch_fields($($rest)*),
1131+
);
1132+
}
11261133
};
11271134
(init_slot($($use_data:ident)?):
11281135
@data($data:ident),
11291136
@slot($slot:ident),
1137+
@guards($($guards:ident,)*),
11301138
// Init by-value.
11311139
@munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
11321140
) => {
@@ -1137,18 +1145,21 @@ macro_rules! __init_internal {
11371145
unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
11381146
// Create the drop guard:
11391147
//
1140-
// We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
1141-
//
1142-
// SAFETY: We forget the guard later when initialization has succeeded.
1143-
let $field = &unsafe {
1144-
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1145-
};
1148+
// We rely on macro hygiene to make it impossible for users to access this local variable.
1149+
// We use `paste!` to create new hygiene for `$field`.
1150+
::kernel::macros::paste! {
1151+
// SAFETY: We forget the guard later when initialization has succeeded.
1152+
let [<$field>] = unsafe {
1153+
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
1154+
};
11461155

1147-
$crate::__init_internal!(init_slot($($use_data)?):
1148-
@data($data),
1149-
@slot($slot),
1150-
@munch_fields($($rest)*),
1151-
);
1156+
$crate::__init_internal!(init_slot($($use_data)?):
1157+
@data($data),
1158+
@slot($slot),
1159+
@guards([<$field>], $($guards,)*),
1160+
@munch_fields($($rest)*),
1161+
);
1162+
}
11521163
};
11531164
(make_initializer:
11541165
@slot($slot:ident),
@@ -1191,29 +1202,6 @@ macro_rules! __init_internal {
11911202
@acc($($acc)* $field: ::core::panic!(),),
11921203
);
11931204
};
1194-
(forget_guards:
1195-
@munch_fields($(,)?),
1196-
) => {
1197-
// Munching finished.
1198-
};
1199-
(forget_guards:
1200-
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
1201-
) => {
1202-
unsafe { $crate::init::__internal::DropGuard::forget($field) };
1203-
1204-
$crate::__init_internal!(forget_guards:
1205-
@munch_fields($($rest)*),
1206-
);
1207-
};
1208-
(forget_guards:
1209-
@munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
1210-
) => {
1211-
unsafe { $crate::init::__internal::DropGuard::forget($field) };
1212-
1213-
$crate::__init_internal!(forget_guards:
1214-
@munch_fields($($rest)*),
1215-
);
1216-
};
12171205
}
12181206

12191207
#[doc(hidden)]

0 commit comments

Comments
 (0)