Messages in this thread | | | From | Andreas Hindborg <> | Subject | Re: [PATCH v1 5/7] rust: workqueue: add helper for defining work_struct fields | Date | Wed, 31 May 2023 12:18:47 +0200 |
| |
Alice Ryhl <aliceryhl@google.com> writes:
> Andreas Hindborg <nmi@metaspace.dk> writes: >> Alice Ryhl <aliceryhl@google.com> writes: >>> +/// Used to safely implement the [`HasWork<T>`] trait. >>> +/// >>> +/// # Examples >>> +/// >>> +/// ``` >>> +/// use kernel::sync::Arc; >>> +/// >>> +/// struct MyStruct { >>> +/// work_field: Work<Arc<MyStruct>>, >>> +/// } >>> +/// >>> +/// impl_has_work! { >>> +/// impl HasWork<Arc<MyStruct>> for MyStruct { self.work_field } >>> +/// } >>> +/// ``` >>> +/// >>> +/// [`HasWork<T>`]: HasWork >>> +#[macro_export] >>> +macro_rules! impl_has_work { >>> + ($(impl$(<$($implarg:ident),*>)? >>> + HasWork<$work_type:ty> >>> + for $self:ident $(<$($selfarg:ident),*>)? >>> + { self.$field:ident } >>> + )*) => {$( >>> + // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right >>> + // type. >>> + unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type> for $self $(<$($selfarg),*>)? { >>> + const OFFSET: usize = $crate::offset_of!(Self, $field) as usize; >>> + >>> + #[inline] >>> + unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_type> { >>> + // SAFETY: The caller promises that the pointer is not dangling. >>> + unsafe { >>> + ::core::ptr::addr_of_mut!((*ptr).$field) >>> + } >>> + } >> >> What is the reason for overriding the default implementation of `raw_get_work()`? >> >> BR Andreas > > That's how the macro checks that the field actually has the type you > claim it has. If you lie about the type, then `raw_get_work` will not > compile. (See the safety comment on the impl block.)
Got it 👍
I was thinking we could do the type check without redefining the method, but that blows up complexity wise fast, since we need a trait to do it to support `Self` in `$work_type`. It strikes me as a bit of a hack to overwrite an otherwise fine implementation, but I guess it is the least complex way.
Also I am a bit annoyed that we need to state the `$work_type` type at all, since it is available in `work_field`. But I can see no way around that.
BR Andreas
| |