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

[PATCH] 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>
---
This patch is extracted from 3 other series to reduce duplication.
---
 rust/kernel/lib.rs    |  5 ++---
 rust/kernel/rbtree.rs | 23 ++++++++++-------------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5..1df11156302a 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/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 }
     }

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250409-container-of-mutness-b153dab4388d

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

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Tamir Duberstein]

On Wed, Apr 9, 2025 at 10:43 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> 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>
> ---
> This patch is extracted from 3 other series to reduce duplication.

Oops, forgot to include

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

from https://lore.kernel.org/all/87semhbncg.fsf@kernel.org/. I hope
this is proper form!

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Alice Ryhl]

On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> 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>
> ---
> This patch is extracted from 3 other series to reduce duplication.
> ---
>  rust/kernel/lib.rs    |  5 ++---
>  rust/kernel/rbtree.rs | 23 ++++++++++-------------
>  2 files changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5..1df11156302a 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>()
>      }}
>  }

This implementation does not check the type of `ptr`. Would we not want
it to have the type of the field?

Alice

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Tamir Duberstein]

On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> > 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>
> > ---
> > This patch is extracted from 3 other series to reduce duplication.
> > ---
> >  rust/kernel/lib.rs    |  5 ++---
> >  rust/kernel/rbtree.rs | 23 ++++++++++-------------
> >  2 files changed, 12 insertions(+), 16 deletions(-)
> >
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index de07aadd1ff5..1df11156302a 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>()
> >      }}
> >  }
>
> This implementation does not check the type of `ptr`. Would we not want
> it to have the type of the field?

I might be missing it but ISTM that the current implementation doesn't
check that either.

It's not obvious to me how you'd implement such a check; given `$ptr`
and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?

Perhaps you could send a separate patch implementing this.

Tamir

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Benno Lossin]

On Fri Apr 11, 2025 at 2:25 PM CEST, Tamir Duberstein wrote:
> On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
>> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
>> > @@ -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>()
>> >      }}
>> >  }
>>
>> This implementation does not check the type of `ptr`. Would we not want
>> it to have the type of the field?
>
> I might be missing it but ISTM that the current implementation doesn't
> check that either.
>
> It's not obvious to me how you'd implement such a check; given `$ptr`
> and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?

I don't think it's possible with current rust, but maybe with field
projection (:

    ($ptr:expr, $type:ty, $($f:tt)*) => {{
        // do not run this code, only use it for type-checking:
        let _ = || {
            let mut ptr = $ptr;
            ptr = $ptr.cast::<<field_of!($t, $($f)*) as Field>::Type>();
        };
        let offset: usize = ::core::mem::offset_of!($type, $($f)*);
        $ptr.byte_sub(offset).cast::<$type>()
    }}

---
Cheers,
Benno

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Alice Ryhl]

On Fri, Apr 11, 2025 at 2:35 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Fri Apr 11, 2025 at 2:25 PM CEST, Tamir Duberstein wrote:
> > On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> >> > @@ -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>()
> >> >      }}
> >> >  }
> >>
> >> This implementation does not check the type of `ptr`. Would we not want
> >> it to have the type of the field?
> >
> > I might be missing it but ISTM that the current implementation doesn't
> > check that either.
> >
> > It's not obvious to me how you'd implement such a check; given `$ptr`
> > and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?
>
> I don't think it's possible with current rust, but maybe with field
> projection (:
>
>     ($ptr:expr, $type:ty, $($f:tt)*) => {{
>         // do not run this code, only use it for type-checking:
>         let _ = || {
>             let mut ptr = $ptr;
>             ptr = $ptr.cast::<<field_of!($t, $($f)*) as Field>::Type>();
>         };
>         let offset: usize = ::core::mem::offset_of!($type, $($f)*);
>         $ptr.byte_sub(offset).cast::<$type>()
>     }}

You can definitely implement the check with current Rust. You use
addr_of! to create a raw pointer with the type of the field, and
trigger a type error if `ptr` doesn't have the same type as that other
pointer. Something along these lines would do it:

let mut ptr = $ptr;
let offset: usize = ::core::mem::offset_of!($type, $($f)*);
let container = ptr.byte_sub(offset).cast::<$type>();
if false {
    ptr = ::core::ptr::addr_of!((*container).$($f)*).cast_mut();
}
container

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Tamir Duberstein]

On Fri, Apr 11, 2025 at 8:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Fri, Apr 11, 2025 at 2:35 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > On Fri Apr 11, 2025 at 2:25 PM CEST, Tamir Duberstein wrote:
> > > On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > >> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> > >> > @@ -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>()
> > >> >      }}
> > >> >  }
> > >>
> > >> This implementation does not check the type of `ptr`. Would we not want
> > >> it to have the type of the field?
> > >
> > > I might be missing it but ISTM that the current implementation doesn't
> > > check that either.
> > >
> > > It's not obvious to me how you'd implement such a check; given `$ptr`
> > > and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?
> >
> > I don't think it's possible with current rust, but maybe with field
> > projection (:
> >
> >     ($ptr:expr, $type:ty, $($f:tt)*) => {{
> >         // do not run this code, only use it for type-checking:
> >         let _ = || {
> >             let mut ptr = $ptr;
> >             ptr = $ptr.cast::<<field_of!($t, $($f)*) as Field>::Type>();
> >         };
> >         let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> >         $ptr.byte_sub(offset).cast::<$type>()
> >     }}
>
> You can definitely implement the check with current Rust. You use
> addr_of! to create a raw pointer with the type of the field, and
> trigger a type error if `ptr` doesn't have the same type as that other
> pointer. Something along these lines would do it:
>
> let mut ptr = $ptr;
> let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> let container = ptr.byte_sub(offset).cast::<$type>();
> if false {
>     ptr = ::core::ptr::addr_of!((*container).$($f)*).cast_mut();
> }
> container

It's a nice idea. Wouldn't it require `$ptr` to be `*mut _` and not
work with `*const _`? In any case, I hope we agree that this can be
done separately.

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Alice Ryhl]

On Fri, Apr 11, 2025 at 3:09 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Fri, Apr 11, 2025 at 8:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > On Fri, Apr 11, 2025 at 2:35 PM Benno Lossin <benno.lossin@proton.me> wrote:
> > >
> > > On Fri Apr 11, 2025 at 2:25 PM CEST, Tamir Duberstein wrote:
> > > > On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > > >> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> > > >> > @@ -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>()
> > > >> >      }}
> > > >> >  }
> > > >>
> > > >> This implementation does not check the type of `ptr`. Would we not want
> > > >> it to have the type of the field?
> > > >
> > > > I might be missing it but ISTM that the current implementation doesn't
> > > > check that either.
> > > >
> > > > It's not obvious to me how you'd implement such a check; given `$ptr`
> > > > and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?
> > >
> > > I don't think it's possible with current rust, but maybe with field
> > > projection (:
> > >
> > >     ($ptr:expr, $type:ty, $($f:tt)*) => {{
> > >         // do not run this code, only use it for type-checking:
> > >         let _ = || {
> > >             let mut ptr = $ptr;
> > >             ptr = $ptr.cast::<<field_of!($t, $($f)*) as Field>::Type>();
> > >         };
> > >         let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> > >         $ptr.byte_sub(offset).cast::<$type>()
> > >     }}
> >
> > You can definitely implement the check with current Rust. You use
> > addr_of! to create a raw pointer with the type of the field, and
> > trigger a type error if `ptr` doesn't have the same type as that other
> > pointer. Something along these lines would do it:
> >
> > let mut ptr = $ptr;
> > let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> > let container = ptr.byte_sub(offset).cast::<$type>();
> > if false {
> >     ptr = ::core::ptr::addr_of!((*container).$($f)*).cast_mut();
> > }
> > container
>
> It's a nice idea. Wouldn't it require `$ptr` to be `*mut _` and not
> work with `*const _`?

As far as I can tell, it should work with const pointers too. The rhs
of the last assignment gets downgraded back to a const pointer in that
case.

> In any case, I hope we agree that this can be
> done separately.

Yes, since the feature does not exist today, it can happen separately.

Alice

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Tamir Duberstein]

On Fri, Apr 11, 2025 at 9:29 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Fri, Apr 11, 2025 at 3:09 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > On Fri, Apr 11, 2025 at 8:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > >
> > > On Fri, Apr 11, 2025 at 2:35 PM Benno Lossin <benno.lossin@proton.me> wrote:
> > > >
> > > > On Fri Apr 11, 2025 at 2:25 PM CEST, Tamir Duberstein wrote:
> > > > > On Fri, Apr 11, 2025 at 5:02 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > > > >> On Wed, Apr 09, 2025 at 10:43:16AM -0400, Tamir Duberstein wrote:
> > > > >> > @@ -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>()
> > > > >> >      }}
> > > > >> >  }
> > > > >>
> > > > >> This implementation does not check the type of `ptr`. Would we not want
> > > > >> it to have the type of the field?
> > > > >
> > > > > I might be missing it but ISTM that the current implementation doesn't
> > > > > check that either.
> > > > >
> > > > > It's not obvious to me how you'd implement such a check; given `$ptr`
> > > > > and `$f`, how do you get your hands on the type of `$ptr->$($f)*`?
> > > >
> > > > I don't think it's possible with current rust, but maybe with field
> > > > projection (:
> > > >
> > > >     ($ptr:expr, $type:ty, $($f:tt)*) => {{
> > > >         // do not run this code, only use it for type-checking:
> > > >         let _ = || {
> > > >             let mut ptr = $ptr;
> > > >             ptr = $ptr.cast::<<field_of!($t, $($f)*) as Field>::Type>();
> > > >         };
> > > >         let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> > > >         $ptr.byte_sub(offset).cast::<$type>()
> > > >     }}
> > >
> > > You can definitely implement the check with current Rust. You use
> > > addr_of! to create a raw pointer with the type of the field, and
> > > trigger a type error if `ptr` doesn't have the same type as that other
> > > pointer. Something along these lines would do it:
> > >
> > > let mut ptr = $ptr;
> > > let offset: usize = ::core::mem::offset_of!($type, $($f)*);
> > > let container = ptr.byte_sub(offset).cast::<$type>();
> > > if false {
> > >     ptr = ::core::ptr::addr_of!((*container).$($f)*).cast_mut();
> > > }
> > > container
> >
> > It's a nice idea. Wouldn't it require `$ptr` to be `*mut _` and not
> > work with `*const _`?
>
> As far as I can tell, it should work with const pointers too. The rhs
> of the last assignment gets downgraded back to a const pointer in that
> case.
>
> > In any case, I hope we agree that this can be
> > done separately.
>
> Yes, since the feature does not exist today, it can happen separately.

Sent as another patch:
https://lore.kernel.org/all/20250411-b4-container-of-type-check-v1-1-08262ef67c95@gmail.com/.

Thanks for the suggestion!
Tamir

Re: [PATCH] rust: retain pointer mut-ness in `container_of!`

[Miguel Ojeda]

On Wed, Apr 9, 2025 at 4:43 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> 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>

Applied to rust-next -- thanks everyone!

Cheers,
Miguel