Re: [PATCH v2 3/3] rust: clist: Add typed iteration with FromListHead trait

[Joel Fernandes <joelagnelf@nvidia.com>]

Hi Alex,

On 11/24/2025 2:01 AM, Alexandre Courbot wrote:
>>  ///
>>  /// # Invariants
>>  ///
>> @@ -69,6 +156,15 @@ pub fn iter_heads(&self) -> ClistHeadIter<'_> {
>>              head: &self.0,
>>          }
>>      }
>> +
>> +    /// Create a high-level iterator over typed items.
>> +    #[inline]
>> +    pub fn iter<L: ClistLink>(&self) -> ClistIter<'_, L> {
>> +        ClistIter {
>> +            head_iter: self.iter_heads(),
>> +            _phantom: PhantomData,
>> +        }
>> +    }
> This looks very dangerous, as it gives any caller the freedom to specify
> the type they want to upcast the `Clist` to, without using unsafe code.
> One could easily invoke this with the wrong type and get no build error
> or warning whatsoever.
> 
> A safer version would have the `Clist` generic over the kind of
> conversion that needs to be performed, using e.g. a closure:
> 
>   pub struct Clist<'a, T, C: Fn(*mut bindings::list_head) -> *mut T> {
>       head: &'a ClistHead,
>       conv: C,
>   }
> 
> `from_raw` would also take the closure as argument, which forces the
> creator of the list to both specify what that list is for, and use an
> `unsafe` statement for unsafe code. Here is a dummy example:
> 
>     let head: bindings::list_head = ...;
> 
>     // SAFETY: list_head always corresponds to the `list` member of
>     // `type_embedding_list_head`.
>     let conv = |head: *mut bindings::list_head| unsafe {
>         crate::container_of!(head, type_embedding_list_head, list)
>     };
> 
>     // SAFETY: ...
>     unsafe { Clist::from_raw(head, conv) }
> 
> Then `conv` would be passed down to the `ClistIter` so it can return
> references to the correct type.
> 
> By doing so you can remove the `ClinkList` and `FromListHead` traits,
> the `impl_from_list_head` and `clist_iterate` macros, as well as the
> hidden ad-hoc types these create. And importantly, all unsafe code must
> be explicitly specified in an `unsafe` block, nothing is hidden by
> macros.
> 
> This approach works better imho because each `list_head` is unique in
> how it has to be iterated: there is no benefit in implementing things
> using types and traits that will only ever be used in a single place
> anyway. And if there was, we could always create a newtype for that.

I agree with your safety concerns, indeed it is possible without any safety
comments to build iterators yielding objects of random type. I think the conv
function is a good idea and with the addition of unsafe blocks within the conv.

One thing I am concerned is with the user interface. I would like to keep the
user interface as simple as possible. I am hoping that with implementing your
idea here on this with the closure, we can still keep it simple, perhaps getting
the assistance of macros. I will give it a try.

> Also as I suspected in v1 `Clist` appears to do very little apart from
> providing an iterator, so I'm more convinced that the front type for
> this should be `ClistHead`.

This part I don't agree with. I prefer to keep it as `Clist` which wraps a
sentinel list head. A random `ClistHead` is not necessarily a sentinel.

thanks,

 - Joel