Re: [PATCH 04/10] rust: add bindings for gpio_{in|out} initialization

[Zhao Liu]

On Wed, Jan 29, 2025 at 11:59:04AM +0100, Paolo Bonzini wrote:
> Date: Wed, 29 Jan 2025 11:59:04 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 04/10] rust: add bindings for gpio_{in|out}
>  initialization
> 
> 
> 
> On Sat, Jan 25, 2025 at 1:32 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > +    fn init_gpio_in<F: for<'a> FnCall<(&'a Self::Target, u32, 
> > u32)>>(&self, num_lines: u32, _f: F) {
> > +        unsafe extern "C" fn rust_irq_handler<T, F: for<'a> FnCall<(&'a T, 
> > u32, u32)>>(
> > +            opaque: *mut c_void,
> > +            line: c_int,
> > +            level: c_int,
> > +        ) {
> > +            // SAFETY: the opaque was passed as a reference to `T`
> > +            F::call((unsafe { &*(opaque.cast::<T>()) }, line as u32, level 
> > as u32))
> > +        }
> > +
> > +        let gpio_in_cb: unsafe extern "C" fn(*mut c_void, c_int, c_int) =
> > +            rust_irq_handler::<Self::Target, F>;
> 
> Please add "let _: () = F::ASSERT_IS_SOME;", which is added by the
> qdev_init_clock_in() patch.
> 

Okay.

I would add `assert!(F::is_some());` at the beginning of init_gpio_in().

There's a difference with origianl C version:

In C side, qdev_get_gpio_in() family could accept a NULL handler, but
there's no such case in current QEMU:

* qdev_get_gpio_in
* qdev_init_gpio_in_named
* qdev_init_gpio_in_named_with_opaque

And from code logic view, creating an input GPIO line but doing nothing
on input, sounds also unusual.

So, for simplicity, in the Rust version I make the handler non-optional.