On Mon, Jan 27, 2025 at 8:38 AM Zhao Liu <zhao1....@intel.com> wrote:
>
> > +impl<T: ObjectType> Owned<T> {
> > + /// Convert a raw C pointer into an owned reference to the QOM
> > + /// object it points to. The object's reference count will be
> > + /// decreased when the `Owned` is dropped.
> > + ///
> > + /// # Panics
> > + ///
> > + /// Panics if `ptr` is NULL.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must indeed own a reference to the QOM object.
> > + /// The object must not be embedded in another unless the outer
> > + /// object is guaranteed to have a longer lifetime.
> > + ///
> > + /// A raw pointer obtained via [`Owned::into_raw()`] can always be
> > passed
> > + /// back to `from_raw()` (assuming the original `Owned` was valid!),
> > + /// since the owned reference remains there between the calls to
> > + /// `into_raw()` and `from_raw()`.
> > + #[allow(clippy::missing_const_for_fn)]
> > + pub unsafe fn from_raw(ptr: *const T) -> Self {
> > + // SAFETY NOTE: while NonNull requires a mutable pointer, only
> > + // Deref is implemented so the pointer passed to from_raw
> > + // remains const
> > + Owned(NonNull::new(ptr as *mut T).unwrap())
> > + }
>
> ...
>
> > + /// Increase the reference count of a QOM object and return
> > + /// a new owned reference to it.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The object must not be embedded in another, unless the outer
> > + /// object is guaranteed to have a longer lifetime.
> > + pub unsafe fn from(obj: &T) -> Self {
> > + unsafe {
> > + object_ref(obj.as_object_mut_ptr().cast::<c_void>());
> > +
> > + // SAFETY NOTE: while NonNull requires a mutable pointer, only
> > + // Deref is implemented so the reference passed to from_raw
> > + // remains shared
> > + Owned(NonNull::new_unchecked(obj.as_mut_ptr()))
> > + }
> > + }
> > +}
> > +
>
> About the difference between from_raw() and from(), I understand if the
> C side also holds a pointer, the Rust side must increase the reference
> count (using Owned::from), and If the C side does not have any other
> pointers, Rust can directly use Owned::from_raw. Am I right?
Pretty much - more precisely you use Object::from_raw 1) if the C side
gifts a reference 2) if you got the pointer from Owned::into_raw. The
second case is similar to Arc::from_raw, which expects that you got a
reference from Arc::into_raw. The first is the more common case.
>
> * The use of from():
>
> let clk = bindings::qdev_init_clock_in(...)
> Owned::from(&*clk)
In this case the C side wants to manage the reference that
qdev_init_clock_in() returns; it is dropped in
qdev_finalize_clocklist(). So Rust code needs to increase the
refcount.
> * The use of from_raw():
>
> fn new() -> Owned<Self> {
> assert!(bql_locked());
> // SAFETY: the object created by object_new is allocated on
> // the heap and has a reference count of 1
> unsafe {
> let obj = &*object_new(Self::TYPE_NAME.as_ptr());
> Owned::from_raw(obj.unsafe_cast::<Self>())
> }
> }
In this case the C side lets the caller manage the (only) reference
when object_new returns, so you must not increase the refcount.
Owned::from() is slightly less efficient, though that almost never
matters. If it does you can use ManuallyDrop::new(Owned::from_raw(p)).
> Comparing with these 2 use cases, I find the difference is
> qdev_init_clock_in() creates a pointer in qdev_init_clocklist().
That is related, but more precisely the difference is that
qdev_init_clock_in() wants to unref that pointer later.
> Then the comment "the clock is heap allocated and does not have
> a reference" sounds like a conflict. I'm sure I'm missing something. :-(
Changed:
// SAFETY: the clock is heap allocated, but qdev_init_clock_in()
// does not gift the reference to its caller; so use Owned::from to
// add one. the callback is disabled automatically when the clock
// is unparented, which happens before the device is finalized.
Thanks for the review!
Paolo
