Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Feedback-ID: iad51458e:Fastmail
Date:   Fri, 3 Nov 2023 10:34:27 -0700
From:   Boqun Feng <boqun.feng@gmail.com>
To:     Benno Lossin <benno.lossin@proton.me>
Cc:     Alice Ryhl <aliceryhl@google.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Arve =?iso-8859-1?B?SGr4bm5lduVn?= <arve@android.com>,
        Todd Kjos <tkjos@android.com>,
        Martijn Coenen <maco@android.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Christian Brauner <brauner@kernel.org>,
        Carlos Llamas <cmllamas@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
        Gary Guo <gary@garyguo.net>,
        =?iso-8859-1?Q?Bj=F6rn?= Roy Baron <bjorn3_gh@protonmail.com>,
        Andreas Hindborg <a.hindborg@samsung.com>,
        Matt Gilbride <mattgilbride@google.com>,
        Jeffrey Vander Stoep <jeffv@google.com>,
        Matthew Maurer <mmaurer@google.com>
Subject: Re: [PATCH RFC 02/20] rust_binder: add binderfs support to Rust
 binder
Message-ID: <ZUUvI0leuPfhCOfE@boqun-archlinux>
References: <20231101-rust-binder-v1-0-08ba9197f637@google.com>
 <20231101-rust-binder-v1-2-08ba9197f637@google.com>
 <sRVdoCqLbxM1-EH0iKVlb9eOEU-wt410-WT5rFTQNNYgmiW6EEpKvCCJyVppOmFYhXBcCN3SsXUXULzpmmweYBGDVHW619pjsIZvorv8Fc8=@proton.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <sRVdoCqLbxM1-EH0iKVlb9eOEU-wt410-WT5rFTQNNYgmiW6EEpKvCCJyVppOmFYhXBcCN3SsXUXULzpmmweYBGDVHW619pjsIZvorv8Fc8=@proton.me>
Precedence: bulk

On Fri, Nov 03, 2023 at 04:30:54PM +0000, Benno Lossin wrote:
> On 01.11.23 19:01, Alice Ryhl wrote:
> > +/// There is one context per binder file (/dev/binder, /dev/hwbinder, etc)
> > +#[pin_data]
> > +pub(crate) struct Context {
> > +    #[pin]
> > +    manager: Mutex<Manager>,
> > +    pub(crate) name: CString,
> > +    #[pin]
> > +    links: ListLinks,
> > +}
> > +
> > +kernel::list::impl_has_list_links! {
> > +    impl HasListLinks<0> for Context { self.links }
> > +}
> > +kernel::list::impl_list_arc_safe! {
> > +    impl ListArcSafe<0> for Context { untracked; }
> > +}
> > +kernel::list::impl_list_item! {
> > +    impl ListItem<0> for Context {
> > +        using ListLinks;
> > +    }
> > +}
> 
> I think at some point it would be worth introducing a derive macro that
> does this for us. So for example:

Agreed.

> 
>     #[pin_data]
>     #[derive(HasListLinks)]
>     pub(crate) struct Context {
>         #[pin]
>         manager: Mutex<Manager>,
>         pub(crate) name: CString,
>         #[pin]
>         #[links]
>         links: ListLinks,
>     }
> 
> And if you need multiple links you could do:
> 
>     #[pin_data]
>     #[derive(HasListLinks)]
>     struct Foo {
>         #[links = 0]
>         a_list: ListLinks,

we will need more discussion on how the derive syntax would look like,
but I'd expect we can reference the field with names instead of numbers
if we use derive macros. In other words type numbering to distinguish
different fields should be an implementation detail.

Regards,
Boqun

>         #[links = 1]
>         b_list: ListLinks,
>     }
> 
> Same for `ListItem` and `HasWork`. I have not yet taken a look at your
> linked list implementation, so I don't know if this is possible (since
> `ListItem` seems to have multiple "backends").
> 
> I think this improvement can wait though, just wanted to mention it.
> 
> -- 
> Cheers,
> Benno
>