Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

["Danilo Krummrich" <dakr@kernel.org>]

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.