> > > - 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?
>
> Right now instance_init is called with only the parent initialized, and the
> remaining memory zeroed; its purpose is to prepare things for
> instance_post_init which can then be safe (it's also kind of wrong for
> instance_post_init to receive a &mut Self, because instance_init will create
> other pointers to the object, for example in a MemoryRegion's "parent"
> field).
Thank you for explanation. It makes a lot of sense.
> The right thing to do would be to have an argument of type &mut
> MaybeUninit<Self>. Then the caller would do something like
>
> let maybe_uninit = obj as *mut MaybeUninit<Self>;
> unsafe {
> Self::INSTANCE_INIT(&mut *maybe_uninit);
> maybe_uninit.assume_init_mut();
> }
>
> Note however that INSTANCE_INIT would still be unsafe, because its safety
> promise is that it prepares things for the caller's assume_init_mut().
Yes, I feel that this approach more clearly explains the purpose of QOM
init.
And since we are talking about the safety of INSTANCE_INIT, I think we
should add some safety guidelines here, like:
* Proper Initialization of pointers and references
* Explicit initialization of Non-Zero Fields
* In placed memory region is correctly initialized.
(Or do you have any additional or clearer guidelines?)
This could be the reference when adding SAFETY comment for the device's
own `unsafe` init(). :-)
And this is also a good explanation to distinguish between initialization
in init() and realize(). For example, HPET attempts to initialize the
timer array in realize().
> The way that this will become safe is to use the pinned_init crate from
> Linux: instance_init returns the initialization as an "impl PinInit<Self>",
Then do we need to place OBJECT in some suitable memory location (Box or
something) for Rust implementation?
> and then instance_post_init can run with a &self. Until then, however,
> instance_init has to remain unsafe.
Thanks for sharing! It's a very reasonable direction.
Regards,
Zhao
