[PATCH 0/5] rust: list: remove HasListLinks::OFFSET

[PATCH 0/5] rust: list: remove HasListLinks::OFFSET

[Tamir Duberstein]

The bulk of this change occurs in the last commit, please its commit
messages for details. The first commit exists in 2 other series but was
picked into this series to allow using `container_of!` without the need
to cast from `*const Self` to `*mut Self`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Tamir Duberstein (5):
      rust: retain pointer mut-ness in `container_of!`
      rust: list: simplify macro capture
      rust: list: use consistent type parameter names
      rust: list: use consistent self parameter name
      rust: list: remove OFFSET constants

 rust/kernel/lib.rs                     |   5 +-
 rust/kernel/list.rs                    |  18 +++--
 rust/kernel/list/impl_list_item_mod.rs | 128 +++++++++++++++------------------
 rust/kernel/pci.rs                     |   2 +-
 rust/kernel/platform.rs                |   2 +-
 rust/kernel/rbtree.rs                  |  23 +++---
 6 files changed, 81 insertions(+), 97 deletions(-)
---
base-commit: 28bb48c4cb34f65a9aa602142e76e1426da31293
change-id: 20250324-list-no-offset-96ef65cb2a95

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>

[PATCH 1/5] rust: retain pointer mut-ness in `container_of!`

[Tamir Duberstein]

Avoid casting the input pointer to `*const _`, allowing the output
pointer to be `*mut` if the input is `*mut`. This allows a number of
`*const` to `*mut` conversions to be removed at the cost of slightly
worse ergonomics when the macro is used with a reference rather than a
pointer; the only example of this was in the macro's own doctest.

Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/lib.rs      |  5 ++---
 rust/kernel/pci.rs      |  2 +-
 rust/kernel/platform.rs |  2 +-
 rust/kernel/rbtree.rs   | 23 ++++++++++-------------
 4 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ba0f3b0297b2..cffa0d837f06 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -190,7 +190,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
 /// }
 ///
 /// let test = Test { a: 10, b: 20 };
-/// let b_ptr = &test.b;
+/// let b_ptr: *const _ = &test.b;
 /// // SAFETY: The pointer points at the `b` field of a `Test`, so the resulting pointer will be
 /// // in-bounds of the same allocation as `b_ptr`.
 /// let test_alias = unsafe { container_of!(b_ptr, Test, b) };
@@ -199,9 +199,8 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
 #[macro_export]
 macro_rules! container_of {
     ($ptr:expr, $type:ty, $($f:tt)*) => {{
-        let ptr = $ptr as *const _ as *const u8;
         let offset: usize = ::core::mem::offset_of!($type, $($f)*);
-        ptr.sub(offset) as *const $type
+        $ptr.byte_sub(offset).cast::<$type>()
     }}
 }
 
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index f7b2743828ae..271a7690a9a0 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -364,7 +364,7 @@ pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
     fn as_raw(&self) -> *mut bindings::pci_dev {
         // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
         // embedded in `struct pci_dev`.
-        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) }
     }
 
     /// Returns the PCI vendor ID.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 1297f5292ba9..84a4ecc642a1 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -189,7 +189,7 @@ unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
     fn as_raw(&self) -> *mut bindings::platform_device {
         // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
         // embedded in `struct platform_device`.
-        unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut()
+        unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }
     }
 }
 
diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 5246b2c8a4ff..8d978c896747 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -424,7 +424,7 @@ pub fn cursor_lower_bound(&mut self, key: &K) -> Option<Cursor<'_, K, V>>
         while !node.is_null() {
             // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
             // point to the links field of `Node<K, V>` objects.
-            let this = unsafe { container_of!(node, Node<K, V>, links) }.cast_mut();
+            let this = unsafe { container_of!(node, Node<K, V>, links) };
             // SAFETY: `this` is a non-null node so it is valid by the type invariants.
             let this_key = unsafe { &(*this).key };
             // SAFETY: `node` is a non-null node so it is valid by the type invariants.
@@ -496,7 +496,7 @@ fn drop(&mut self) {
             // but it is not observable. The loop invariant is still maintained.
 
             // SAFETY: `this` is valid per the loop invariant.
-            unsafe { drop(KBox::from_raw(this.cast_mut())) };
+            unsafe { drop(KBox::from_raw(this)) };
         }
     }
 }
@@ -761,7 +761,7 @@ pub fn remove_current(self) -> (Option<Self>, RBTreeNode<K, V>) {
         let next = self.get_neighbor_raw(Direction::Next);
         // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
         // point to the links field of `Node<K, V>` objects.
-        let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) }.cast_mut();
+        let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) };
         // SAFETY: `this` is valid by the type invariants as described above.
         let node = unsafe { KBox::from_raw(this) };
         let node = RBTreeNode { node };
@@ -806,7 +806,7 @@ fn remove_neighbor(&mut self, direction: Direction) -> Option<RBTreeNode<K, V>>
             unsafe { bindings::rb_erase(neighbor, addr_of_mut!(self.tree.root)) };
             // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
             // point to the links field of `Node<K, V>` objects.
-            let this = unsafe { container_of!(neighbor, Node<K, V>, links) }.cast_mut();
+            let this = unsafe { container_of!(neighbor, Node<K, V>, links) };
             // SAFETY: `this` is valid by the type invariants as described above.
             let node = unsafe { KBox::from_raw(this) };
             return Some(RBTreeNode { node });
@@ -912,7 +912,7 @@ unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b
     unsafe fn to_key_value_raw<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, *mut V) {
         // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
         // point to the links field of `Node<K, V>` objects.
-        let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) }.cast_mut();
+        let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) };
         // SAFETY: The passed `node` is the current node or a non-null neighbor,
         // thus `this` is valid by the type invariants.
         let k = unsafe { &(*this).key };
@@ -1021,7 +1021,7 @@ fn next(&mut self) -> Option<Self::Item> {
 
         // SAFETY: By the type invariant of `IterRaw`, `self.next` is a valid node in an `RBTree`,
         // and by the type invariant of `RBTree`, all nodes point to the links field of `Node<K, V>` objects.
-        let cur = unsafe { container_of!(self.next, Node<K, V>, links) }.cast_mut();
+        let cur = unsafe { container_of!(self.next, Node<K, V>, links) };
 
         // SAFETY: `self.next` is a valid tree node by the type invariants.
         self.next = unsafe { bindings::rb_next(self.next) };
@@ -1216,7 +1216,7 @@ pub fn get_mut(&mut self) -> &mut V {
         // SAFETY:
         // - `self.node_links` is a valid pointer to a node in the tree.
         // - We have exclusive access to the underlying tree, and can thus give out a mutable reference.
-        unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+        unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
     }
 
     /// Converts the entry into a mutable reference to its value.
@@ -1226,7 +1226,7 @@ pub fn into_mut(self) -> &'a mut V {
         // SAFETY:
         // - `self.node_links` is a valid pointer to a node in the tree.
         // - This consumes the `&'a mut RBTree<K, V>`, therefore it can give out a mutable reference that lives for `'a`.
-        unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+        unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
     }
 
     /// Remove this entry from the [`RBTree`].
@@ -1239,9 +1239,7 @@ pub fn remove_node(self) -> RBTreeNode<K, V> {
         RBTreeNode {
             // SAFETY: The node was a node in the tree, but we removed it, so we can convert it
             // back into a box.
-            node: unsafe {
-                KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut())
-            },
+            node: unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) },
         }
     }
 
@@ -1272,8 +1270,7 @@ fn replace(self, node: RBTreeNode<K, V>) -> RBTreeNode<K, V> {
         // SAFETY:
         // - `self.node_ptr` produces a valid pointer to a node in the tree.
         // - Now that we removed this entry from the tree, we can convert the node to a box.
-        let old_node =
-            unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut()) };
+        let old_node = unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) };
 
         RBTreeNode { node: old_node }
     }

-- 
2.48.1

[PATCH 2/5] rust: list: simplify macro capture

[Tamir Duberstein]

Avoid manually capturing generics; use `ty` to capture the whole type
instead.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee1..5ed66fdce953 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -43,7 +43,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
 macro_rules! impl_has_list_links {
     ($(impl$(<$($implarg:ident),*>)?
        HasListLinks$(<$id:tt>)?
-       for $self:ident $(<$($selfarg:ty),*>)?
+       for $self:ty
        { self$(.$field:ident)* }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
@@ -51,9 +51,7 @@ macro_rules! impl_has_list_links {
         //
         // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
         // equivalent to the pointer offset operation in the trait definition.
-        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for
-            $self $(<$($selfarg),*>)?
-        {
+        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
 
             #[inline]
@@ -85,18 +83,14 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 macro_rules! impl_has_list_links_self_ptr {
     ($(impl$({$($implarg:tt)*})?
        HasSelfPtr<$item_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ty),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for
-            $self $(<$($selfarg),*>)?
-        {}
+        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for
-            $self $(<$($selfarg),*>)?
-        {
+        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]

-- 
2.48.1

[PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
the same name used in `impl_list_item!`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 5ed66fdce953..9d2102138c48 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
 /// Implements the [`HasListLinks`] trait for the given type.
 #[macro_export]
 macro_rules! impl_has_list_links {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$(<$($generics:ident),*>)?
        HasListLinks$(<$id:tt>)?
        for $self:ty
        { self$(.$field:ident)* }
@@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
         //
         // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
         // equivalent to the pointer offset operation in the trait definition.
-        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
+        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
 
             #[inline]
@@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
 #[macro_export]
 macro_rules! impl_has_list_links_self_ptr {
-    ($(impl$({$($implarg:tt)*})?
+    ($(impl$({$($generics:tt)*})?
        HasSelfPtr<$item_type:ty $(, $id:tt)?>
        for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
+        unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
+        unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]

-- 
2.48.1

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Boqun Feng]

On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
> the same name used in `impl_list_item!`.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> index 5ed66fdce953..9d2102138c48 100644
> --- a/rust/kernel/list/impl_list_item_mod.rs
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
>  /// Implements the [`HasListLinks`] trait for the given type.
>  #[macro_export]
>  macro_rules! impl_has_list_links {
> -    ($(impl$(<$($implarg:ident),*>)?
> +    ($(impl$(<$($generics:ident),*>)?
>         HasListLinks$(<$id:tt>)?
>         for $self:ty
>         { self$(.$field:ident)* }
> @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
>          //
>          // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
>          // equivalent to the pointer offset operation in the trait definition.
> -        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> +        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
>              const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
>  
>              #[inline]
> @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
>  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
>  #[macro_export]
>  macro_rules! impl_has_list_links_self_ptr {
> -    ($(impl$({$($implarg:tt)*})?
> +    ($(impl$({$($generics:tt)*})?

While you're at it, can you also change this to be

	($(impl$(<$($generics:tt)*>)?

?

I don't know why we chose <> for impl_has_list_links, but {} for
impl_has_list_links_self_ptr ;-)

Regards,
Boqun

>         HasSelfPtr<$item_type:ty $(, $id:tt)?>
>         for $self:ty
>         { self.$field:ident }
>      )*) => {$(
>          // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
>          // right type.
> -        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
> +        unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
>  
> -        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> +        unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
>              const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
>  
>              #[inline]
> 
> -- 
> 2.48.1
> 

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
> > the same name used in `impl_list_item!`.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> >  rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> > index 5ed66fdce953..9d2102138c48 100644
> > --- a/rust/kernel/list/impl_list_item_mod.rs
> > +++ b/rust/kernel/list/impl_list_item_mod.rs
> > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> >  /// Implements the [`HasListLinks`] trait for the given type.
> >  #[macro_export]
> >  macro_rules! impl_has_list_links {
> > -    ($(impl$(<$($implarg:ident),*>)?
> > +    ($(impl$(<$($generics:ident),*>)?
> >         HasListLinks$(<$id:tt>)?
> >         for $self:ty
> >         { self$(.$field:ident)* }
> > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
> >          //
> >          // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
> >          // equivalent to the pointer offset operation in the trait definition.
> > -        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > +        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> >              const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
> >
> >              #[inline]
> > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> >  #[macro_export]
> >  macro_rules! impl_has_list_links_self_ptr {
> > -    ($(impl$({$($implarg:tt)*})?
> > +    ($(impl$({$($generics:tt)*})?
>
> While you're at it, can you also change this to be
>
>         ($(impl$(<$($generics:tt)*>)?
>
> ?
>
> I don't know why we chose <> for impl_has_list_links, but {} for
> impl_has_list_links_self_ptr ;-)

This doesn't work in all cases:

error: local ambiguity when calling macro `impl_has_work`: multiple
parsing options: built-in NTs tt ('generics') or 1 other option.
   --> ../rust/kernel/workqueue.rs:522:11
    |
522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }

The reason that `impl_has_list_links` uses <> and all others use {} is
that `impl_has_list_links` is the only one that captures the generic
parameter as an `ident`, the rest use `tt`. So we could change
`impl_has_list_links` to use {}, but not the other way around.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
> > > the same name used in `impl_list_item!`.
> > >
> > > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > > ---
> > >  rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> > > index 5ed66fdce953..9d2102138c48 100644
> > > --- a/rust/kernel/list/impl_list_item_mod.rs
> > > +++ b/rust/kernel/list/impl_list_item_mod.rs
> > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> > >  /// Implements the [`HasListLinks`] trait for the given type.
> > >  #[macro_export]
> > >  macro_rules! impl_has_list_links {
> > > -    ($(impl$(<$($implarg:ident),*>)?
> > > +    ($(impl$(<$($generics:ident),*>)?
> > >         HasListLinks$(<$id:tt>)?
> > >         for $self:ty
> > >         { self$(.$field:ident)* }
> > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
> > >          //
> > >          // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
> > >          // equivalent to the pointer offset operation in the trait definition.
> > > -        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > > +        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > >              const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
> > >
> > >              #[inline]
> > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> > >  #[macro_export]
> > >  macro_rules! impl_has_list_links_self_ptr {
> > > -    ($(impl$({$($implarg:tt)*})?
> > > +    ($(impl$({$($generics:tt)*})?
> >
> > While you're at it, can you also change this to be
> >
> >         ($(impl$(<$($generics:tt)*>)?
> >
> > ?
> >
> > I don't know why we chose <> for impl_has_list_links, but {} for
> > impl_has_list_links_self_ptr ;-)
>
> This doesn't work in all cases:
>
> error: local ambiguity when calling macro `impl_has_work`: multiple
> parsing options: built-in NTs tt ('generics') or 1 other option.
>    --> ../rust/kernel/workqueue.rs:522:11
>     |
> 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
>
> The reason that `impl_has_list_links` uses <> and all others use {} is
> that `impl_has_list_links` is the only one that captures the generic
> parameter as an `ident`, the rest use `tt`. So we could change
> `impl_has_list_links` to use {}, but not the other way around.

I've changed it to `{}` so it's consistent everywhere.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Boqun Feng]

On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> > > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
> > > > the same name used in `impl_list_item!`.
> > > >
> > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > > > ---
> > > >  rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> > > > index 5ed66fdce953..9d2102138c48 100644
> > > > --- a/rust/kernel/list/impl_list_item_mod.rs
> > > > +++ b/rust/kernel/list/impl_list_item_mod.rs
> > > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> > > >  /// Implements the [`HasListLinks`] trait for the given type.
> > > >  #[macro_export]
> > > >  macro_rules! impl_has_list_links {
> > > > -    ($(impl$(<$($implarg:ident),*>)?
> > > > +    ($(impl$(<$($generics:ident),*>)?
> > > >         HasListLinks$(<$id:tt>)?
> > > >         for $self:ty
> > > >         { self$(.$field:ident)* }
> > > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
> > > >          //
> > > >          // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
> > > >          // equivalent to the pointer offset operation in the trait definition.
> > > > -        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > > > +        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > > >              const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
> > > >
> > > >              #[inline]
> > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> > > >  #[macro_export]
> > > >  macro_rules! impl_has_list_links_self_ptr {
> > > > -    ($(impl$({$($implarg:tt)*})?
> > > > +    ($(impl$({$($generics:tt)*})?
> > >
> > > While you're at it, can you also change this to be
> > >
> > >         ($(impl$(<$($generics:tt)*>)?
> > >
> > > ?
> > >
> > > I don't know why we chose <> for impl_has_list_links, but {} for
> > > impl_has_list_links_self_ptr ;-)
> >
> > This doesn't work in all cases:
> >
> > error: local ambiguity when calling macro `impl_has_work`: multiple
> > parsing options: built-in NTs tt ('generics') or 1 other option.
> >    --> ../rust/kernel/workqueue.rs:522:11
> >     |
> > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> >
> > The reason that `impl_has_list_links` uses <> and all others use {} is
> > that `impl_has_list_links` is the only one that captures the generic
> > parameter as an `ident`, the rest use `tt`. So we could change

Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
a bit curious.

Regards,
Boqun

> > `impl_has_list_links` to use {}, but not the other way around.
> 
> I've changed it to `{}` so it's consistent everywhere.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > >
> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> > > > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by
> > > > > the same name used in `impl_list_item!`.
> > > > >
> > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > > > > ---
> > > > >  rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
> > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> > > > > index 5ed66fdce953..9d2102138c48 100644
> > > > > --- a/rust/kernel/list/impl_list_item_mod.rs
> > > > > +++ b/rust/kernel/list/impl_list_item_mod.rs
> > > > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> > > > >  /// Implements the [`HasListLinks`] trait for the given type.
> > > > >  #[macro_export]
> > > > >  macro_rules! impl_has_list_links {
> > > > > -    ($(impl$(<$($implarg:ident),*>)?
> > > > > +    ($(impl$(<$($generics:ident),*>)?
> > > > >         HasListLinks$(<$id:tt>)?
> > > > >         for $self:ty
> > > > >         { self$(.$field:ident)* }
> > > > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
> > > > >          //
> > > > >          // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
> > > > >          // equivalent to the pointer offset operation in the trait definition.
> > > > > -        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > > > > +        unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> > > > >              const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
> > > > >
> > > > >              #[inline]
> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> > > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> > > > >  #[macro_export]
> > > > >  macro_rules! impl_has_list_links_self_ptr {
> > > > > -    ($(impl$({$($implarg:tt)*})?
> > > > > +    ($(impl$({$($generics:tt)*})?
> > > >
> > > > While you're at it, can you also change this to be
> > > >
> > > >         ($(impl$(<$($generics:tt)*>)?
> > > >
> > > > ?
> > > >
> > > > I don't know why we chose <> for impl_has_list_links, but {} for
> > > > impl_has_list_links_self_ptr ;-)
> > >
> > > This doesn't work in all cases:
> > >
> > > error: local ambiguity when calling macro `impl_has_work`: multiple
> > > parsing options: built-in NTs tt ('generics') or 1 other option.
> > >    --> ../rust/kernel/workqueue.rs:522:11
> > >     |
> > > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> > >
> > > The reason that `impl_has_list_links` uses <> and all others use {} is
> > > that `impl_has_list_links` is the only one that captures the generic
> > > parameter as an `ident`, the rest use `tt`. So we could change
>
> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
> a bit curious.

I think it's because `ident` cannot deal with lifetimes or const
generics - or at least I was not able to make it work with them.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Benno Lossin]

On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
>> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
>> > > > >              #[inline]
>> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
>> > > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
>> > > > >  #[macro_export]
>> > > > >  macro_rules! impl_has_list_links_self_ptr {
>> > > > > -    ($(impl$({$($implarg:tt)*})?
>> > > > > +    ($(impl$({$($generics:tt)*})?
>> > > >
>> > > > While you're at it, can you also change this to be
>> > > >
>> > > >         ($(impl$(<$($generics:tt)*>)?
>> > > >
>> > > > ?
>> > > >
>> > > > I don't know why we chose <> for impl_has_list_links, but {} for
>> > > > impl_has_list_links_self_ptr ;-)
>> > >
>> > > This doesn't work in all cases:
>> > >
>> > > error: local ambiguity when calling macro `impl_has_work`: multiple
>> > > parsing options: built-in NTs tt ('generics') or 1 other option.
>> > >    --> ../rust/kernel/workqueue.rs:522:11
>> > >     |
>> > > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
>> > >
>> > > The reason that `impl_has_list_links` uses <> and all others use {} is
>> > > that `impl_has_list_links` is the only one that captures the generic
>> > > parameter as an `ident`, the rest use `tt`. So we could change
>>
>> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
>> a bit curious.
>
> I think it's because `ident` cannot deal with lifetimes or const
> generics - or at least I was not able to make it work with them.

If you use `ident`, you can use the normal `<>` as the delimiters of
generics. For `tt`, you have to use `{}` (or `()`/`[]`). 

---
Cheers,
Benno

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote:
> > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
> >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> >> > > > >              #[inline]
> >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> >> > > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> >> > > > >  #[macro_export]
> >> > > > >  macro_rules! impl_has_list_links_self_ptr {
> >> > > > > -    ($(impl$({$($implarg:tt)*})?
> >> > > > > +    ($(impl$({$($generics:tt)*})?
> >> > > >
> >> > > > While you're at it, can you also change this to be
> >> > > >
> >> > > >         ($(impl$(<$($generics:tt)*>)?
> >> > > >
> >> > > > ?
> >> > > >
> >> > > > I don't know why we chose <> for impl_has_list_links, but {} for
> >> > > > impl_has_list_links_self_ptr ;-)
> >> > >
> >> > > This doesn't work in all cases:
> >> > >
> >> > > error: local ambiguity when calling macro `impl_has_work`: multiple
> >> > > parsing options: built-in NTs tt ('generics') or 1 other option.
> >> > >    --> ../rust/kernel/workqueue.rs:522:11
> >> > >     |
> >> > > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> >> > >
> >> > > The reason that `impl_has_list_links` uses <> and all others use {} is
> >> > > that `impl_has_list_links` is the only one that captures the generic
> >> > > parameter as an `ident`, the rest use `tt`. So we could change
> >>
> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
> >> a bit curious.
> >
> > I think it's because `ident` cannot deal with lifetimes or const
> > generics - or at least I was not able to make it work with them.
>
> If you use `ident`, you can use the normal `<>` as the delimiters of
> generics. For `tt`, you have to use `{}` (or `()`/`[]`).

Yes I know. But with `ident` you cannot capture lifetimes or const generics.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Benno Lossin]

On Tue Mar 25, 2025 at 11:42 AM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote:
>> > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>> >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
>> >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>> >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
>> >> > > > >              #[inline]
>> >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
>> >> > > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
>> >> > > > >  #[macro_export]
>> >> > > > >  macro_rules! impl_has_list_links_self_ptr {
>> >> > > > > -    ($(impl$({$($implarg:tt)*})?
>> >> > > > > +    ($(impl$({$($generics:tt)*})?
>> >> > > >
>> >> > > > While you're at it, can you also change this to be
>> >> > > >
>> >> > > >         ($(impl$(<$($generics:tt)*>)?
>> >> > > >
>> >> > > > ?
>> >> > > >
>> >> > > > I don't know why we chose <> for impl_has_list_links, but {} for
>> >> > > > impl_has_list_links_self_ptr ;-)
>> >> > >
>> >> > > This doesn't work in all cases:
>> >> > >
>> >> > > error: local ambiguity when calling macro `impl_has_work`: multiple
>> >> > > parsing options: built-in NTs tt ('generics') or 1 other option.
>> >> > >    --> ../rust/kernel/workqueue.rs:522:11
>> >> > >     |
>> >> > > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
>> >> > >
>> >> > > The reason that `impl_has_list_links` uses <> and all others use {} is
>> >> > > that `impl_has_list_links` is the only one that captures the generic
>> >> > > parameter as an `ident`, the rest use `tt`. So we could change
>> >>
>> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
>> >> a bit curious.
>> >
>> > I think it's because `ident` cannot deal with lifetimes or const
>> > generics - or at least I was not able to make it work with them.
>>
>> If you use `ident`, you can use the normal `<>` as the delimiters of
>> generics. For `tt`, you have to use `{}` (or `()`/`[]`).
>
> Yes I know. But with `ident` you cannot capture lifetimes or const generics.

Why is that required for this macro? I think we could use `tt`.

---
Cheers,
Benno

Re: [PATCH 3/5] rust: list: use consistent type parameter names

[Tamir Duberstein]

On Tue, Mar 25, 2025 at 7:18 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Tue Mar 25, 2025 at 11:42 AM CET, Tamir Duberstein wrote:
> > On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >> On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote:
> >> > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >> >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote:
> >> >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >> >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >> >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote:
> >> >> > > > >              #[inline]
> >> >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> >> >> > > > >  /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> >> >> > > > >  #[macro_export]
> >> >> > > > >  macro_rules! impl_has_list_links_self_ptr {
> >> >> > > > > -    ($(impl$({$($implarg:tt)*})?
> >> >> > > > > +    ($(impl$({$($generics:tt)*})?
> >> >> > > >
> >> >> > > > While you're at it, can you also change this to be
> >> >> > > >
> >> >> > > >         ($(impl$(<$($generics:tt)*>)?
> >> >> > > >
> >> >> > > > ?
> >> >> > > >
> >> >> > > > I don't know why we chose <> for impl_has_list_links, but {} for
> >> >> > > > impl_has_list_links_self_ptr ;-)
> >> >> > >
> >> >> > > This doesn't work in all cases:
> >> >> > >
> >> >> > > error: local ambiguity when calling macro `impl_has_work`: multiple
> >> >> > > parsing options: built-in NTs tt ('generics') or 1 other option.
> >> >> > >    --> ../rust/kernel/workqueue.rs:522:11
> >> >> > >     |
> >> >> > > 522 |     impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> >> >> > >
> >> >> > > The reason that `impl_has_list_links` uses <> and all others use {} is
> >> >> > > that `impl_has_list_links` is the only one that captures the generic
> >> >> > > parameter as an `ident`, the rest use `tt`. So we could change
> >> >>
> >> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm
> >> >> a bit curious.
> >> >
> >> > I think it's because `ident` cannot deal with lifetimes or const
> >> > generics - or at least I was not able to make it work with them.
> >>
> >> If you use `ident`, you can use the normal `<>` as the delimiters of
> >> generics. For `tt`, you have to use `{}` (or `()`/`[]`).
> >
> > Yes I know. But with `ident` you cannot capture lifetimes or const generics.
>
> Why is that required for this macro? I think we could use `tt`.

We're in violent agreement. The change I've made is to use `tt`
everywhere, including where `ident` is currently used with `<>`.

[PATCH 4/5] rust: list: use consistent self parameter name

[Tamir Duberstein]

Refer to the self parameter of `impl_list_item!` by the same name used
in `impl_has_list_links{,_self_ptr}!`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 9d2102138c48..705b46150b97 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -114,12 +114,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 #[macro_export]
 macro_rules! impl_list_item {
     (
-        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
             using ListLinks;
         })*
     ) => {$(
         // SAFETY: See GUARANTEES comment on each method.
-        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
             // * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
             //   is implemented in terms of `view_links`.
@@ -178,12 +178,12 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
     )*};
 
     (
-        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
             using ListLinksSelfPtr;
         })*
     ) => {$(
         // SAFETY: See GUARANTEES comment on each method.
-        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
             // This implementation of `ListItem` will not give out exclusive access to the same
             // `ListLinks` several times because calls to `prepare_to_insert` and `post_remove`

-- 
2.48.1

[PATCH 5/5] rust: list: remove OFFSET constants

[Tamir Duberstein]

Replace `ListLinksSelfPtr::LIST_LINKS_SELF_PTR_OFFSET` with `unsafe fn
raw_get_self_ptr` which returns a pointer to the field rather than
requiring the caller to do pointer arithmetic.

Implement `HasListLinks::raw_get_list_links` in `impl_has_list_links!`,
narrowing the interface of `HasListLinks` and replacing pointer
arithmetic with `container_of!`.

Modify `impl_list_item` to also invoke `impl_has_list_links!` or
`impl_has_list_links_self_ptr!`. This is necessary to allow
`impl_list_item` to see more of the tokens used by
`impl_has_list_links{,_self_ptr}!`.

A similar API change was discussed on the hrtimer series[1].

Link: https://lore.kernel.org/all/20250224-hrtimer-v3-v6-12-rc2-v9-1-5bd3bf0ce6cc@kernel.org/ [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list.rs                    |  18 +++---
 rust/kernel/list/impl_list_item_mod.rs | 100 +++++++++++++++------------------
 2 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index a335c3b1ff5e..f370a8c1df98 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -212,9 +212,6 @@ unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
 unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
 
 impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
-    /// The offset from the [`ListLinks`] to the self pointer field.
-    pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
-
     /// Creates a new initializer for this type.
     pub fn new() -> impl PinInit<Self> {
         // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
@@ -229,6 +226,16 @@ pub fn new() -> impl PinInit<Self> {
             self_ptr: Opaque::uninit(),
         }
     }
+
+    /// Returns a pointer to the self pointer.
+    ///
+    /// # Safety
+    ///
+    /// The provided pointer must point at a valid struct of type `Self`.
+    pub unsafe fn raw_get_self_ptr(me: *mut Self) -> *const Opaque<*const T> {
+        // SAFETY: The caller promises that the pointer is valid.
+        unsafe { ptr::addr_of!((*me).self_ptr) }
+    }
 }
 
 impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
@@ -603,14 +610,11 @@ fn next(&mut self) -> Option<ArcBorrow<'a, T>> {
 ///     }
 /// }
 ///
-/// kernel::list::impl_has_list_links! {
-///     impl HasListLinks<0> for ListItem { self.links }
-/// }
 /// kernel::list::impl_list_arc_safe! {
 ///     impl ListArcSafe<0> for ListItem { untracked; }
 /// }
 /// kernel::list::impl_list_item! {
-///     impl ListItem<0> for ListItem { using ListLinks; }
+///     impl ListItem<0> for ListItem { using ListLinks { self.links }; }
 /// }
 ///
 /// // Use a cursor to remove the first element with the given value.
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 705b46150b97..4f9100aadbce 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -6,21 +6,18 @@
 
 use crate::list::ListLinks;
 
-/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
+/// Declares that this type has a [`ListLinks<ID>`] field.
 ///
-/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
+/// This trait is only used to help implement [`ListItem`] safely. If [`ListItem`] is implemented
 /// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
 /// this trait.
 ///
 /// # Safety
 ///
-/// All values of this type must have a `ListLinks<ID>` field at the given offset.
+/// The methods on this trait must have exactly the behavior that the definitions given below have.
 ///
-/// The behavior of `raw_get_list_links` must not be changed.
+/// [`ListItem`]: crate::list::ListItem
 pub unsafe trait HasListLinks<const ID: u64 = 0> {
-    /// The offset of the `ListLinks` field.
-    const OFFSET: usize;
-
     /// Returns a pointer to the [`ListLinks<T, ID>`] field.
     ///
     /// # Safety
@@ -28,14 +25,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
     /// The provided pointer must point at a valid struct of type `Self`.
     ///
     /// [`ListLinks<T, ID>`]: ListLinks
-    // We don't really need this method, but it's necessary for the implementation of
-    // `impl_has_list_links!` to be correct.
-    #[inline]
-    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
-        // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
-        // `OFFSET` constant is correct.
-        unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
-    }
+    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID>;
 }
 
 /// Implements the [`HasListLinks`] trait for the given type.
@@ -48,14 +38,11 @@ macro_rules! impl_has_list_links {
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        //
-        // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
-        // equivalent to the pointer offset operation in the trait definition.
         unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
-            const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
-
             #[inline]
             unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
+                const _: usize = ::core::mem::offset_of!($self, $($field).*);
+
                 // SAFETY: The caller promises that the pointer is not dangling. We know that this
                 // expression doesn't follow any pointers, as the `offset_of!` invocation above
                 // would otherwise not compile.
@@ -66,12 +53,15 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 }
 pub use impl_has_list_links;
 
-/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
+/// Declares that the [`ListLinks<ID>`] field in this struct is inside a
+/// [`ListLinksSelfPtr<T, ID>`].
 ///
 /// # Safety
 ///
-/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
-/// inside a `ListLinksSelfPtr<T, ID>`.
+/// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
+/// inside a [`ListLinksSelfPtr<T, ID>`].
+///
+/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
 pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 where
     Self: HasListLinks<ID>,
@@ -91,8 +81,6 @@ macro_rules! impl_has_list_links_self_ptr {
         unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
         unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
-            const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
-
             #[inline]
             unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
                 // SAFETY: The caller promises that the pointer is not dangling.
@@ -115,9 +103,13 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 macro_rules! impl_list_item {
     (
         $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
-            using ListLinks;
+            using ListLinks { self$(.$field:ident)* };
         })*
     ) => {$(
+        $crate::list::impl_has_list_links! {
+            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
+        }
+
         // SAFETY: See GUARANTEES comment on each method.
         unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
@@ -133,20 +125,19 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
             }
 
             // GUARANTEES:
-            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
-            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
-            //   `offset` from `me` so it returns the pointer originally passed to
-            //   `prepare_to_insert`.
+            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
+            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
+            //   This method uses `container_of` to perform the inverse operation, so it returns the
+            //   pointer originally passed to `prepare_to_insert`.
             // * The pointer remains valid until the next call to `post_remove` because the caller
             //   of the most recent call to `prepare_to_insert` promised to retain ownership of the
             //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
             //   be destroyed while a `ListArc` reference exists.
             unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
-                // points at the field at offset `offset` in a value of type `Self`. Thus,
-                // subtracting `offset` from `me` is still in-bounds of the allocation.
-                unsafe { (me as *const u8).sub(offset) as *const Self }
+                // points at the field `$field` in a value of type `Self`. Thus, reversing that
+                // operation is still in-bounds of the allocation.
+                $crate::container_of!(me, Self, $($field)*)
             }
 
             // GUARANTEES:
@@ -163,25 +154,28 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
             }
 
             // GUARANTEES:
-            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
-            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
-            //   `offset` from `me` so it returns the pointer originally passed to
-            //   `prepare_to_insert`.
+            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
+            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
+            //   This method uses `container_of` to perform the inverse operation, so it returns the
+            //   pointer originally passed to `prepare_to_insert`.
             unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
-                // points at the field at offset `offset` in a value of type `Self`. Thus,
-                // subtracting `offset` from `me` is still in-bounds of the allocation.
-                unsafe { (me as *const u8).sub(offset) as *const Self }
+                // points at the field `$field` in a value of type `Self`. Thus, reversing that
+                // operation is still in-bounds of the allocation.
+                $crate::container_of!(me, Self, $($field)*)
             }
         }
     )*};
 
     (
         $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
-            using ListLinksSelfPtr;
+            using ListLinksSelfPtr { self$(.$field:ident)* };
         })*
     ) => {$(
+        $crate::list::impl_has_list_links_self_ptr! {
+            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
+        }
+
         // SAFETY: See GUARANTEES comment on each method.
         unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
@@ -196,13 +190,10 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
                 // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
                 let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
 
-                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
-                // Goes via the offset as the field is private.
-                //
-                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
-                // the pointer stays in bounds of the allocation.
-                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
-                    as *const $crate::types::Opaque<*const Self>;
+                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+                let self_ptr = unsafe {
+                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
+                };
                 let cell_inner = $crate::types::Opaque::raw_get(self_ptr);
 
                 // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
@@ -241,11 +232,10 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
             //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
             //   be destroyed while a `ListArc` reference exists.
             unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
-                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
-                // the pointer stays in bounds of the allocation.
-                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
-                    as *const ::core::cell::UnsafeCell<*const Self>;
+                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+                let self_ptr = unsafe {
+                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
+                };
                 let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
                 // SAFETY: This is not a data race, because the only function that writes to this
                 // value is `prepare_to_insert`, but by the safety requirements the

-- 
2.48.1

Re: [PATCH 5/5] rust: list: remove OFFSET constants

[Alice Ryhl]

On Mon, Mar 24, 2025 at 05:33:47PM -0400, Tamir Duberstein wrote:
> Replace `ListLinksSelfPtr::LIST_LINKS_SELF_PTR_OFFSET` with `unsafe fn
> raw_get_self_ptr` which returns a pointer to the field rather than
> requiring the caller to do pointer arithmetic.
> 
> Implement `HasListLinks::raw_get_list_links` in `impl_has_list_links!`,
> narrowing the interface of `HasListLinks` and replacing pointer
> arithmetic with `container_of!`.
> 
> Modify `impl_list_item` to also invoke `impl_has_list_links!` or
> `impl_has_list_links_self_ptr!`. This is necessary to allow
> `impl_list_item` to see more of the tokens used by
> `impl_has_list_links{,_self_ptr}!`.
> 
> A similar API change was discussed on the hrtimer series[1].
> 
> Link: https://lore.kernel.org/all/20250224-hrtimer-v3-v6-12-rc2-v9-1-5bd3bf0ce6cc@kernel.org/ [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  rust/kernel/list.rs                    |  18 +++---
>  rust/kernel/list/impl_list_item_mod.rs | 100 +++++++++++++++------------------
>  2 files changed, 56 insertions(+), 62 deletions(-)
> 
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index a335c3b1ff5e..f370a8c1df98 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -212,9 +212,6 @@ unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
>  unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
>  
>  impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
> -    /// The offset from the [`ListLinks`] to the self pointer field.
> -    pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
> -
>      /// Creates a new initializer for this type.
>      pub fn new() -> impl PinInit<Self> {
>          // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
> @@ -229,6 +226,16 @@ pub fn new() -> impl PinInit<Self> {
>              self_ptr: Opaque::uninit(),
>          }
>      }
> +
> +    /// Returns a pointer to the self pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The provided pointer must point at a valid struct of type `Self`.
> +    pub unsafe fn raw_get_self_ptr(me: *mut Self) -> *const Opaque<*const T> {
> +        // SAFETY: The caller promises that the pointer is valid.
> +        unsafe { ptr::addr_of!((*me).self_ptr) }
> +    }
>  }
>  
>  impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
> @@ -603,14 +610,11 @@ fn next(&mut self) -> Option<ArcBorrow<'a, T>> {
>  ///     }
>  /// }
>  ///
> -/// kernel::list::impl_has_list_links! {
> -///     impl HasListLinks<0> for ListItem { self.links }
> -/// }
>  /// kernel::list::impl_list_arc_safe! {
>  ///     impl ListArcSafe<0> for ListItem { untracked; }
>  /// }
>  /// kernel::list::impl_list_item! {
> -///     impl ListItem<0> for ListItem { using ListLinks; }
> +///     impl ListItem<0> for ListItem { using ListLinks { self.links }; }
>  /// }
>  ///
>  /// // Use a cursor to remove the first element with the given value.
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> index 705b46150b97..4f9100aadbce 100644
> --- a/rust/kernel/list/impl_list_item_mod.rs
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -6,21 +6,18 @@
>  
>  use crate::list::ListLinks;
>  
> -/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
> +/// Declares that this type has a [`ListLinks<ID>`] field.
>  ///
> -/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
> +/// This trait is only used to help implement [`ListItem`] safely. If [`ListItem`] is implemented
>  /// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
>  /// this trait.
>  ///
>  /// # Safety
>  ///
> -/// All values of this type must have a `ListLinks<ID>` field at the given offset.
> +/// The methods on this trait must have exactly the behavior that the definitions given below have.
>  ///
> -/// The behavior of `raw_get_list_links` must not be changed.
> +/// [`ListItem`]: crate::list::ListItem
>  pub unsafe trait HasListLinks<const ID: u64 = 0> {
> -    /// The offset of the `ListLinks` field.
> -    const OFFSET: usize;
> -
>      /// Returns a pointer to the [`ListLinks<T, ID>`] field.
>      ///
>      /// # Safety
> @@ -28,14 +25,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
>      /// The provided pointer must point at a valid struct of type `Self`.
>      ///
>      /// [`ListLinks<T, ID>`]: ListLinks
> -    // We don't really need this method, but it's necessary for the implementation of
> -    // `impl_has_list_links!` to be correct.
> -    #[inline]
> -    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> -        // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
> -        // `OFFSET` constant is correct.
> -        unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
> -    }
> +    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID>;
>  }
>  
>  /// Implements the [`HasListLinks`] trait for the given type.
> @@ -48,14 +38,11 @@ macro_rules! impl_has_list_links {
>      )*) => {$(
>          // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
>          // right type.
> -        //
> -        // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
> -        // equivalent to the pointer offset operation in the trait definition.
>          unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> -            const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
> -
>              #[inline]
>              unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
> +                const _: usize = ::core::mem::offset_of!($self, $($field).*);
> +
>                  // SAFETY: The caller promises that the pointer is not dangling. We know that this
>                  // expression doesn't follow any pointers, as the `offset_of!` invocation above
>                  // would otherwise not compile.
> @@ -66,12 +53,15 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
>  }
>  pub use impl_has_list_links;
>  
> -/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
> +/// Declares that the [`ListLinks<ID>`] field in this struct is inside a
> +/// [`ListLinksSelfPtr<T, ID>`].
>  ///
>  /// # Safety
>  ///
> -/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
> -/// inside a `ListLinksSelfPtr<T, ID>`.
> +/// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
> +/// inside a [`ListLinksSelfPtr<T, ID>`].
> +///
> +/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
>  pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
>  where
>      Self: HasListLinks<ID>,
> @@ -91,8 +81,6 @@ macro_rules! impl_has_list_links_self_ptr {
>          unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
>  
>          unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> -            const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
> -
>              #[inline]
>              unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
>                  // SAFETY: The caller promises that the pointer is not dangling.
> @@ -115,9 +103,13 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
>  macro_rules! impl_list_item {
>      (
>          $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
> -            using ListLinks;
> +            using ListLinks { self$(.$field:ident)* };
>          })*
>      ) => {$(
> +        $crate::list::impl_has_list_links! {
> +            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
> +        }
> +
>          // SAFETY: See GUARANTEES comment on each method.
>          unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
>              // GUARANTEES:
> @@ -133,20 +125,19 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
>              }
>  
>              // GUARANTEES:
> -            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
> -            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
> -            //   `offset` from `me` so it returns the pointer originally passed to
> -            //   `prepare_to_insert`.
> +            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
> +            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
> +            //   This method uses `container_of` to perform the inverse operation, so it returns the
> +            //   pointer originally passed to `prepare_to_insert`.
>              // * The pointer remains valid until the next call to `post_remove` because the caller
>              //   of the most recent call to `prepare_to_insert` promised to retain ownership of the
>              //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
>              //   be destroyed while a `ListArc` reference exists.
>              unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> -                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
>                  // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
> -                // points at the field at offset `offset` in a value of type `Self`. Thus,
> -                // subtracting `offset` from `me` is still in-bounds of the allocation.
> -                unsafe { (me as *const u8).sub(offset) as *const Self }
> +                // points at the field `$field` in a value of type `Self`. Thus, reversing that
> +                // operation is still in-bounds of the allocation.
> +                $crate::container_of!(me, Self, $($field)*)
>              }
>  
>              // GUARANTEES:
> @@ -163,25 +154,28 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
>              }
>  
>              // GUARANTEES:
> -            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
> -            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
> -            //   `offset` from `me` so it returns the pointer originally passed to
> -            //   `prepare_to_insert`.
> +            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
> +            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
> +            //   This method uses `container_of` to perform the inverse operation, so it returns the
> +            //   pointer originally passed to `prepare_to_insert`.
>              unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> -                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
>                  // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
> -                // points at the field at offset `offset` in a value of type `Self`. Thus,
> -                // subtracting `offset` from `me` is still in-bounds of the allocation.
> -                unsafe { (me as *const u8).sub(offset) as *const Self }
> +                // points at the field `$field` in a value of type `Self`. Thus, reversing that
> +                // operation is still in-bounds of the allocation.
> +                $crate::container_of!(me, Self, $($field)*)
>              }
>          }
>      )*};
>  
>      (
>          $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
> -            using ListLinksSelfPtr;
> +            using ListLinksSelfPtr { self$(.$field:ident)* };
>          })*
>      ) => {$(
> +        $crate::list::impl_has_list_links_self_ptr! {
> +            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
> +        }
> +
>          // SAFETY: See GUARANTEES comment on each method.
>          unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
>              // GUARANTEES:
> @@ -196,13 +190,10 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
>                  // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
>                  let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
>  
> -                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
> -                // Goes via the offset as the field is private.
> -                //
> -                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
> -                // the pointer stays in bounds of the allocation.
> -                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
> -                    as *const $crate::types::Opaque<*const Self>;
> +                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
> +                let self_ptr = unsafe {
> +                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
> +                };
>                  let cell_inner = $crate::types::Opaque::raw_get(self_ptr);
>  
>                  // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
> @@ -241,11 +232,10 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
>              //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
>              //   be destroyed while a `ListArc` reference exists.
>              unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
> -                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
> -                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
> -                // the pointer stays in bounds of the allocation.
> -                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
> -                    as *const ::core::cell::UnsafeCell<*const Self>;
> +                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
> +                let self_ptr = unsafe {
> +                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
> +                };
>                  let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);

I ran this with Rust Binder. After adjusting the macro invocations, I
got this error:

error[E0308]: mismatched types
   --> /proc/self/cwd/common/drivers/android/binder/rust_binder.rs:178:1
    |
178 | / kernel::list::impl_list_item! {
179 | |     impl ListItem<0> for DTRWrap<dyn DeliverToRead> {
180 | |         using ListLinksSelfPtr { self.links };
181 | |     }
182 | | }
    | | ^
    | | |
    | |_expected `*mut ListLinksSelfPtr<DTRWrap<...>>`, found `*mut ListLinks`
    |   arguments to this function are incorrect
    |
    = note: expected raw pointer `*mut ListLinksSelfPtr<DTRWrap<(dyn DeliverToRead + 'static)>>`
               found raw pointer `*mut ListLinks`
note: associated function defined here
   --> /proc/self/cwd/common/rust/kernel/list.rs:235:19
    = note: this error originates in the macro `kernel::list::impl_list_item` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> /proc/self/cwd/common/drivers/android/binder/rust_binder.rs:178:1
    |
178 | / kernel::list::impl_list_item! {
179 | |     impl ListItem<0> for DTRWrap<dyn DeliverToRead> {
180 | |         using ListLinksSelfPtr { self.links };
181 | |     }
182 | | }
    | | ^
    | | |
    | |_expected `*const UnsafeCell<_>`, found `*const Opaque<*const DTRWrap<...>>`
    |   arguments to this function are incorrect
    |
    = note: expected raw pointer `*const UnsafeCell<_>`
               found raw pointer `*const Opaque<*const DTRWrap<(dyn DeliverToRead + 'static)>>`
note: associated function defined here
   --> /proc/self/cwd/prebuilts/rust/linux-x86/1.82.0/lib/rustlib/src/rust/library/core/src/cell.rs:2210:18
    = note: this error originates in the macro `kernel::list::impl_list_item` (in Nightly builds, run with -Z macro-backtrace for more info)


The relevant code is:

    #[pin_data]
    struct DTRWrap<T: ?Sized> {
        #[pin]
        links: ListLinksSelfPtr<DTRWrap<dyn DeliverToRead>>,
        #[pin]
        wrapped: T,
    }
    kernel::list::impl_list_arc_safe! {
        impl{T: ListArcSafe + ?Sized} ListArcSafe<0> for DTRWrap<T> {
            tracked_by wrapped: T;
        }
    }
    kernel::list::impl_list_item! {
        impl ListItem<0> for DTRWrap<dyn DeliverToRead> {
            using ListLinksSelfPtr { self.links };
        }
    }

Alice

Re: [PATCH 5/5] rust: list: remove OFFSET constants

[Alice Ryhl]

On Tue, Apr 22, 2025 at 4:08 PM Alice Ryhl <aliceryhl@google.com> wrote:
>

Sorry this reply is to v1 by accident, but I did run the test using v2.

Alice