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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <44b99b6e-1ab7-404f-aeb4-62eacd40ad8d@nvidia.com>
Date: Wed, 3 Dec 2025 10:08:36 -0500
From: Joel Fernandes <joelagnelf@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 nouveau@...ts.freedesktop.org, Danilo Krummrich <dakr@...nel.org>,
 Dave Airlie <airlied@...il.com>
Cc: Alistair Popple <apopple@...dia.com>, Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>, bjorn3_gh@...tonmail.com,
 Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
 Simona Vetter <simona@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 John Hubbard <jhubbard@...dia.com>, Timur Tabi <ttabi@...dia.com>,
 Joel Fernandes <joel@...lfernandes.org>,
 Lyude Paul <elle@...thered-steel.dev>,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Andrea Righi <arighi@...dia.com>, Philipp Stanner <phasta@...nel.org>,
 Nouveau <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v3] rust: clist: Add support to interface with C linked
 lists

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