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 <pbonz...@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....@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.
