Messages in this thread |  | | Date | Thu, 19 Mar 2026 13:05:55 +0100 | | Subject | Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface | | From | "Danilo Krummrich" <> |
| |
On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote: > On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote: >> Add a new module `kernel::interop::list` for working with C's doubly >> circular linked lists. Provide low-level iteration over list nodes. >> >> Typed iteration over actual items is provided with a `clist_create` >> macro to assist in creation of the `CList` type. >> >> Cc: Nikola Djukic <ndjukic@nvidia.com> >> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com> >> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> >> Acked-by: Alexandre Courbot <acourbot@nvidia.com> >> Acked-by: Gary Guo <gary@garyguo.net> >> Acked-by: Miguel Ojeda <ojeda@kernel.org> >> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com> >> --- >> MAINTAINERS | 8 + >> rust/helpers/helpers.c | 1 + >> rust/helpers/list.c | 17 ++ >> rust/kernel/interop.rs | 9 + >> rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++ >> rust/kernel/lib.rs | 2 + >> 6 files changed, 379 insertions(+) >> create mode 100644 rust/helpers/list.c >> create mode 100644 rust/kernel/interop.rs >> create mode 100644 rust/kernel/interop/list.rs >> >> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer. >> +/// >> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type` >> +/// linked via the `$field` field in the underlying C struct `$c_type`. >> +/// >> +/// # Arguments >> +/// >> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`). >> +/// - `$rust_type`: Each item's rust wrapper type. >> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`. >> +/// - `$field`: The name of the `list_head` field within the C struct. >> +/// >> +/// # Safety >> +/// >> +/// The caller must ensure: >> +/// >> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`) >> +/// pointing to a list that is not concurrently modified for the lifetime of the [`CList`]. >> +/// - The list contains items of type `$c_type` linked via an embedded `$field`. >> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout. >> +/// >> +/// # Examples >> +/// >> +/// Refer to the examples in the [`crate::interop::list`] module documentation. >> +#[macro_export] >> +macro_rules! clist_create { >> + (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{ >> + // Compile-time check that field path is a `list_head`. >> + // SAFETY: `p` is a valid pointer to `$c_type`. >> + let _: fn(*const $c_type) -> *const $crate::bindings::list_head = >> + |p| unsafe { &raw const (*p).$($field).+ }; > > Actually, this check is insufficient, you should create a reference instead > (just in case people put this inside `repr(packed)`. > > This could be something like > > let _ = |p: &$c_type| { _ = &p.$($field).+ } > > ? > >> + >> + // Calculate offset and create `CList`. >> + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+); >> + // SAFETY: The caller of this macro is responsible for ensuring safety. >> + unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) } > > Given that this is unsafe, I am not sure why the macro should have unsafe > keyword in it, rather than just being `clist_create(a, b, c, d)` and just have > user write unsafe.
Either you are proposing to not wrap unsafe code within unsafe {} within the macro, such that the user is forced to write an unsafe {} around the macro, but then they calls within the macro are not justified individually, or you propose to let the user write an unsafe {} around the macro regardless of the inner unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe and nothing forces the user to actually wrap it in unsafe {}.
Is there a third option I'm not aware of? I.e. for the above reason impl_device_context_deref!() was designed the same way.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650
|  |