Re: [PATCH v2 09/13] rust: clean up define_property macro

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/23/24 12:38, Manos Pitsidianakis wrote:
> Hello Paolo,
> 
> On Mon, 21 Oct 2024 19:35, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Use the "struct update" syntax to initialize most of the fields to zero,
>> and simplify the handmade type-checking of $name.
> 
> Note: It wasn't meant for type checking but for making sure the linker
> doesn't strip the symbol (hence the #[used] attribute). These were left
> over when I was debugging linker issues and slapped #[used] everywhere
> but they are not needed in this case indeed.

Well, it does do type checking as well, :) otherwise you end up 
duck-typing on whether $name as as_ptr().  I guess you are okay with the 
change below and the comment:

>> -            name: {
>> -                #[used]
>> -                static _TEMP: &::core::ffi::CStr = $name;
>> -                _TEMP.as_ptr()
>> -            },
>> +            // use associated function syntax for type checking
>> +            name: ::core::ffi::CStr::as_ptr($name),

?

>>              info: $prop,
>>              offset: ::core::mem::offset_of!($state, $field)
>>                  .try_into()
>>                  .expect("Could not fit offset value to type"),
>> -            bitnr: 0,
>> -            bitmask: 0,
>>              set_default: true,
>>              defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval.into() },
>> -            arrayoffset: 0,
>> -            arrayinfo: ::core::ptr::null(),
>> -            arrayfieldsize: 0,
>> -            link_type: ::core::ptr::null(),
>> +            ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() }
> 
> Call it personal taste but I don't like emulating C's zero initializer
> syntax in Rust :) Is it that much trouble to explicitly write down every
> field in a macro, anyway? No strong preference here though.

Rust does make generous use of "..Default::default()", so I think it's 
more idiomatic to use the struct update syntax.  We just cannot use it 
here because it's not const-ified.

I'm okay with switching from Zeroable::ZERO to something like 
ConstDefault::CONST_DEFAULT; it's basically just a rename.  On the other 
hand you do rely on "zero-ness" in PL011State::init()... so I thought I 
was actually following your style. :)

Paolo