Re: [PATCH v2 08/13] rust: cleanup module_init!, use it from #[derive(Object)]

[Paolo Bonzini]

On Tue, Oct 22, 2024 at 4:12 AM Junjie Mao <junjie....@hotmail.com> wrote:
>
>
> Paolo Bonzini <pbonz...@redhat.com> writes:
>
> > Remove the duplicate code by using the module_init! macro; at the same time,
> > simplify how module_init! is used, by taking inspiration from the 
> > implementation
> > of #[derive(Object)].
> >
> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>
> Reviewed-by: Junjie Mao <junjie....@hotmail.com>
>
> One minor comment below.
>
> > ---
> >  rust/qemu-api-macros/src/lib.rs  | 33 +++-------------
> >  rust/qemu-api/src/definitions.rs | 66 ++++++++++++++------------------
> >  2 files changed, 33 insertions(+), 66 deletions(-)
> >
> <snip>
> > diff --git a/rust/qemu-api/src/definitions.rs 
> > b/rust/qemu-api/src/definitions.rs
> > index 3323a665d92..f180c38bfb2 100644
> > --- a/rust/qemu-api/src/definitions.rs
> > +++ b/rust/qemu-api/src/definitions.rs
> > @@ -29,51 +29,40 @@ pub trait Class {
> >
> >  #[macro_export]
> >  macro_rules! module_init {
> <snip>
> > +    ($type:ident => $body:block) => {
> > +        const _: () = {
> > +            #[used]
> > +            #[cfg_attr(
> > +                not(any(target_vendor = "apple", target_os = "windows")),
> > +                link_section = ".init_array"
> > +            )]
> > +            #[cfg_attr(target_vendor = "apple", link_section = 
> > "__DATA,__mod_init_func")]
> > +            #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
> > +            pub static LOAD_MODULE: extern "C" fn() = {
> > +                extern "C" fn init_fn() {
>
> init_fn() should be unsafe fn according to the signature of
> register_module_init.

I think it *can* be unsafe (which bindgen does by default). It's
always okay to pass a non-unsafe function as unsafe function pointer:

fn f() {
    println!("abc");
}

fn g(pf: unsafe fn()) {
    unsafe {
        pf();
    }
}

fn main() {
    g(f);
}

> Being unsafe fn also makes sense here because it
> is the caller, not init_fn() itself, that is responsible for the safety of
> the unsafe code in the body.

Isn't it the opposite? Since the caller of module_init! is responsible
for the safety, init_fn() itself can be safe. With
unsafe_op_in_unsafe_fn it's not a big deal; but without it, an unsafe
init_fn would hide what is safe and what is not in the argument of
module_init!.

It's also relevant in this respect that init_fn is called *after*
main(), only ctor_fn() is called before main.

Thanks,

Paolo