Messages in this thread | | | Date | Sat, 24 Jun 2023 15:11:35 +0000 | From | Björn Roy Baron <> | Subject | Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields |
| |
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Add the struct update syntax to the init macros, but only for > `..Zeroable::zeroed()`. Adding this at the end of the struct initializer > allows one to omit fields from the initializer, these fields will be > initialized with 0x00 set to every byte. Only types that implement the > `Zeroable` trait can utilize this. > > Suggested-by: Asahi Lina <lina@asahilina.net> > Signed-off-by: Benno Lossin <benno.lossin@proton.me> > --- > rust/kernel/init.rs | 16 +++++- > rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 128 insertions(+), 2 deletions(-) > > diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs > index ecf6a4bd0ce4..44bc3e77419a 100644 > --- a/rust/kernel/init.rs > +++ b/rust/kernel/init.rs > @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init { > /// - Fields that you want to initialize in-place have to use `<-` instead of `:`. > /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`] > /// pointer named `this` inside of the initializer. > +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the > +/// struct, this initializes every field with 0 and then runs all initializers specified in the > +/// body. This can only be done if [`Zeroable`] is implemented for the struct. > /// > /// For instance: > /// > /// ```rust > /// # use kernel::pin_init; > -/// # use macros::pin_data; > +/// # use macros::{pin_data, Zeroable}; > /// # use core::{ptr::addr_of_mut, marker::PhantomPinned}; > /// #[pin_data] > +/// #[derive(Zeroable)] > /// struct Buf { > /// // `ptr` points into `buf`. > /// ptr: *mut u8, > @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init { > /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() }, > /// pin: PhantomPinned, > /// }); > +/// pin_init!(Buf { > +/// buf: [1; 64], > +/// ..Zeroable::zeroed(), > +/// }); > /// ``` > /// > /// [`try_pin_init!`]: kernel::try_pin_init > @@ -547,6 +555,7 @@ macro_rules! pin_init { > @data(PinData, use_data), > @has_data(HasPinData, __pin_data), > @construct_closure(pin_init_from_closure), > + @munch_fields($($fields)*), > ) > }; > } > @@ -603,6 +612,7 @@ macro_rules! try_pin_init { > @data(PinData, use_data), > @has_data(HasPinData, __pin_data), > @construct_closure(pin_init_from_closure), > + @munch_fields($($fields)*), > ) > }; > ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? { > @@ -616,6 +626,7 @@ macro_rules! try_pin_init { > @data(PinData, use_data), > @has_data(HasPinData, __pin_data), > @construct_closure(pin_init_from_closure), > + @munch_fields($($fields)*), > ) > }; > } > @@ -650,6 +661,7 @@ macro_rules! init { > @data(InitData, /*no use_data*/), > @has_data(HasInitData, __init_data), > @construct_closure(init_from_closure), > + @munch_fields($($fields)*), > ) > } > } > @@ -700,6 +712,7 @@ macro_rules! try_init { > @data(InitData, /*no use_data*/), > @has_data(HasInitData, __init_data), > @construct_closure(init_from_closure), > + @munch_fields($($fields)*), > ) > }; > ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? { > @@ -713,6 +726,7 @@ macro_rules! try_init { > @data(InitData, /*no use_data*/), > @has_data(HasInitData, __init_data), > @construct_closure(init_from_closure), > + @munch_fields($($fields)*), > ) > }; > } > diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs > index 1e0c4aca055a..5dcb2e513f26 100644 > --- a/rust/kernel/init/macros.rs > +++ b/rust/kernel/init/macros.rs > @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*> > /// > /// This macro has multiple internal call configurations, these are always the very first ident: > /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros. > +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled. > /// - `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. > @@ -1007,6 +1008,82 @@ macro_rules! __init_internal { > @has_data($has_data:ident, $get_data:ident), > // `pin_init_from_closure` or `init_from_closure`. > @construct_closure($construct_closure:ident), > + @munch_fields(), > + ) => { > + $crate::__init_internal!(with_update_parsed: > + @this($($this)?), > + @typ($t $(::<$($generics),*>)? ), > + @fields($($fields)*), > + @error($err), > + @data($data, $($use_data)?), > + @has_data($has_data, $get_data), > + @construct_closure($construct_closure), > + @zeroed(), // nothing means default behavior. > + ) > + }; > + ( > + @this($($this:ident)?), > + @typ($t:ident $(::<$($generics:ty),*>)?), > + @fields($($fields:tt)*), > + @error($err:ty), > + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData` > + // case. > + @data($data:ident, $($use_data:ident)?), > + // `HasPinData` or `HasInitData`. > + @has_data($has_data:ident, $get_data:ident), > + // `pin_init_from_closure` or `init_from_closure`. > + @construct_closure($construct_closure:ident), > + @munch_fields(..Zeroable::zeroed()), > + ) => { > + $crate::__init_internal!(with_update_parsed: > + @this($($this)?), > + @typ($t $(::<$($generics),*>)? ), > + @fields($($fields)*), > + @error($err), > + @data($data, $($use_data)?), > + @has_data($has_data, $get_data), > + @construct_closure($construct_closure), > + @zeroed(()), // `()` means zero all fields not mentioned. > + ) > + }; > + ( > + @this($($this:ident)?), > + @typ($t:ident $(::<$($generics:ty),*>)?), > + @fields($($fields:tt)*), > + @error($err:ty), > + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData` > + // case. > + @data($data:ident, $($use_data:ident)?), > + // `HasPinData` or `HasInitData`. > + @has_data($has_data:ident, $get_data:ident), > + // `pin_init_from_closure` or `init_from_closure`. > + @construct_closure($construct_closure:ident), > + @munch_fields($ignore:tt $($rest:tt)*), > + ) => { > + $crate::__init_internal!( > + @this($($this)?), > + @typ($t $(::<$($generics),*>)? ), > + @fields($($fields)*), > + @error($err), > + @data($data, $($use_data)?), > + @has_data($has_data, $get_data), > + @construct_closure($construct_closure), > + @munch_fields($($rest)*), > + ) > + }; > + (with_update_parsed: > + @this($($this:ident)?), > + @typ($t:ident $(::<$($generics:ty),*>)?), > + @fields($($fields:tt)*), > + @error($err:ty), > + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData` > + // case. > + @data($data:ident, $($use_data:ident)?), > + // `HasPinData` or `HasInitData`. > + @has_data($has_data:ident, $get_data:ident), > + // `pin_init_from_closure` or `init_from_closure`. > + @construct_closure($construct_closure:ident), > + @zeroed($($init_zeroed:expr)?), > ) => {{ > // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return > // type and shadow it later when we insert the arbitrary user code. That way there will be > @@ -1024,6 +1101,17 @@ macro_rules! __init_internal { > { > // Shadow the structure so it cannot be used to return early. > struct __InitOk; > + // If `$init_zeroed` is present we should zero the slot now and not emit an > + // error when fields are missing (since they will be zeroed). We also have to > + // check that the type actually implements `Zeroable`. > + $( > + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
Maybe call this assert_zeroable?
> + // Ensure that the struct is indeed `Zeroable`. > + is_zeroable(slot); > + // SAFETY: The type implements `Zeroable` by the check above. > + unsafe { ::core::ptr::write_bytes(slot, 0, 1) }; > + $init_zeroed // this will be `()` if set.
How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
> + )? > // Create the `this` so it can be referenced by the user inside of the > // expressions creating the individual fields. > $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)? > @@ -1064,7 +1152,7 @@ macro_rules! __init_internal { > @data($data:ident), > @slot($slot:ident), > @guards($($guards:ident,)*), > - @munch_fields($(,)?), > + @munch_fields($(..Zeroable::zeroed())? $(,)?), > ) => { > // 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. > @@ -1157,6 +1245,30 @@ macro_rules! __init_internal { > @munch_fields($($rest)*), > ); > }; > + (make_initializer: > + @slot($slot:ident), > + @type_name($t:ident), > + @munch_fields(..Zeroable::zeroed() $(,)?), > + @acc($($acc:tt)*), > + ) => { > + // Endpoint, nothing more to munch, create the initializer. Since the users specified > + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have > + // not been overwritten are thus zero and initialized. We still check that all fields are > + // actually accessible by using the struct update syntax ourselves. > + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to > + // get the correct type inference here: > + unsafe { > + let mut zeroed = ::core::mem::zeroed(); > + // We have to use type inference her to make zeroed have the correct type. This does
*here
> + // not get executed, so it has no effect. > + ::core::ptr::write($slot, zeroed); > + zeroed = ::core::mem::zeroed(); > + ::core::ptr::write($slot, $t { > + $($acc)* > + ..zeroed > + }); > + } > + }; > (make_initializer: > @slot($slot:ident), > @type_name($t:ident), > -- > 2.41.0
Cheers, Björn
| |