Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011

[Zhao Liu]

On Mon, Dec 09, 2024 at 01:37:05PM +0100, Paolo Bonzini wrote:
> Date: Mon,  9 Dec 2024 13:37:05 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of
>  pl011
> X-Mailer: git-send-email 2.47.1
> 
> Allow the ObjectImpl trait to expose Rust functions that avoid raw
> pointers (though INSTANCE_INIT for example is still unsafe).
> ObjectImpl::TYPE_INFO adds thunks around the functions in
> ObjectImpl.
> 
> While at it, document `TypeInfo`.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs | 40 +++++++--------------
>  rust/qemu-api/src/definitions.rs | 61 +++++++++++++++++++++++++++++---
>  2 files changed, 69 insertions(+), 32 deletions(-)
> 
> diff --git a/rust/hw/char/pl011/src/device.rs 
> b/rust/hw/char/pl011/src/device.rs
> index 56403c36609..b9f8fb134b5 100644
> --- a/rust/hw/char/pl011/src/device.rs
> +++ b/rust/hw/char/pl011/src/device.rs
> @@ -110,7 +110,7 @@ impl ObjectImpl for PL011State {
>      type Class = PL011Class;
>      const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
>      const PARENT_TYPE_NAME: Option<&'static CStr> = 
> Some(TYPE_SYS_BUS_DEVICE);
> -    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = 
> Some(pl011_init);
> +    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);

No need to keep `unsafe` here?

> +///
> +/// - the struct must be `#[repr(C)]`
> +///
> +/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is
> +///   automatic if the class is defined via `ObjectImpl`).
> +///
> +/// - the first field of the struct must be of the instance struct 
> corresponding
> +///   to the superclass declared as `PARENT_TYPE_NAME`
>  pub trait ObjectImpl: ClassInitImpl + Sized {
> +    /// The QOM class object corresponding to this struct.  Not used yet.
>      type Class;
> +
> +    /// The name of the type, which can be passed to `object_new()` to
> +    /// generate an instance of this type.
>      const TYPE_NAME: &'static CStr;
> +
> +    /// The parent of the type.  This should match the first field of
> +    /// the struct that implements `ObjectImpl`:
>      const PARENT_TYPE_NAME: Option<&'static CStr>;
> +
> +    /// Whether the object can be instantiated
>      const ABSTRACT: bool = false;
> -    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = 
> None;
> -    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> 
> = None;
> +
> +    /// Function that is called to initialize an object.  The parent class 
> will
> +    /// have already been initialized so the type is only responsible for
> +    /// initializing its own members.
> +    ///
> +    /// FIXME: The argument is not really a valid reference. `&mut
> +    /// MaybeUninit<Self>` would be a better description.
> +    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = None;

And here.

> +    /// Function that is called to finish initialization of an object, once
> +    /// `INSTANCE_INIT` functions have been called.
> +    const INSTANCE_POST_INIT: Option<fn(&mut Self)> = None;
>  

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>