Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes <joelagnelf@nvidia.com>]

Hi Alex,

On 12/3/2025 8:06 AM, Alexandre Courbot wrote:
[..]
>> +}
>> +
>> +impl<'a> Iterator for ClistHeadIter<'a> {
>> +    type Item = &'a ClistHead;
>> +
>> +    #[inline]
>> +    fn next(&mut self) -> Option<Self::Item> {
>> +        if self.exhausted {
>> +            return None;
>> +        }
>> +
>> +        // Advance to next node.
>> +        self.current_head = self.current_head.next();
>> +
>> +        // Check if we've circled back to the sentinel head.
>> +        if self.current_head == self.list_head {
>> +            self.exhausted = true;
>> +            return None;
>> +        }
>> +
>> +        Some(self.current_head)
>> +    }
> 
> IIUC you could just rewrite this as
> 
>     let next = self.current_head.next();
>     if next == self.list_head {
>         None
>     } else {
>         self.current_head = next;
>         Some(self.current_head)
>     }
> 
> and drop `exhausted` altogether.
> 

Yes, this is better, changed, thanks!
>> +    _phantom: PhantomData<&'a T>,
>> +}
>> +
>> +impl<'a, T> Clist<'a, T> {
>> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
>> +    ///
>> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
>> +    /// the C struct that `T` wraps.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>> +    ///   representing a list sentinel.
>> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
>> +    #[inline]
>> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
>> +        Self {
>> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>> +            head: unsafe { ClistHead::from_raw(ptr) },
>> +            offset: OFFSET,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +
>> +    /// Get the raw sentinel `list_head` pointer.
>> +    #[inline]
>> +    pub fn as_raw(&self) -> *mut bindings::list_head {
>> +        self.head.as_raw()
>> +    }
>> +
>> +    /// Check if the list is empty.
>> +    #[inline]
>> +    pub fn is_empty(&self) -> bool {
>> +        let raw = self.as_raw();
>> +        // SAFETY: self.as_raw() is valid per type invariants.
>> +        unsafe { (*raw).next == raw }
>> +    }
>> +
>> +    /// Create an iterator over typed items.
>> +    #[inline]
>> +    pub fn iter(&self) -> ClistIter<'a, T> {
>> +        ClistIter {
>> +            head_iter: ClistHeadIter {
>> +                current_head: self.head,
>> +                list_head: self.head,
>> +                exhausted: false,
>> +            },
>> +            offset: self.offset,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +}
>> +
>> +/// High-level iterator over typed list items.
>> +pub struct ClistIter<'a, T> {
>> +    head_iter: ClistHeadIter<'a>,
>> +    offset: usize,
> 
> Now that Clist's offset has moved to a const generic, let's do the same
> here as well.

Yes, done.

> Overall I think this version looks pretty clean! A nice,
> easy to understand wrapper over the C API.

Indeed, it has come out to be very nice. Simple design and clean code. Thanks
for all the reviews! I will shoot for posting this patch with the DRM buddy
series together today. There is also a pin initialization patch for `Clist` that
I will separately add to the series on top of this patch.

thanks,

 - Joel