[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aAcg86QfvlA0zAh4@Mac.home>
Date: Mon, 21 Apr 2025 21:54:11 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Fiona Behrens <me@...enk.dev>,
Christian Schrefl <chrisi.schrefl@...il.com>,
Alban Kurti <kurti@...icto.ai>, Michael Vetter <jubalh@...oru.org>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro
On Mon, Apr 21, 2025 at 10:18:52PM +0000, Benno Lossin wrote:
> This derive macro implements `Zeroable` for structs & unions precisely
> if all fields also implement `Zeroable` and does nothing otherwise. The
> plain `Zeroable` derive macro instead errors when it cannot derive
> `Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases
> where manual checking is infeasible such as with the bindings crate.
>
Hmm... seems we need a customized auto trait? How hard would that be?
Regards,
Boqun
> Move the zeroable generics parsing into a standalone function in order
> to avoid code duplication between the two derive macros.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/1165cdad1a391b923efaf30cf76bc61e38da022e
> Signed-off-by: Benno Lossin <benno.lossin@...ton.me>
> ---
> rust/pin-init/internal/src/lib.rs | 5 +++
> rust/pin-init/internal/src/zeroable.rs | 27 +++++++++++-
> rust/pin-init/src/lib.rs | 30 +++++++++++++
> rust/pin-init/src/macros.rs | 59 ++++++++++++++++++++++++++
> 4 files changed, 120 insertions(+), 1 deletion(-)
>
> diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs
> index 56aa9ecc1e1a..297b0129a5bf 100644
> --- a/rust/pin-init/internal/src/lib.rs
> +++ b/rust/pin-init/internal/src/lib.rs
> @@ -47,3 +47,8 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> zeroable::derive(input.into()).into()
> }
> +
> +#[proc_macro_derive(MaybeZeroable)]
> +pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::maybe_derive(input.into()).into()
> +}
> diff --git a/rust/pin-init/internal/src/zeroable.rs b/rust/pin-init/internal/src/zeroable.rs
> index acc94008c152..e0ed3998445c 100644
> --- a/rust/pin-init/internal/src/zeroable.rs
> +++ b/rust/pin-init/internal/src/zeroable.rs
> @@ -6,7 +6,14 @@
> use crate::helpers::{parse_generics, Generics};
> use proc_macro::{TokenStream, TokenTree};
>
> -pub(crate) fn derive(input: TokenStream) -> TokenStream {
> +pub(crate) fn parse_zeroable_derive_input(
> + input: TokenStream,
> +) -> (
> + Vec<TokenTree>,
> + Vec<TokenTree>,
> + Vec<TokenTree>,
> + Option<TokenTree>,
> +) {
> let (
> Generics {
> impl_generics,
> @@ -64,6 +71,11 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
> if in_generic && !inserted {
> new_impl_generics.extend(quote! { : ::pin_init::Zeroable });
> }
> + (rest, new_impl_generics, ty_generics, last)
> +}
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
> quote! {
> ::pin_init::__derive_zeroable!(
> parse_input:
> @@ -74,3 +86,16 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
> );
> }
> }
> +
> +pub(crate) fn maybe_derive(input: TokenStream) -> TokenStream {
> + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
> + quote! {
> + ::pin_init::__maybe_derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#new_impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index 774f8ca033bc..05a0cd6ad8f4 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -413,6 +413,36 @@
> /// ```
> pub use ::pin_init_internal::Zeroable;
>
> +/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`].
> +///
> +/// Contrary to the derive macro named [`macro@...oable`], this one silently fails when a field
> +/// doesn't implement [`Zeroable`].
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use pin_init::MaybeZeroable;
> +///
> +/// // implmements `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +///
> +/// // does not implmement `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData2 {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// // this field doesn't implement `Zeroable`
> +/// other_data: &'static i32,
> +/// }
> +/// ```
> +pub use ::pin_init_internal::MaybeZeroable;
> +
> /// Initialize and pin a type directly on the stack.
> ///
> /// # Examples
> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
> index 332d7e08925b..935d77745d1d 100644
> --- a/rust/pin-init/src/macros.rs
> +++ b/rust/pin-init/src/macros.rs
> @@ -1443,3 +1443,62 @@ fn ensure_zeroable<$($impl_generics)*>()
> };
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __maybe_derive_zeroable {
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis struct $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field_vis:vis $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $(
> + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> + // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> + $field_ty: for<'__dummy> $crate::Zeroable,
> + )*
> + $($($whr)*)?
> + {}
> + };
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis union $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field_vis:vis $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $(
> + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> + // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> + $field_ty: for<'__dummy> $crate::Zeroable,
> + )*
> + $($($whr)*)?
> + {}
> + };
> +}
> --
> 2.48.1
>
>
Powered by blists - more mailing lists