Re: [PATCH 04/19] rust/qobject: add basic bindings

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/5/25 10:35, Markus Armbruster wrote:
>> +/// A wrapper for a C `QObject`.
>> +///
>> +/// Because `QObject` is not thread-safe, the safety of these bindings
>> +/// right now hinges on treating them as immutable.  It is part of the
>> +/// contract with the `QObject` constructors that the Rust struct is
>> +/// only built after the contents are stable.
>> +///
>> +/// Only a bare bones API is public; production and consumption of `QObject`
>> +/// generally goes through `serde`.
>> +pub struct QObject(&'static UnsafeCell<bindings::QObject>);
> 
> This defines the Rust QObject.  All it contains is a reference (wrapped
> in UnsafeCell) self.0 to the C QObject.  Correct?

Correct.

>> +
>> +// SAFETY: the QObject API are not thread-safe other than reference counting;
>> +// but the Rust struct is only created once the contents are stable, and
>> +// therefore it obeys the aliased XOR mutable invariant.
> 
> In other words, we promise never to change a QObject while Rust code
> holds a reference, except for the reference counts.  Correct?
> 
> The reference count is the mutable part of an otherwise immutable
> object.  Not mentioned here: it is atomic.  Therefore, concurrent
> updates cannot mess it up.  Nothing depends on its value except
> deallocation when the last reference drops.  I figure that's why the
> exception to "aliased XOR mutable" is fine.  Correct?

Yes, it's one of a few exceptions to "aliased XOR mutable" including:

- Mutex (because only one guy can access it at all anyway)

- RefCell (enforces aliased XOR mutable at run-time, enforces 
single-thread usage at compile-time)

- atomics (a mini mutex)

- Cell (Mutex:RefCell = atomics:Cell, in other words every access is 
independent but also single-thread usage is checked at compile time)

>> +unsafe impl Send for QObject {}
>> +unsafe impl Sync for QObject {}
>> +
>> +// Since a QObject can be a floating-point value, and potentially a NaN,
>> +// do not implement Eq
>> +impl PartialEq for QObject {
>> +    fn eq(&self, other: &Self) -> bool {
>> +        unsafe { bindings::qobject_is_equal(self.0.get(), other.0.get()) }
>> +    }
>> +}
>> +
>> +impl QObject {
>> +    /// Construct a [`QObject`] from a C `QObjectBase` pointer.
> 
> It's spelled QObjectBase_.  More of the same below, not flagging again.
> 
> Comment next to its definition:
> 
>      /* Not for use outside include/qobject/ */
> 
> We're using it outside now.  Do we really need to?

It's because we're defining equivalents of inline functions in 
include/qobject.

I can however replace uses of from_base with a macro similar to QOBJECT()
>> +    /// Obtain a raw C pointer from a reference. `self` is consumed
>> +    /// and the C `QObject` pointer is leaked.
> 
> What exactly do you mean by "leaked"?

s/and the.*/without decreasing the reference count, thus transferring 
the reference to the `*mut bindings::QOjbect`/

>> +    pub fn into_raw(self) -> *mut bindings::QObject {
>> +        let src = ManuallyDrop::new(self);
>> +        src.0.get()
>> +    }
>> +
>> +    /// Construct a [`QObject`] from a C `QObject` pointer.
> 
> Pasto?  Isn't it QObjectBase_ here?

Yes.

>> +impl From<()> for QObject {
>> +    fn from(_null: ()) -> Self {
>> +        unsafe { QObject::cloned_from_base(addr_of!(bindings::qnull_.base)) }
> 
> qnull_ is not meant for use outside qnull.[ch] and its unit test
> check-qnull.c.  Could we use qnull()?

Same as above---it's inline.  The above is a translation of

static inline QNull *qnull(void)
{
     return qobject_ref(&qnull_);
}

>> +macro_rules! from_double {
>> +    ($t:ty) => {
>> +        impl From<$t> for QObject {
>> +            fn from(n: $t) -> Self {
>> +                let qobj = unsafe { &*bindings::qnum_from_double(n.into()) };
>> +                unsafe { QObject::from_base(addr_of!(qobj.base)) }
>> +            }
>> +        }
>> +    };
>> +}
>> +
>> +from_double!(f32);
> 
> Uh, isn't the double in from_double misleading?

It's a reference to the function that it calls (qnum_from_double).  Can 
rename it to impl_from_returning_qnum_double.

>> +from_double!(f64);
> 
> Can you briefly explain why we need more than i64, u64, and double?

Because Rust doesn't do automatic casts.  So it's nicer (and also less 
error prone) if the subsequent patches do not have to always convert to 
u64 or i64.

> Skipping the remainder, it's too much macro magic for poor, ignorant me
> :)

It's not really hard.  The thing to the left of => effectively defines a 
parser. Each thing of the shape $IDENT:RULE matches a piece of Rust 
grammar; expr is expression an tt is token tree (either a single token 
or a parenthesized group).  To access $IDENT that appears within $(...)? 
on the left of => you must have a similar $(...)? on the right, and the 
whole $(...)? on the right will be skipped if the left-side wasn't there.

The macro is used like this:

         match_qobject! { (self) =>
             () => Unexpected::Unit,
             bool(b) => Unexpected::Bool(b),
             i64(n) => Unexpected::Signed(n),
             u64(n) => Unexpected::Unsigned(n),
             f64(n) => Unexpected::Float(n),
             CStr(s) => s.to_str().map_or_else(
                 |_| Unexpected::Other("string with invalid UTF-8"),
                 Unexpected::Str),
             QList(_) => Unexpected::Seq,
             QDict(_) => Unexpected::Map,
         }

And it produces a "switch" on QObject types, where each "case" extracts 
the datum, places it in the variable to the left of "=>" (such as "b" 
for bool), and returns the value on the right of "=>" (such as 
"Unexpected::Bool(b)"):


>> +    ) => {
>> +        loop {
>> +            let qobj_ = $qobj.0.get();
>> +            match unsafe { &* qobj_ }.base.type_ {
>> +                $($crate::bindings::QTYPE_QNULL => break $unit,)?
>> +                $($crate::bindings::QTYPE_QBOOL => break {
>> +                    let qbool__: *mut $crate::bindings::QBool = qobj_.cast();
>> +                    let $boolvar = unsafe { (&*qbool__).value };
>> +                    $bool
>> +                },)?

(The loop/break is just a syntactic convenience---the loop never rolls 
more than once).

Paolo