[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DDVYV1VT441A.11L5C11F8R7C9@kernel.org>
Date: Thu, 30 Oct 2025 22:15:19 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Joel Fernandes" <joelagnelf@...dia.com>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
 <dri-devel@...ts.freedesktop.org>, "David Airlie" <airlied@...il.com>,
 <acourbot@...dia.com>, "Alistair Popple" <apopple@...dia.com>, "Miguel
 Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Simona Vetter" <simona@...ll.ch>,
 "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard"
 <mripard@...nel.org>, "Thomas Zimmermann" <tzimmermann@...e.de>, "John
 Hubbard" <jhubbard@...dia.com>, "Timur Tabi" <ttabi@...dia.com>,
 <joel@...lfernandes.org>, "Elle Rhumsaa" <elle@...thered-steel.dev>,
 "Daniel Almeida" <daniel.almeida@...labora.com>, "Andrea Righi"
 <arighi@...dia.com>, "Philipp Stanner" <phasta@...nel.org>,
 <nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over
 C linked lists
On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
> Provides a safe interface for iterating over C's intrusive
I'm not sure we're there just yet, I count eight unsafe blocks in the subsequent
sample code.
Don't get me wrong, there is no way to make the API safe entirely, but "safe
interface" is clearly a wrong promise. :)
Some more high level comments below.
> +//! // Rust-side struct to hold pointer to C-side struct.
> +//! struct Item {
> +//!     ptr: NonNull<bindings::c_item>,
> +//! }
> +//!
> +//! impl clist::FromListHead for Item {
> +//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> +//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
> +//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
> +//!     }
> +//! }
If you just embedd a pointer to the C struct in a struct Item you don't cover
the lifetime relationship.
Instead this should be something like
	#[repr(transparent)]
	struct Entry<T>(Opaque<T>);
or
	struct Entry<'a, T>(NonNull<T>, PhantomData<&'a T>);
where T is the C list entry type.
You can then have a setup where an &Entry borrows from a &CListHead, which
borrows from a Clist.
I'd also provide a macro for users to generate this structure as well as the
corresponding FromListHead impl.
> +//! // Rust wraps and iterates over the list.
> +//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };
This function has a lot of safety requirements that need to be covered.
It also should be, besides the unsafe FromListHead trait, the only unsafe
function needed.
The Clist should ideally have methods for all kinds of list iterators, e.g.
list_for_each_entry_{safe,reverse,continue}() etc.
Of course you don't need to provide all of them in an initial implementation,
but we should set the direction.
Powered by blists - more mailing lists
 
