Re: [PATCH 02/14] rust: add basic QObject bindings

Zhao Liu <[email protected]>
Newsgroups org.nongnu.qemu-rust,org.nongnu.qemu-devel
Message-ID <[email protected]>
> > > +pub struct QObject(&'static UnsafeCell<bindings::QObject>);
> > 
> > It seems Opaque<> feels more natural than UnsafeCell<>.
> > 
> > Opaque::from_raw() requires *mut T, but QObject::from_raw() and
> > QObject::clone_from_raw() mainly play with C bindings which usually use
> > *mut pointer. So it seems unnecessary to convert *mut to *const in the
> > middle.
> > 
> > And furthermore, I think QObject(Opaque<bindings::QObject>) is better
> > than QObject(&'static Opaque<bindings::QObject>). From a semantic view,
> > C's QObject is a struct, while Rust's QObject is a reference, which seems
> > somewhat mismatched.
> > 
> > I'm not sure yet if there may be gaps when remove &'static, but it
> > looks like using &'static Opaque<> instead of &'static UnsafeCell<> is
> > Okay in code?
> 
> I am using UnsafeCell because the QObject here is always valid, i.e.
> MaybeUninit is explicitly not necessary.  Opaque explicitly allows it to be
> invalid, here instead the API is "create via C code or FFI and only then
> create the QObject".

Just want to clarify the use for Opaque, we've already used Opaque<>::from_raw()
for the case "create via C code or FFI and only then create the" something? In
DeviceState::init_clock_[in|out](), and ObjectClassMethods::new().

But I also agree, for this case MaybeUninit is not necessary. And PhantomPinned
is simply replaced by &'static.

> However, while it is possible to use Opaque<> instead of UnsafeCell<>, it is
> not possible to make this a simple wrapper because QObject is unmovable and
> reference counted.  That is, QObject is the equivalent of (for example)
> Owned<DeviceState>.

Got it, if Rust owns data by QObject(Opaque<>) but without Owned<>, this
can't work since it's no way to handle refcnt.

So &'static UnsafeCell<> is the simplest way to meet both unmovable &
refcnt rquirements.

I'm good with this design. Thanks!

Regards,
Zhao
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.