diff mbox series

[1/6] rust: add `container_of!` macro

Message ID 20240205-b4-rbtree-v1-1-995e3eee38c0@google.com
State New
Headers
Series Red-black tree abstraction needed by Rust Binder |

Commit Message

Matt Gilbride Feb. 5, 2024, 3:50 p.m. UTC 
  From: Wedson Almeida Filho <wedsonaf@gmail.com>

This macro is used to obtain a pointer to an entire struct
when given a pointer to a field in that struct.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Matt Gilbride <mattgilbride@google.com>
---
 rust/kernel/lib.rs | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Alice Ryhl Feb. 9, 2024, 1:10 p.m. UTC | #1 
On Mon, Feb 5, 2024 at 4:50 PM <mattgilbride@google.com> wrote:
>
> From: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> This macro is used to obtain a pointer to an entire struct
> when given a pointer to a field in that struct.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Matt Gilbride <mattgilbride@google.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Trevor Gross Feb. 10, 2024, 8:05 a.m. UTC | #2 
On Mon, Feb 5, 2024 at 9:50 AM <mattgilbride@google.com> wrote:
>
> From: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> This macro is used to obtain a pointer to an entire struct
> when given a pointer to a field in that struct.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Matt Gilbride <mattgilbride@google.com>
> ---
>  rust/kernel/lib.rs | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7ac39874aeac..c7963efd1318 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -102,3 +102,35 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>      // SAFETY: FFI call.
>      unsafe { bindings::BUG() };
>  }
> +
> +/// Produces a pointer to an object from a pointer to one of its fields.

It is in the examples but a note would be good to make it obvious:

    ///
    /// This macro must be called from within an `unsafe { }` block.

> +/// # Safety
> +///
> +/// The pointer passed to this macro, and the pointer returned by this macro, must both be in
> +/// bounds of the same allocation.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::container_of;
> +/// struct Test {
> +///     a: u64,
> +///     b: u32,
> +/// }
> +///
> +/// let test = Test { a: 10, b: 20 };
> +/// let b_ptr = &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) };
> +/// assert!(core::ptr::eq(&test, test_alias));
> +/// ```
> +#[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)*);

`offset_of` will be stable in 1.77 BUT only for a single field [1]. I
don't know if there are other blockers in the kernel already, but if
this could be changed to call `offset_of!` recursively then  it will
work with the stable version.

We might want an `offset_of_many!(a, b, c)` macro somewhere if there
are other places that need this nesting.

[1]: https://github.com/rust-lang/rust/pull/118799

> +        ptr.sub(offset) as *const $type

Instead of casting to and from `u8`, you should be able to use `byte_sub`

> +    }}
> +}

- Trevor
Alice Ryhl Feb. 12, 2024, 9:56 a.m. UTC | #3 
On Sat, Feb 10, 2024 at 9:05 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> On Mon, Feb 5, 2024 at 9:50 AM <mattgilbride@google.com> wrote:
> > +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)*);
>
> `offset_of` will be stable in 1.77 BUT only for a single field [1]. I
> don't know if there are other blockers in the kernel already, but if
> this could be changed to call `offset_of!` recursively then  it will
> work with the stable version.
>
> We might want an `offset_of_many!(a, b, c)` macro somewhere if there
> are other places that need this nesting.
>
> [1]: https://github.com/rust-lang/rust/pull/118799

Rust Binder *does* need that in one place. Creating a nested
offset_of! is kind of tricky, because you have to fail to compile when
you're following a pointer with the Deref trait. I haven't figured out
how to do that yet.

> > +        ptr.sub(offset) as *const $type
>
> Instead of casting to and from `u8`, you should be able to use `byte_sub`

Casting to and from u8 also has the side-effect of making it not work
when the target type is !Sized. Not allowing this might be desirable.

Alice
diff mbox series

Patch

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7ac39874aeac..c7963efd1318 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -102,3 +102,35 @@  fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
     // SAFETY: FFI call.
     unsafe { bindings::BUG() };
 }
+
+/// Produces a pointer to an object from a pointer to one of its fields.
+///
+/// # Safety
+///
+/// The pointer passed to this macro, and the pointer returned by this macro, must both be in
+/// bounds of the same allocation.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::container_of;
+/// struct Test {
+///     a: u64,
+///     b: u32,
+/// }
+///
+/// let test = Test { a: 10, b: 20 };
+/// let b_ptr = &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) };
+/// assert!(core::ptr::eq(&test, test_alias));
+/// ```
+#[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
+    }}
+}