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
