On Sun Nov 30, 2025 at 6:30 AM JST, Joel Fernandes wrote:
<snip>
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list
> (`list_head`). Caller has to
> +/// perform conversion of returned `ClistHead` to an item (using
> `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// `ClistHeadIter` is iterating over an allocated, initialized and valid
> list.
> +struct ClistHeadIter<'a> {
> + current_head: &'a ClistHead,
> + list_head: &'a ClistHead,
> + exhausted: bool,
Mmm, I suspect `exhausted` is not needed at all - please see below.
> +}
> +
> +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.
> +}
> +
> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
Is there a reason for not implementing `DoubleEndedIterator` as well?
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific
> item.
> +///
> +/// # Invariants
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the
> list's sentinel.
> +/// - `offset` is the byte offset of the `list_head` field within the C
> struct that `T` wraps.
> +///
> +/// # Safety
> +///
> +/// - All the list's `list_head` nodes must be allocated and have valid
> next/prev pointers.
> +/// - The underlying `list_head` (and entire list) must not be modified by C
> for the
> +/// lifetime 'a of `Clist`.
> +pub struct Clist<'a, T> {
> + head: &'a ClistHead,
> + offset: usize,
Joining the chorus suggesting to move `offset` to a const generic. Not
only will it make the struct smaller, it is also better because now
every single C `list_head` becomes its own `Clist` type, allowing you to
discriminate between them in the type system! Not that we will ever make
use of that, but still! :)
In my review of v2 I also suggested to use a closure as the generic
argument, and that the offset case could be a specialization, but
looking at this version I realize this was overengineered - just using
the offset should be enough for all cases, and is much more elegant, so
that looks like the right call indeed.
> + _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.
Overall I think this version looks pretty clean! A nice,
easy to understand wrapper over the C API.
