Support linked lists that can hold many different structs at once. This
is generally done using trait objects. The main challenge is figuring
what the struct is given only a pointer to the ListLinks.
We do this by storing a pointer to the struct next to the ListLinks
field. The container_of operation will then just read that pointer. When
the type is a trait object, that pointer will be a fat pointer whose
metadata is a vtable that tells you what kind of struct it is.
Heterogeneous lists are heavily used by Rust Binder. There are a lot of
so-called todo lists containing various events that need to be delivered
to userspace next time userspace calls into the driver. And there are
quite a few different todo item types: incoming transaction, changes to
refcounts, death notifications, and more.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 47 +++++++++++-
rust/kernel/list/impl_list_item_mod.rs | 129 +++++++++++++++++++++++++++++++++
2 files changed, 175 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index f36e17369382..f61b19c145db 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -7,12 +7,16 @@
use crate::init::PinInit;
use crate::sync::ArcBorrow;
use crate::types::Opaque;
+use core::cell::UnsafeCell;
use core::iter::{DoubleEndedIterator, FusedIterator};
use core::marker::PhantomData;
+use core::mem::MaybeUninit;
use core::ptr;
mod impl_list_item_mod;
-pub use self::impl_list_item_mod::{impl_has_list_links, impl_list_item, HasListLinks};
+pub use self::impl_list_item_mod::{
+ impl_has_list_links, impl_has_list_links_self_ptr, impl_list_item, HasListLinks, HasSelfPtr,
+};
mod arc;
pub use self::arc::{
@@ -181,6 +185,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
}
}
+/// Similar to [`ListLinks`], but also contains a pointer to the full value.
+///
+/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
+#[repr(C)]
+pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
+ /// The `ListLinks` field inside this value.
+ ///
+ /// This is public so that it can be used with `impl_has_list_links!`.
+ pub inner: ListLinks<ID>,
+ self_ptr: UnsafeCell<MaybeUninit<*const T>>,
+}
+
+// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
+unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
+// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
+// it's okay to have immutable access to a ListLinks from several threads at once.
+//
+// Note that `inner` being a public field does not prevent this type from being opaque, since
+// `inner` is a opaque type.
+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
+ // not be constructed in an `Arc` that already has a `ListArc`.
+ Self {
+ inner: ListLinks {
+ inner: Opaque::new(ListLinksFields {
+ prev: ptr::null_mut(),
+ next: ptr::null_mut(),
+ }),
+ },
+ self_ptr: UnsafeCell::new(MaybeUninit::zeroed()),
+ }
+ }
+}
+
impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
/// Creates a new empty list.
pub const fn new() -> Self {
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 9b1947371c63..9335edd8f350 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -67,6 +67,49 @@ 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>`.
+///
+/// # Safety
+///
+/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
+/// inside a `ListLinksSelfPtr<T, ID>`.
+pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
+where
+ Self: HasListLinks<ID>,
+{
+}
+
+/// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
+#[macro_export]
+macro_rules! impl_has_list_links_self_ptr {
+ ($(impl$({$($implarg:tt)*})?
+ HasSelfPtr<$item_type:ty $(, $id:tt)?>
+ for $self:ident $(<$($selfarg: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::HasListLinks$(<$id>)? for
+ $self $(<$($selfarg),*>)?
+ {
+ 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.
+ let ptr: *mut $crate::list::ListLinksSelfPtr<$item_type $(, $id)?> =
+ unsafe { ::core::ptr::addr_of_mut!((*ptr).$field) };
+ ptr.cast()
+ }
+ }
+ )*};
+}
+pub use impl_has_list_links_self_ptr;
+
/// Implements the [`ListItem`] trait for the given type.
///
/// Assumes that the type implements [`HasListLinks`].
@@ -135,5 +178,91 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
}
}
};
+
+ (
+ impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+ using ListLinksSelfPtr;
+ } $($rest:tt)*
+ ) => {
+ // SAFETY: See GUARANTEES comment on each method.
+ unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+ // 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`
+ // must alternate and exclusive access is given up when `post_remove` is called.
+ //
+ // Other invocations of `impl_list_item!` also cannot give out exclusive access to the
+ // same `ListLinks` because you can only implement `ListItem` once for each value of
+ // `ID`, and the `ListLinks` fields only work with the specified `ID`.
+ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ // 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;
+ // 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);
+
+ // 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,
+ // none of these three methods may be called in parallel with this call to
+ // `prepare_to_insert`, so this write will not race with any other access to the
+ // value.
+ unsafe { ::core::ptr::write(cell_inner, me) };
+
+ links_field
+ }
+
+ // GUARANTEES:
+ // * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
+ // returns the return value of `view_links`.
+ // * By the type invariants of `ListLinks`, the `ListLinks` has two null pointers when
+ // this value is not in a list.
+ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
+ unsafe { <Self as HasListLinks<$num>>::raw_get_list_links(me.cast_mut()) }
+ }
+
+ // This function is also used as the implementation of `post_remove`, so the caller
+ // may choose to satisfy the safety requirements of `post_remove` instead of the safety
+ // requirements for `view_value`.
+ //
+ // GUARANTEES:
+ // * This returns the same pointer as the one passed to the most recent call to
+ // `prepare_to_insert` since that call wrote that pointer to this location. The value
+ // is only modified in `prepare_to_insert`, so it has not been modified since the
+ // most recent call.
+ //
+ // GUARANTEES: (when using the `view_value` safety requirements)
+ // * 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(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>;
+ 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
+ // `prepare_to_insert` method may not be called in parallel with `view_value` or
+ // `post_remove`.
+ unsafe { ::core::ptr::read(cell_inner) }
+ }
+
+ // GUARANTEES:
+ // The first guarantee of `view_value` is exactly what `post_remove` guarantees.
+ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
+ // SAFETY: This specific implementation of `view_value` allows the caller to
+ // promise the safety requirements of `post_remove` instead of the safety
+ // requirements for `view_value`.
+ unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
+ }
+ }
+ };
}
pub use impl_list_item;
--
2.45.2.1089.g2a221341d9-googOn 23.07.24 10:22, Alice Ryhl wrote:
> @@ -181,6 +185,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
> }
> }
>
> +/// Similar to [`ListLinks`], but also contains a pointer to the full value.
> +///
> +/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
> +#[repr(C)]
> +pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
> + /// The `ListLinks` field inside this value.
> + ///
> + /// This is public so that it can be used with `impl_has_list_links!`.
> + pub inner: ListLinks<ID>,
> + self_ptr: UnsafeCell<MaybeUninit<*const T>>,
Why do you need `MaybeUninit`?
> +}
> +
> +// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
> +unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
> +// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
> +// it's okay to have immutable access to a ListLinks from several threads at once.
> +//
> +// Note that `inner` being a public field does not prevent this type from being opaque, since
> +// `inner` is a opaque type.
> +unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
[...]
> @@ -135,5 +178,91 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> }
> }
> };
> +
> + (
> + impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
> + using ListLinksSelfPtr;
> + } $($rest:tt)*
> + ) => {
> + // SAFETY: See GUARANTEES comment on each method.
> + unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
> + // 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`
> + // must alternate and exclusive access is given up when `post_remove` is called.
> + //
> + // Other invocations of `impl_list_item!` also cannot give out exclusive access to the
> + // same `ListLinks` because you can only implement `ListItem` once for each value of
> + // `ID`, and the `ListLinks` fields only work with the specified `ID`.
> + unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + // 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;
> + // 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>;
A bit confused why you need to do it this way, can't you just do this?:
let links_self_field = links_field.cast::<$crate::list::ListLinksSelfPtr>();
// SAFETY: ...
let self_ptr = unsafe { ::core::ptr::addr_of_mut!((*links_self_field).self_ptr) };
> + let cell_inner = ::core::cell::UnsafeCell::raw_get(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,
> + // none of these three methods may be called in parallel with this call to
> + // `prepare_to_insert`, so this write will not race with any other access to the
> + // value.
> + unsafe { ::core::ptr::write(cell_inner, me) };
> +
> + links_field
> + }
> +
> + // GUARANTEES:
> + // * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
> + // returns the return value of `view_links`.
> + // * By the type invariants of `ListLinks`, the `ListLinks` has two null pointers when
> + // this value is not in a list.
> + unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
> + unsafe { <Self as HasListLinks<$num>>::raw_get_list_links(me.cast_mut()) }
> + }
> +
> + // This function is also used as the implementation of `post_remove`, so the caller
> + // may choose to satisfy the safety requirements of `post_remove` instead of the safety
> + // requirements for `view_value`.
This almost sounds like a magic card :)
> + //
> + // GUARANTEES:
Can you also put in "()" here that this satisfies the guarantees of
`post_remove`?
> + // * This returns the same pointer as the one passed to the most recent call to
> + // `prepare_to_insert` since that call wrote that pointer to this location. The value
> + // is only modified in `prepare_to_insert`, so it has not been modified since the
> + // most recent call.
> + //
> + // GUARANTEES: (when using the `view_value` safety requirements)
> + // * 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(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>;
> + 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
> + // `prepare_to_insert` method may not be called in parallel with `view_value` or
> + // `post_remove`.
> + unsafe { ::core::ptr::read(cell_inner) }
> + }
> +
> + // GUARANTEES:
> + // The first guarantee of `view_value` is exactly what `post_remove` guarantees.
> + unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> + // SAFETY: This specific implementation of `view_value` allows the caller to
> + // promise the safety requirements of `post_remove` instead of the safety
> + // requirements for `view_value`.
I like this solution better than what you have for `impl_list_item`
"using ListLinks;"
---
Cheers,
Benno
> + unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
> + }
> + }
> + };
> }
> pub use impl_list_item;
>
> --
> 2.45.2.1089.g2a221341d9-goog
>
On Thu, Aug 1, 2024 at 11:24 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On 23.07.24 10:22, Alice Ryhl wrote:
> > @@ -181,6 +185,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
> > }
> > }
> >
> > +/// Similar to [`ListLinks`], but also contains a pointer to the full value.
> > +///
> > +/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
> > +#[repr(C)]
> > +pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
> > + /// The `ListLinks` field inside this value.
> > + ///
> > + /// This is public so that it can be used with `impl_has_list_links!`.
> > + pub inner: ListLinks<ID>,
> > + self_ptr: UnsafeCell<MaybeUninit<*const T>>,
>
> Why do you need `MaybeUninit`?
Right now the constructor initializes it to MaybeUninit::zeroed().
What would you initialize it to without MaybeUninit? Remember that the
vtable pointer in a fat pointer has strict validity requirements.
> > +}
> > +
> > +// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
> > +unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
> > +// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
> > +// it's okay to have immutable access to a ListLinks from several threads at once.
> > +//
> > +// Note that `inner` being a public field does not prevent this type from being opaque, since
> > +// `inner` is a opaque type.
> > +unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
>
> [...]
>
> > @@ -135,5 +178,91 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> > }
> > }
> > };
> > +
> > + (
> > + impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
> > + using ListLinksSelfPtr;
> > + } $($rest:tt)*
> > + ) => {
> > + // SAFETY: See GUARANTEES comment on each method.
> > + unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
> > + // 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`
> > + // must alternate and exclusive access is given up when `post_remove` is called.
> > + //
> > + // Other invocations of `impl_list_item!` also cannot give out exclusive access to the
> > + // same `ListLinks` because you can only implement `ListItem` once for each value of
> > + // `ID`, and the `ListLinks` fields only work with the specified `ID`.
> > + unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> > + // 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;
> > + // 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>;
>
> A bit confused why you need to do it this way, can't you just do this?:
>
> let links_self_field = links_field.cast::<$crate::list::ListLinksSelfPtr>();
> // SAFETY: ...
> let self_ptr = unsafe { ::core::ptr::addr_of_mut!((*links_self_field).self_ptr) };
If nothing else, the field is not public. I can't remember if there
was another reason.
Alice
On 01.08.24 11:38, Alice Ryhl wrote:
> On Thu, Aug 1, 2024 at 11:24 AM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On 23.07.24 10:22, Alice Ryhl wrote:
>>> @@ -181,6 +185,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
>>> }
>>> }
>>>
>>> +/// Similar to [`ListLinks`], but also contains a pointer to the full value.
>>> +///
>>> +/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
>>> +#[repr(C)]
>>> +pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
>>> + /// The `ListLinks` field inside this value.
>>> + ///
>>> + /// This is public so that it can be used with `impl_has_list_links!`.
>>> + pub inner: ListLinks<ID>,
>>> + self_ptr: UnsafeCell<MaybeUninit<*const T>>,
>>
>> Why do you need `MaybeUninit`?
>
> Right now the constructor initializes it to MaybeUninit::zeroed().
> What would you initialize it to without MaybeUninit? Remember that the
> vtable pointer in a fat pointer has strict validity requirements.
Oh... I forgot about that, can you add a comment about that? Also why
not use `Opaque` in that case then?
>>> +}
>>> +
>>> +// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
>>> +unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
>>> +// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
>>> +// it's okay to have immutable access to a ListLinks from several threads at once.
>>> +//
>>> +// Note that `inner` being a public field does not prevent this type from being opaque, since
>>> +// `inner` is a opaque type.
>>> +unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
>>
>> [...]
>>
>>> @@ -135,5 +178,91 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
>>> }
>>> }
>>> };
>>> +
>>> + (
>>> + impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
>>> + using ListLinksSelfPtr;
>>> + } $($rest:tt)*
>>> + ) => {
>>> + // SAFETY: See GUARANTEES comment on each method.
>>> + unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
>>> + // 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`
>>> + // must alternate and exclusive access is given up when `post_remove` is called.
>>> + //
>>> + // Other invocations of `impl_list_item!` also cannot give out exclusive access to the
>>> + // same `ListLinks` because you can only implement `ListItem` once for each value of
>>> + // `ID`, and the `ListLinks` fields only work with the specified `ID`.
>>> + unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
>>> + // 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;
>>> + // 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>;
>>
>> A bit confused why you need to do it this way, can't you just do this?:
>>
>> let links_self_field = links_field.cast::<$crate::list::ListLinksSelfPtr>();
>> // SAFETY: ...
>> let self_ptr = unsafe { ::core::ptr::addr_of_mut!((*links_self_field).self_ptr) };
>
> If nothing else, the field is not public. I can't remember if there
> was another reason.
Oh yeah that's true, then you have to go via the offset.
---
Cheers,
Benno
On Thu, Aug 1, 2024 at 12:50 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On 01.08.24 11:38, Alice Ryhl wrote:
> > On Thu, Aug 1, 2024 at 11:24 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> On 23.07.24 10:22, Alice Ryhl wrote:
> >>> @@ -181,6 +185,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
> >>> }
> >>> }
> >>>
> >>> +/// Similar to [`ListLinks`], but also contains a pointer to the full value.
> >>> +///
> >>> +/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
> >>> +#[repr(C)]
> >>> +pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
> >>> + /// The `ListLinks` field inside this value.
> >>> + ///
> >>> + /// This is public so that it can be used with `impl_has_list_links!`.
> >>> + pub inner: ListLinks<ID>,
> >>> + self_ptr: UnsafeCell<MaybeUninit<*const T>>,
> >>
> >> Why do you need `MaybeUninit`?
> >
> > Right now the constructor initializes it to MaybeUninit::zeroed().
> > What would you initialize it to without MaybeUninit? Remember that the
> > vtable pointer in a fat pointer has strict validity requirements.
>
> Oh... I forgot about that, can you add a comment about that? Also why
> not use `Opaque` in that case then?
It used to just be UnsafeCell, but then I tried it in miri and found
out about the issue and added MaybeUninit. But I can make it use
Opaque instead.
Alice
© 2016 - 2024 Red Hat, Inc.