Re: [PATCH v3] rust: check type of `$ptr` in `container_of!`

[John Hubbard <jhubbard@nvidia.com>]

On 4/28/25 2:40 AM, Alice Ryhl wrote:
> On Sun, Apr 27, 2025 at 03:59:48PM -0700, John Hubbard wrote:
>> On 4/23/25 10:40 AM, Tamir Duberstein wrote:
>> ...
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index 1df11156302a..d14ed86efb68 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -198,9 +198,15 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>>>   /// ```
>>>   #[macro_export]
>>>   macro_rules! container_of {
>>> -    ($ptr:expr, $type:ty, $($f:tt)*) => {{
>>> -        let offset: usize = ::core::mem::offset_of!($type, $($f)*);
>>> -        $ptr.byte_sub(offset).cast::<$type>()
>>> +    ($field_ptr:expr, $Container:ty, $($fields:tt)*) => {{
>>> +        let offset: usize = ::core::mem::offset_of!($Container, $($fields)*);
>>> +        let field_ptr = $field_ptr;
>>> +        let container_ptr = field_ptr.byte_sub(offset).cast::<$Container>();
>>> +        if false {
>>
>> This jumped out at me. It's something that I'd like to recommend NOT
>> doing, here or anywhere else, because:
>>
>>     a) Anything of the form "if false" will get removed by any compiler
>>        worthy of the name, especially in kernel builds.
> 
> The `if false` branch is used to trigger a compilation failure when the
> macro is used incorrectly. The intent is that the compiler should
> optimize it out. I don't think there's anything wrong with that pattern.

OK...probably best to either encapsulate that, or at least comment
it. I'm accustomed to seeing that pattern in cases where people
expected the code to *not* get optimized out, so it triggers me. :)

thanks,
-- 
John Hubbard