On Thu, Feb 19, 2026 at 12:21:56PM +0100, Danilo Krummrich wrote:
> On Wed Feb 18, 2026 at 9:55 PM CET, Joel Fernandes wrote:
> > +RUST TO C LIST INTERFACES
>
> Maybe this should just be "RUST [FFI]" instead (in case Alex and you want to
> sign up for looking after FFI helper infrastructure in general)?
Good idea, done.
> > +F: rust/kernel/ffi/clist.rs
>
> <snip>
>
> > +//! This module provides Rust abstractions for iterating over C
> > `list_head`-based
> > +//! linked lists. It is intended for FFI use-cases where a C subsystem
> > manages a
> > +//! circular linked list that Rust code needs to read. This is generally
> > required
> > +//! only for special cases and should be avoided by drivers.
>
> Maybe generalize the statement a bit and say that this should only be used for
> cases where C and Rust code share direct access to the same linked list
> through
> an FFI interface.
>
> Additionally, add a separate note that this *must not* be used by Rust
> components that just aim for a linked list primitive and instead refer to the
> Rust linked list implementation with an intra-doc link.
Done. Updated the module doc to say "It should only be used for cases
where C and Rust code share direct access to the same linked list
through an FFI interface" and added a separate note:
Note: This *must not* be used by Rust components that just need a
linked list primitive. Use [`kernel::list::List`] instead.
> > +//! types::Opaque, //
>
> Examples don't necessarily need '//' at the end, as they are not automatically
> formatted anyways.
Removed from the example. Non-example imports keep the '//' as a
rustfmt guard.
> > +//! # // SAFETY: pointers are to allocated test objects with a
> > list_head field.
> > +//! # unsafe {
>
> I understand that this is just setup code for a doc-test, but I still think we
> should hold it to the same standards, i.e. let's separate the different unsafe
> calls into their own unsafe blocks and add proper safety comments.
Done. Split into three separate unsafe blocks with individual SAFETY
comments for the value write, INIT_LIST_HEAD, and list_add_tail calls.
> > + PinInit //
>
> Should be 'PinInit, //'.
Fixed.
> > +/// - [`CListHead`] represents an allocated and valid `list_head`
> > structure.
>
> What does "allocated" mean in this context? (Dynamic allocations, stack, .data
> section of the binary, any of those?)
>
> In case of the latter, I'd just remove "allocated".
Removed "allocated". The invariant now reads:
The underlying `list_head` has been initialized (e.g. via
`INIT_LIST_HEAD()`) and its `next`/`prev` pointers are valid and
non-NULL.
> > + /// - `ptr` must be a valid pointer to an allocated and initialized
> > `list_head` structure.
>
> Same here, what exactly is meant by "allocated"?
Removed "allocated" from from_raw() safety docs as well. Updated to:
`ptr` must be a valid pointer to an initialized `list_head` (e.g.
via `INIT_LIST_HEAD()`), with valid non-NULL `next`/`prev` pointers.
> > + /// - The list and all linked `list_head` nodes must not be modified
> > by non-Rust code
> > + /// for the lifetime `'a`.
>
> This is a bit vague I think, concurrent modifications of (other) Rust code are
> not OK either.
Fixed. Changed to "must not be concurrently modified for the lifetime
`'a`" which covers both Rust and C code.
> > + // SAFETY:
> > + // - `self.as_raw()` is valid per type invariants.
> > + // - The `next` pointer is guaranteed to be non-NULL.
>
> I'm not sure whether "valid" in the type invariant implies that the struct
> list_head is initialized. From a language point of view it is also valid if
> the
> pointers are NULL.
>
> So, I think the invariant (and the safety requirements of from_raw()) have to
> ensure that the struct list_head is initialized in the sense of
> INIT_LIST_HEAD().
Agreed. The invariant and from_raw() safety requirements now explicitly
require INIT_LIST_HEAD() initialization with valid non-NULL next/prev
pointers. The next() SAFETY comment now reads:
- `self.as_raw()` is valid and initialized per type invariants.
- The `next` pointer is valid and non-NULL per type invariants
(initialized via `INIT_LIST_HEAD()` or equivalent).
> > +/// - The [`CListHead`] is an allocated and valid sentinel C `list_head`
> > structure.
> > +/// - `OFFSET` is the byte offset of the `list_head` field within the
> > struct that `T` wraps.
> > +/// - All the list's `list_head` nodes are allocated and have valid
> > next/prev pointers.
>
> Comments from CListHead also apply here.
Updated CList invariants and from_raw() safety docs to match the
CListHead pattern (removed "allocated", added INIT_LIST_HEAD, non-NULL
pointers, "concurrently modified").
> > +/// This is an unsafe macro. The caller must ensure:
>
> Given that, we should probably use the same (or a similar) trick as in [1].
>
> [1] https://rust.docs.kernel.org/src/kernel/device.rs.html#665-688
Done. Applied the device.rs pattern - the macro now requires
`clist_create!(unsafe { ... })` syntax, which forces callers to
acknowledge the safety requirements at the call site. The macro
internally wraps the `CList::from_raw` call in an unsafe block.
Thanks for the review!
Joel
