On Thu Mar 19, 2026 at 4:24 AM JST, Joel Fernandes wrote:
> On Wed, Mar 18, 2026 at 07:57:14PM +0100, Miguel Ojeda wrote:
>> On Wed, Mar 18, 2026 at 7:31 PM Joel Fernandes <[email protected]> wrote:
>> >
>> > Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
>> > here, instead of // SAFETY:
>> > https://lore.kernel.org/all/caniq72kendyupnwmzmhejytjioeijuk_c-yqjk77dpid89l...@mail.gmail.com/
>>
>> So, to clarify, I suggested it as a temporary thing we could do if we
>> want to use that "fake `unsafe` block in macro matcher" pattern more
>> and more.
>>
>> i.e. if we plan to use the pattern more, then I am happy to ask
>> upstream if it would make sense for Clippy to recognize it (or perhaps
>> it is just a false negative instead of a false positive, given
>> `impl_device_context_deref`), so that we don't need a hacked safety
>> tag (Cc'ing Alejandra).
>>
>> But if we could put it outside, then we wouldn't need any of that.
>> Unsafe macros support could help perhaps here, which I have had it in
>> our wishlist too (https://github.com/Rust-for-Linux/linux/issues/354),
>> but I guess the fake block could still be useful to make only certain
>> macro arms unsafe? (Perhaps Rust could allow `unsafe` just at the
>> start of each arm for that...).
>
> Even if I reworked the macro to be outisde, it doesn't work as below, still
> need the 'disabled' comment on the macro's generate unsafe { } block below.
>
> If we don't want the SAFETY*: hack, we could do the following.
>
> Perhaps, we can file the github bug and also do the below. Once the
> github bug is fixed, we could remove the 'disable lint' below.
>
> Thoughts?
>
> ---8<-----------------------
>
> diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
> index 495497f0405e..dfa2e1490202 100644
> --- a/rust/kernel/interop/list.rs
> +++ b/rust/kernel/interop/list.rs
> @@ -73,7 +73,7 @@
> //!
> //!
> //! // Create typed [`CList`] from sentinel head.
> -//! // SAFETY*: `head` is valid and initialized, items are `SampleItemC` with
> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
> //! // embedded `link` field, and `Item` is `#[repr(transparent)]` over
> `SampleItemC`.
> //! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
> //!
> @@ -328,17 +328,19 @@ impl<'a, T, const OFFSET: usize> FusedIterator for
> CListIter<'a, T, OFFSET> {}
> /// Refer to the examples in the [`crate::interop::list`] module
> documentation.
> #[macro_export]
> macro_rules! clist_create {
> - (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> + (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => (
> + // SAFETY: disable lint.
> + unsafe { {{
> // Compile-time check that field path is a `list_head`.
> let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
> |p| {
> // SAFETY: `p` is a valid pointer to `$c_type`.
> - unsafe { &raw const (*p).$($field).+ }
> + &raw const (*p).$($field).+
> };
>
> // Calculate offset and create `CList`.
> const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> // SAFETY: The caller of this macro is responsible for ensuring
> safety.
> - unsafe { $crate::interop::list::CList::<$rust_type,
> OFFSET>::from_raw($head) }
> - }};
> + $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head)
> + } }});
> }
> pub use clist_create;
I think I like this, it preserves the expected use of `SAFETY:` without
that confusing `*`. The unsafe blocks is a bit larger that it should, be
we are in a controlled environment.
Even after using the `SAFETY*:` I was still getting errors because the
in-macro SAFETY comment wasn't at the right place:
warning: unsafe block missing a safety comment
--> ../rust/kernel/interop/list.rs:335:17
|
335 | |p| unsafe { &raw const (*p).$($field).+ };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
::: ../rust/kernel/gpu/buddy.rs:527:21
|
527 | let clist = clist_create!(unsafe {
| _____________________-
528 | | head,
529 | | Block,
530 | | bindings::gpu_buddy_block,
531 | | __bindgen_anon_1.link
532 | | });
| |__________- in this macro invocation
The diff below fixes that, but I believe your proposal should as well on
top of letting callers use the expected SAFETY statement.
--- a/rust/kernel/interop/list.rs
+++ b/rust/kernel/interop/list.rs
@@ -330,8 +330,8 @@ impl<'a, T, const OFFSET: usize> FusedIterator for
CListIter<'a, T, OFFSET> {}
macro_rules! clist_create {
(unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
// Compile-time check that field path is a `list_head`.
- // SAFETY: `p` is a valid pointer to `$c_type`.
let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
+ // SAFETY: `p` is a valid pointer to `$c_type`.
|p| unsafe { &raw const (*p).$($field).+ };
// Calculate offset and create `CList`.
