From b61b0092eaf22ef34936516d2e7181bb9cee25ac Mon Sep 17 00:00:00 2001 From: Albin Babu Varghese Date: Tue, 27 May 2025 16:49:28 -0400 Subject: rust: list: replace unwrap() with ? in doctest examples Using `unwrap()` in kernel doctests can cause panics on error and may give newcomers the mistaken impression that panicking is acceptable in kernel code. Replace all `.unwrap()` calls in `kernel::list` examples with `.ok_or(EINVAL)?` so that errors are properly propagated. Suggested-by: Miguel Ojeda Link: https://github.com/Rust-for-Linux/linux/issues/1164 Reviewed-by: Benno Lossin Signed-off-by: Albin Babu Varghese Reviewed-by: Alice Ryhl Reviewed-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250527204928.5117-1-albinbabuvarghese20@gmail.com [ Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'rust/kernel/list.rs') diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index c391c30b80f8..fe58a3920e70 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -82,9 +82,9 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// // [15, 10, 30] /// { /// let mut iter = list.iter(); -/// assert_eq!(iter.next().unwrap().value, 15); -/// assert_eq!(iter.next().unwrap().value, 10); -/// assert_eq!(iter.next().unwrap().value, 30); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 15); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 10); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 30); /// assert!(iter.next().is_none()); /// /// // Verify the length of the list. @@ -93,9 +93,9 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// /// // Pop the items from the list using `pop_back()` and verify the content. /// { -/// assert_eq!(list.pop_back().unwrap().value, 30); -/// assert_eq!(list.pop_back().unwrap().value, 10); -/// assert_eq!(list.pop_back().unwrap().value, 15); +/// assert_eq!(list.pop_back().ok_or(EINVAL)?.value, 30); +/// assert_eq!(list.pop_back().ok_or(EINVAL)?.value, 10); +/// assert_eq!(list.pop_back().ok_or(EINVAL)?.value, 15); /// } /// /// // Insert 3 elements using `push_front()`. @@ -107,9 +107,9 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// // [30, 10, 15] /// { /// let mut iter = list.iter(); -/// assert_eq!(iter.next().unwrap().value, 30); -/// assert_eq!(iter.next().unwrap().value, 10); -/// assert_eq!(iter.next().unwrap().value, 15); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 30); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 10); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 15); /// assert!(iter.next().is_none()); /// /// // Verify the length of the list. @@ -118,8 +118,8 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// /// // Pop the items from the list using `pop_front()` and verify the content. /// { -/// assert_eq!(list.pop_front().unwrap().value, 30); -/// assert_eq!(list.pop_front().unwrap().value, 10); +/// assert_eq!(list.pop_front().ok_or(EINVAL)?.value, 30); +/// assert_eq!(list.pop_front().ok_or(EINVAL)?.value, 10); /// } /// /// // Push `list2` to `list` through `push_all_back()`. @@ -135,9 +135,9 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// // list: [15, 25, 35] /// // list2: [] /// let mut iter = list.iter(); -/// assert_eq!(iter.next().unwrap().value, 15); -/// assert_eq!(iter.next().unwrap().value, 25); -/// assert_eq!(iter.next().unwrap().value, 35); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 15); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 25); +/// assert_eq!(iter.next().ok_or(EINVAL)?.value, 35); /// assert!(iter.next().is_none()); /// assert!(list2.is_empty()); /// } @@ -809,11 +809,11 @@ impl<'a, T: ?Sized + ListItem, const ID: u64> Iterator for Iter<'a, T, ID> { /// merge_sorted(&mut list, list2); /// /// let mut items = list.into_iter(); -/// assert_eq!(items.next().unwrap().value, 10); -/// assert_eq!(items.next().unwrap().value, 11); -/// assert_eq!(items.next().unwrap().value, 12); -/// assert_eq!(items.next().unwrap().value, 13); -/// assert_eq!(items.next().unwrap().value, 14); +/// assert_eq!(items.next().ok_or(EINVAL)?.value, 10); +/// assert_eq!(items.next().ok_or(EINVAL)?.value, 11); +/// assert_eq!(items.next().ok_or(EINVAL)?.value, 12); +/// assert_eq!(items.next().ok_or(EINVAL)?.value, 13); +/// assert_eq!(items.next().ok_or(EINVAL)?.value, 14); /// assert!(items.next().is_none()); /// # Result::<(), Error>::Ok(()) /// ``` -- cgit v1.2.3 From 64fb810bce03a4e2b4d3ecbba04bb97da3536dd8 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 24 Jun 2025 15:27:56 +0000 Subject: rust: types: rename Opaque::raw_get to cast_into In the previous patch we added Opaque::cast_from() that performs the opposite operation to Opaque::raw_get(). For consistency with this naming, rename raw_get() to cast_from(). There are a few other options such as calling cast_from() something closer to raw_get() rather than renaming this method. However, I could not find a great naming scheme that works with raw_get(). The previous version of this patch used from_raw(), but functions of that name typically have a different signature, so that's not a great option. Suggested-by: Danilo Krummrich Signed-off-by: Alice Ryhl Acked-by: Benno Lossin Acked-by: Andreas Hindborg Acked-by: Boqun Feng Reviewed-by: Danilo Krummrich Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250624-opaque-from-raw-v2-2-e4da40bdc59c@google.com [ Removed `HrTimer::raw_get` change. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/configfs.rs | 2 +- rust/kernel/init.rs | 6 +++--- rust/kernel/lib.rs | 4 ++-- rust/kernel/list.rs | 2 +- rust/kernel/list/impl_list_item_mod.rs | 4 ++-- rust/kernel/time/hrtimer.rs | 2 +- rust/kernel/types.rs | 8 ++++---- rust/kernel/workqueue.rs | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) (limited to 'rust/kernel/list.rs') diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs index d287e11e4233..2736b798cdc6 100644 --- a/rust/kernel/configfs.rs +++ b/rust/kernel/configfs.rs @@ -279,7 +279,7 @@ impl Group { // within the `group` field. unsafe impl HasGroup for Group { unsafe fn group(this: *const Self) -> *const bindings::config_group { - Opaque::raw_get( + Opaque::cast_into( // SAFETY: By impl and function safety requirements this field // projection is within bounds of the allocation. unsafe { &raw const (*this).group }, diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index 49a61fa3dee8..75d3d99aed6a 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -100,13 +100,13 @@ //! let foo = addr_of_mut!((*slot).foo); //! //! // Initialize the `foo` -//! bindings::init_foo(Opaque::raw_get(foo)); +//! bindings::init_foo(Opaque::cast_into(foo)); //! //! // Try to enable it. -//! let err = bindings::enable_foo(Opaque::raw_get(foo), flags); +//! let err = bindings::enable_foo(Opaque::cast_into(foo), flags); //! if err != 0 { //! // Enabling has failed, first clean up the foo and then return the error. -//! bindings::destroy_foo(Opaque::raw_get(foo)); +//! bindings::destroy_foo(Opaque::cast_into(foo)); //! return Err(Error::from_errno(err)); //! } //! diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 529ce9074996..f61ac6f81f5d 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -204,11 +204,11 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { /// Produces a pointer to an object from a pointer to one of its fields. /// -/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::raw_get`] or +/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::cast_into`] or /// [`Opaque::cast_from`] to resolve the mismatch. /// /// [`Opaque`]: crate::types::Opaque -/// [`Opaque::raw_get`]: crate::types::Opaque::raw_get +/// [`Opaque::cast_into`]: crate::types::Opaque::cast_into /// [`Opaque::cast_from`]: crate::types::Opaque::cast_from /// /// # Safety diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index fe58a3920e70..7ebb81b2a3d4 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -284,7 +284,7 @@ impl ListLinks { #[inline] unsafe fn fields(me: *mut Self) -> *mut ListLinksFields { // SAFETY: The caller promises that the pointer is valid. - unsafe { Opaque::raw_get(ptr::addr_of!((*me).inner)) } + unsafe { Opaque::cast_into(ptr::addr_of!((*me).inner)) } } /// # Safety diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs index 1f9498c1458f..c1edba0a9501 100644 --- a/rust/kernel/list/impl_list_item_mod.rs +++ b/rust/kernel/list/impl_list_item_mod.rs @@ -209,7 +209,7 @@ macro_rules! impl_list_item { // 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>; - let cell_inner = $crate::types::Opaque::raw_get(self_ptr); + let cell_inner = $crate::types::Opaque::cast_into(self_ptr); // SAFETY: This value is not accessed in any other places than `prepare_to_insert`, // `post_remove`, or `view_value`. By the safety requirements of those methods, @@ -252,7 +252,7 @@ macro_rules! impl_list_item { // 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>; - let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr); + let cell_inner = ::core::cell::UnsafeCell::cast_into(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 // `prepare_to_insert` method may not be called in parallel with `view_value` or diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs index 36e1290cd079..113463e64815 100644 --- a/rust/kernel/time/hrtimer.rs +++ b/rust/kernel/time/hrtimer.rs @@ -148,7 +148,7 @@ impl HrTimer { // SAFETY: The field projection to `timer` does not go out of bounds, // because the caller of this function promises that `this` points to an // allocation of at least the size of `Self`. - unsafe { Opaque::raw_get(core::ptr::addr_of!((*this).timer)) } + unsafe { Opaque::cast_into(core::ptr::addr_of!((*this).timer)) } } /// Cancel an initialized and potentially running timer. diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 9de8e0011b1d..49a0e8e9326b 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -377,7 +377,7 @@ impl Opaque { // initialize the `T`. unsafe { pin_init::pin_init_from_closure::<_, ::core::convert::Infallible>(move |slot| { - init_func(Self::raw_get(slot)); + init_func(Self::cast_into(slot)); Ok(()) }) } @@ -397,7 +397,7 @@ impl Opaque { // SAFETY: We contain a `MaybeUninit`, so it is OK for the `init_func` to not fully // initialize the `T`. unsafe { - pin_init::pin_init_from_closure::<_, E>(move |slot| init_func(Self::raw_get(slot))) + pin_init::pin_init_from_closure::<_, E>(move |slot| init_func(Self::cast_into(slot))) } } @@ -410,11 +410,11 @@ impl Opaque { /// /// This function is useful to get access to the value without creating intermediate /// references. - pub const fn raw_get(this: *const Self) -> *mut T { + pub const fn cast_into(this: *const Self) -> *mut T { UnsafeCell::raw_get(this.cast::>>()).cast::() } - /// The opposite operation of [`Opaque::raw_get`]. + /// The opposite operation of [`Opaque::cast_into`]. pub const fn cast_from(this: *const T) -> *const Self { this.cast() } diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index cce23684af24..c90b7431bbba 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -403,7 +403,7 @@ impl Work { // // A pointer cast would also be ok due to `#[repr(transparent)]`. We use `addr_of!` so that // the compiler does not complain that the `work` field is unused. - unsafe { Opaque::raw_get(core::ptr::addr_of!((*ptr).work)) } + unsafe { Opaque::cast_into(core::ptr::addr_of!((*ptr).work)) } } } -- cgit v1.2.3 From c77f85b347dd506ab6ef047031e75c2d03101187 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 9 Jul 2025 15:31:16 -0400 Subject: rust: list: remove OFFSET constants 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] Tested-by: Alice Ryhl Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250709-list-no-offset-v4-6-a429e75840a9@gmail.com [ Fixed broken intra-doc links. Used the renamed `Opaque::cast_into`. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list.rs | 23 +++--- rust/kernel/list/impl_list_item_mod.rs | 145 ++++++++++++++++----------------- 2 files changed, 81 insertions(+), 87 deletions(-) (limited to 'rust/kernel/list.rs') diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index 7ebb81b2a3d4..44e5219cfcbc 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -57,14 +57,11 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// } /// } /// -/// impl_has_list_links! { -/// impl HasListLinks<0> for BasicItem { self.links } -/// } /// impl_list_arc_safe! { /// impl ListArcSafe<0> for BasicItem { untracked; } /// } /// impl_list_item! { -/// impl ListItem<0> for BasicItem { using ListLinks; } +/// impl ListItem<0> for BasicItem { using ListLinks { self.links }; } /// } /// /// // Create a new empty list. @@ -320,9 +317,6 @@ unsafe impl Send for ListLinksSelfPtr {} unsafe impl Sync for ListLinksSelfPtr {} impl ListLinksSelfPtr { - /// 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 { // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will @@ -337,6 +331,16 @@ impl ListLinksSelfPtr { 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: *const Self) -> *const Opaque<*const T> { + // SAFETY: The caller promises that the pointer is valid. + unsafe { ptr::addr_of!((*me).self_ptr) } + } } impl, const ID: u64> List { @@ -711,14 +715,11 @@ impl<'a, T: ?Sized + ListItem, const ID: u64> Iterator for Iter<'a, T, ID> { /// } /// } /// -/// 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 374b3dd812d0..f4c91832a875 100644 --- a/rust/kernel/list/impl_list_item_mod.rs +++ b/rust/kernel/list/impl_list_item_mod.rs @@ -4,21 +4,19 @@ //! Helpers for implementing list traits safely. -/// Declares that this type has a `ListLinks` field at a fixed offset. +/// Declares that this type has a [`ListLinks`] 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` 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. +/// [`ListLinks`]: crate::list::ListLinks +/// [`ListItem`]: crate::list::ListItem pub unsafe trait HasListLinks { - /// The offset of the `ListLinks` field. - const OFFSET: usize; - /// Returns a pointer to the [`ListLinks`] field. /// /// # Safety @@ -26,14 +24,7 @@ pub unsafe trait HasListLinks { /// The provided pointer must point at a valid struct of type `Self`. /// /// [`ListLinks`]: crate::list::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 crate::list::ListLinks { - // SAFETY: The caller promises that the pointer is valid. The implementer promises that the - // `OFFSET` constant is correct. - unsafe { ptr.cast::().add(Self::OFFSET).cast() } - } + unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks; } /// Implements the [`HasListLinks`] trait for the given type. @@ -46,14 +37,15 @@ 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>)? { + // Statically ensure that `$(.field)*` doesn't follow any pointers. + // + // Cannot be `const` because `$self` may contain generics and E0401 says constants + // "can't use {`Self`,generic parameters} from outer item". + if false { let _: 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. @@ -64,12 +56,16 @@ macro_rules! impl_has_list_links { } pub use impl_has_list_links; -/// Declares that the `ListLinks` field in this struct is inside a `ListLinksSelfPtr`. +/// Declares that the [`ListLinks`] field in this struct is inside a +/// [`ListLinksSelfPtr`]. /// /// # Safety /// -/// The `ListLinks` field of this struct at the offset `HasListLinks::OFFSET` must be -/// inside a `ListLinksSelfPtr`. +/// The [`ListLinks`] field of this struct at [`HasListLinks::raw_get_list_links`] must be +/// inside a [`ListLinksSelfPtr`]. +/// +/// [`ListLinks`]: crate::list::ListLinks +/// [`ListLinksSelfPtr`]: crate::list::ListLinksSelfPtr pub unsafe trait HasSelfPtr where Self: HasListLinks, @@ -89,8 +85,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. @@ -120,16 +114,12 @@ pub use impl_has_list_links_self_ptr; /// links: kernel::list::ListLinks, /// } /// -/// kernel::list::impl_has_list_links! { -/// impl HasListLinks<0> for SimpleListItem { self.links } -/// } -/// /// kernel::list::impl_list_arc_safe! { /// impl ListArcSafe<0> for SimpleListItem { untracked; } /// } /// /// kernel::list::impl_list_item! { -/// impl ListItem<0> for SimpleListItem { using ListLinks; } +/// impl ListItem<0> for SimpleListItem { using ListLinks { self.links }; } /// } /// /// struct ListLinksHolder { @@ -143,16 +133,12 @@ pub use impl_has_list_links_self_ptr; /// links: ListLinksHolder, /// } /// -/// kernel::list::impl_has_list_links! { -/// impl{T, U} HasListLinks<0> for ComplexListItem { self.links.inner } -/// } -/// /// kernel::list::impl_list_arc_safe! { /// impl{T, U} ListArcSafe<0> for ComplexListItem { untracked; } /// } /// /// kernel::list::impl_list_item! { -/// impl{T, U} ListItem<0> for ComplexListItem { using ListLinks; } +/// impl{T, U} ListItem<0> for ComplexListItem { using ListLinks { self.links.inner }; } /// } /// ``` /// @@ -168,12 +154,8 @@ pub use impl_has_list_links_self_ptr; /// impl ListArcSafe<0> for SimpleListItem { untracked; } /// } /// -/// kernel::list::impl_has_list_links_self_ptr! { -/// impl HasSelfPtr for SimpleListItem { self.links } -/// } -/// /// kernel::list::impl_list_item! { -/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr; } +/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr { self.links }; } /// } /// /// struct ListLinksSelfPtrHolder { @@ -191,21 +173,23 @@ pub use impl_has_list_links_self_ptr; /// impl{T, U} ListArcSafe<0> for ComplexListItem { untracked; } /// } /// -/// kernel::list::impl_has_list_links_self_ptr! { -/// impl{T, U} HasSelfPtr> for ComplexListItem { self.links.inner } -/// } -/// /// kernel::list::impl_list_item! { -/// impl{T, U} ListItem<0> for ComplexListItem { using ListLinksSelfPtr; } +/// impl{T, U} ListItem<0> for ComplexListItem { +/// using ListLinksSelfPtr { self.links.inner }; +/// } /// } /// ``` #[macro_export] 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)*})? HasListLinks<$num> for $self { self$(.$field)* } + } + // SAFETY: See GUARANTEES comment on each method. unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self { // GUARANTEES: @@ -221,20 +205,19 @@ macro_rules! impl_list_item { } // 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 = >::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: @@ -251,25 +234,28 @@ macro_rules! impl_list_item { } // 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 = >::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)*})? HasSelfPtr<$self> for $self { self$(.$field)* } + } + // SAFETY: See GUARANTEES comment on each method. unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self { // GUARANTEES: @@ -284,13 +270,15 @@ macro_rules! impl_list_item { // SAFETY: The caller promises that `me` points at a valid value of type `Self`. let links_field = unsafe { >::view_links(me) }; - let spoff = $crate::list::ListLinksSelfPtr::::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>; + let container = $crate::container_of!( + links_field, $crate::list::ListLinksSelfPtr, inner + ); + + // SAFETY: By the same reasoning above, `links_field` is a valid pointer. + let self_ptr = unsafe { + $crate::list::ListLinksSelfPtr::raw_get_self_ptr(container) + }; + let cell_inner = $crate::types::Opaque::cast_into(self_ptr); // SAFETY: This value is not accessed in any other places than `prepare_to_insert`, @@ -331,12 +319,17 @@ macro_rules! impl_list_item { // `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::::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>; - let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr); + let container = $crate::container_of!( + links_field, $crate::list::ListLinksSelfPtr, inner + ); + + // SAFETY: By the same reasoning above, `links_field` is a valid pointer. + let self_ptr = unsafe { + $crate::list::ListLinksSelfPtr::raw_get_self_ptr(container) + }; + + let cell_inner = $crate::types::Opaque::cast_into(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 // `prepare_to_insert` method may not be called in parallel with `view_value` or -- cgit v1.2.3