Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: iad51458e:Fastmail
Date:   Wed, 22 Mar 2023 23:30:35 -0700
From:   Boqun Feng <boqun.feng@gmail.com>
To:     Benno Lossin <y86-dev@protonmail.com>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Gary Guo <gary@garyguo.net>,
        =?iso-8859-1?Q?Bj=F6rn?= Roy Baron <bjorn3_gh@protonmail.com>,
        rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
        patches@lists.linux.dev
Subject: Re: [PATCH v2 3/5] rust: add pin-init API
Message-ID: <ZBvyCwPcpTnk2R7h@Boquns-Mac-mini.local>
References: <20230321194934.908891-1-y86-dev@protonmail.com>
 <20230321194934.908891-4-y86-dev@protonmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230321194934.908891-4-y86-dev@protonmail.com>
Precedence: bulk

On Tue, Mar 21, 2023 at 07:50:00PM +0000, Benno Lossin wrote:
[...]
> +/// # Syntax
> +///
> +/// As already mentioned in the examples above, inside of `pin_init!` a `struct` initializer with
> +/// the following modifications is expected:
> +/// - 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.
> +///
> +/// For instance:
> +///
> +/// ```rust
> +/// # use kernel::pin_init;
> +/// # use macros::pin_data;
> +/// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
> +/// #[pin_data]
> +/// struct Buf {
> +///     ptr: *mut u8,
> +///     buf: [u8; 64],

Say we have an extra field,

           a: u8,

> +///     #[pin]
> +///     pin: PhantomPinned,
> +/// }
> +/// pin_init!(&this in Buf {
> +///     buf: [0; 64],
> +///     ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },

And I think we want to disallow:

           a: unsafe { (*addr_of!(*this.as_ptr().buf))[0] }

, right? Because we don't want `pin_init!` to provide any initialization
order guarantee? If so, maybe add one or two sentences to call it out.

If not sure, I think we can leave it as it is, until someone really uses
this pattern ;-)

Regards,
Boqun

> +///     pin: PhantomPinned,
> +/// });
> +/// ```
> +///
> +/// [`try_pin_init!`]: kernel::try_pin_init
> +/// [`NonNull<Self>`]: core::ptr::NonNull
> +#[macro_export]
> +macro_rules! pin_init {
> +    ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> +        $($fields:tt)*
> +    }) => {
> +        $crate::try_pin_init!(
> +            @this($($this)?),
> +            @type_name($t),
> +            @typ($t $(<$($generics),*>)?),
> +            @fields($($fields)*),
> +            @error(::core::convert::Infallible),
> +        )
> +    };
> +}
> +
[...]