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

Paolo Bonzini posted 19 patches 4 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>
There is a newer version of this series
[PATCH 04/19] rust/qobject: add basic bindings
Posted by Paolo Bonzini 4 months ago
This is only a basic API, intended to be used by the serde traits.

Co-authored-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/util/wrapper.h          |   7 +
 rust/util/meson.build        |   6 +-
 rust/util/src/lib.rs         |   2 +
 rust/util/src/qobject/mod.rs | 317 +++++++++++++++++++++++++++++++++++
 4 files changed, 330 insertions(+), 2 deletions(-)
 create mode 100644 rust/util/src/qobject/mod.rs

diff --git a/rust/util/wrapper.h b/rust/util/wrapper.h
index b9ed68a01d8..0907dd59142 100644
--- a/rust/util/wrapper.h
+++ b/rust/util/wrapper.h
@@ -30,3 +30,10 @@ typedef enum memory_order {
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
+#include "qobject/qnull.h"
+#include "qobject/qbool.h"
+#include "qobject/qnum.h"
+#include "qobject/qstring.h"
+#include "qobject/qobject.h"
+#include "qobject/qlist.h"
+#include "qobject/qdict.h"
diff --git a/rust/util/meson.build b/rust/util/meson.build
index 8ad344dccbd..ce468ea5227 100644
--- a/rust/util/meson.build
+++ b/rust/util/meson.build
@@ -36,8 +36,10 @@ _util_rs = static_library(
       'src/module.rs',
       'src/timer.rs',
     ],
-    {'.': _util_bindings_inc_rs}
-  ),
+    {'.': _util_bindings_inc_rs,
+    'qobject': [
+      'src/qobject/mod.rs',
+    ]}),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
   dependencies: [anyhow_rs, libc_rs, foreign_rs, glib_sys_rs, common_rs, qom, qemuutil],
diff --git a/rust/util/src/lib.rs b/rust/util/src/lib.rs
index 16c89b95174..fe0128103c8 100644
--- a/rust/util/src/lib.rs
+++ b/rust/util/src/lib.rs
@@ -4,6 +4,8 @@
 pub mod error;
 pub mod log;
 pub mod module;
+#[macro_use]
+pub mod qobject;
 pub mod timer;
 
 pub use error::{Error, Result};
diff --git a/rust/util/src/qobject/mod.rs b/rust/util/src/qobject/mod.rs
new file mode 100644
index 00000000000..9c6f168d6e1
--- /dev/null
+++ b/rust/util/src/qobject/mod.rs
@@ -0,0 +1,317 @@
+//! `QObject` bindings
+//!
+//! This module implements bindings for QEMU's `QObject` data structure.
+//! The bindings integrate with `serde`, which take the role of visitors
+//! in Rust code.
+
+#![deny(clippy::unwrap_used)]
+
+use std::{
+    cell::UnsafeCell,
+    ffi::{c_char, CString},
+    mem::ManuallyDrop,
+    ptr::{addr_of, addr_of_mut},
+    sync::atomic::{AtomicUsize, Ordering},
+};
+
+use common::assert_field_type;
+
+use crate::bindings;
+
+/// 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>);
+
+// 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.
+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.
+    /// The caller cedes its reference to the returned struct.
+    ///
+    /// # Safety
+    ///
+    /// The `QObjectBase` must not be changed from C code while
+    /// the Rust `QObject` lives
+    const unsafe fn from_base(p: *const bindings::QObjectBase_) -> Self {
+        QObject(unsafe { &*p.cast() })
+    }
+
+    /// Construct a [`QObject`] from a C `QObject` pointer.
+    /// The caller cedes its reference to the returned struct.
+    ///
+    /// # Safety
+    ///
+    /// The `QObject` must not be changed from C code while
+    /// the Rust `QObject` lives
+    pub const unsafe fn from_raw(p: *const bindings::QObject) -> Self {
+        QObject(unsafe { &*p.cast() })
+    }
+
+    /// Obtain a raw C pointer from a reference. `self` is consumed
+    /// and the C `QObject` pointer is leaked.
+    pub fn into_raw(self) -> *mut bindings::QObject {
+        let src = ManuallyDrop::new(self);
+        src.0.get()
+    }
+
+    /// Construct a [`QObject`] from a C `QObject` pointer.
+    /// The caller *does not* cede its reference to the returned struct.
+    ///
+    /// # Safety
+    ///
+    /// The `QObjectBase` must not be changed from C code while
+    /// the Rust `QObject` lives
+    unsafe fn cloned_from_base(p: *const bindings::QObjectBase_) -> Self {
+        let orig = unsafe { ManuallyDrop::new(QObject::from_base(p)) };
+        (*orig).clone()
+    }
+
+    /// Construct a [`QObject`] from a C `QObject` pointer.
+    /// The caller *does not* cede its reference to the returned struct.
+    ///
+    /// # Safety
+    ///
+    /// The `QObject` must not be changed from C code while
+    /// the Rust `QObject` lives
+    pub unsafe fn cloned_from_raw(p: *const bindings::QObject) -> Self {
+        let orig = unsafe { ManuallyDrop::new(QObject::from_raw(p)) };
+        (*orig).clone()
+    }
+
+    fn refcnt(&self) -> &AtomicUsize {
+        assert_field_type!(bindings::QObjectBase_, refcnt, usize);
+        let qobj = self.0.get();
+        unsafe { AtomicUsize::from_ptr(addr_of_mut!((*qobj).base.refcnt)) }
+    }
+}
+
+impl From<()> for QObject {
+    fn from(_null: ()) -> Self {
+        unsafe { QObject::cloned_from_base(addr_of!(bindings::qnull_.base)) }
+    }
+}
+
+impl<T> From<Option<T>> for QObject
+where
+    QObject: From<T>,
+{
+    fn from(o: Option<T>) -> Self {
+        o.map_or_else(|| ().into(), Into::into)
+    }
+}
+
+impl From<bool> for QObject {
+    fn from(b: bool) -> Self {
+        let qobj = unsafe { &*bindings::qbool_from_bool(b) };
+        unsafe { QObject::from_base(addr_of!(qobj.base)) }
+    }
+}
+
+macro_rules! from_int {
+    ($t:ty) => {
+        impl From<$t> for QObject {
+            fn from(n: $t) -> Self {
+                let qobj = unsafe { &*bindings::qnum_from_int(n.into()) };
+                unsafe { QObject::from_base(addr_of!(qobj.base)) }
+            }
+        }
+    };
+}
+
+from_int!(i8);
+from_int!(i16);
+from_int!(i32);
+from_int!(i64);
+
+macro_rules! from_uint {
+    ($t:ty) => {
+        impl From<$t> for QObject {
+            fn from(n: $t) -> Self {
+                let qobj = unsafe { &*bindings::qnum_from_uint(n.into()) };
+                unsafe { QObject::from_base(addr_of!(qobj.base)) }
+            }
+        }
+    };
+}
+
+from_uint!(u8);
+from_uint!(u16);
+from_uint!(u32);
+from_uint!(u64);
+
+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);
+from_double!(f64);
+
+impl From<CString> for QObject {
+    fn from(s: CString) -> Self {
+        let qobj = unsafe { &*bindings::qstring_from_str(s.as_ptr()) };
+        unsafe { QObject::from_base(addr_of!(qobj.base)) }
+    }
+}
+
+impl<A> FromIterator<A> for QObject
+where
+    Self: From<A>,
+{
+    fn from_iter<I: IntoIterator<Item = A>>(it: I) -> Self {
+        let qlist = unsafe { &mut *bindings::qlist_new() };
+        for elem in it {
+            let elem: QObject = elem.into();
+            let elem = elem.into_raw();
+            unsafe {
+                bindings::qlist_append_obj(qlist, elem);
+            }
+        }
+        unsafe { QObject::from_base(addr_of!(qlist.base)) }
+    }
+}
+
+impl<A> FromIterator<(CString, A)> for QObject
+where
+    Self: From<A>,
+{
+    fn from_iter<I: IntoIterator<Item = (CString, A)>>(it: I) -> Self {
+        let qdict = unsafe { &mut *bindings::qdict_new() };
+        for (key, val) in it {
+            let val: QObject = val.into();
+            let val = val.into_raw();
+            unsafe {
+                bindings::qdict_put_obj(qdict, key.as_ptr().cast::<c_char>(), val);
+            }
+        }
+        unsafe { QObject::from_base(addr_of!(qdict.base)) }
+    }
+}
+
+impl Clone for QObject {
+    fn clone(&self) -> Self {
+        self.refcnt().fetch_add(1, Ordering::Acquire);
+        QObject(self.0)
+    }
+}
+
+impl Drop for QObject {
+    fn drop(&mut self) {
+        if self.refcnt().fetch_sub(1, Ordering::Release) == 1 {
+            unsafe {
+                bindings::qobject_destroy(self.0.get());
+            }
+        }
+    }
+}
+
+#[allow(unused)]
+macro_rules! match_qobject {
+    (@internal ($qobj:expr) =>
+        $(() => $unit:expr,)?
+        $(bool($boolvar:tt) => $bool:expr,)?
+        $(i64($i64var:tt) => $i64:expr,)?
+        $(u64($u64var:tt) => $u64:expr,)?
+        $(f64($f64var:tt) => $f64:expr,)?
+        $(CStr($cstrvar:tt) => $cstr:expr,)?
+        $(QList($qlistvar:tt) => $qlist:expr,)?
+        $(QDict($qdictvar:tt) => $qdict:expr,)?
+        $(_ => $other:expr,)?
+    ) => {
+        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
+                },)?
+                $crate::bindings::QTYPE_QNUM => {
+                    let qnum__: *mut $crate::bindings::QNum = qobj_.cast();
+                    let qnum__ = unsafe { &*qnum__ };
+                    match qnum__.kind {
+                        $crate::bindings::QNUM_I64 |
+                        $crate::bindings::QNUM_U64 |
+                        $crate::bindings::QNUM_DOUBLE => {}
+                        _ => {
+                            panic!("unreachable");
+                        }
+                    }
+
+                    match qnum__.kind {
+                        $($crate::bindings::QNUM_I64 => break {
+                            let $i64var = unsafe { qnum__.u.i64_ };
+                            $i64
+                        },)?
+                        $($crate::bindings::QNUM_U64 => break {
+                            let $u64var = unsafe { qnum__.u.u64_ };
+                            $u64
+                        },)?
+                        $($crate::bindings::QNUM_DOUBLE => break {
+                            let $f64var = unsafe { qnum__.u.dbl };
+                            $f64
+                        },)?
+                        _ => {}
+                    }
+                },
+                $($crate::bindings::QTYPE_QSTRING => break {
+                    let qstring__: *mut $crate::bindings::QString = qobj_.cast();
+                    let $cstrvar = unsafe { ::core::ffi::CStr::from_ptr((&*qstring__).string) };
+                    $cstr
+                },)?
+                $($crate::bindings::QTYPE_QLIST => break {
+                    let qlist__: *mut $crate::bindings::QList = qobj_.cast();
+                    let $qlistvar = unsafe { &*qlist__ };
+                    $qlist
+                },)?
+                $($crate::bindings::QTYPE_QDICT => break {
+                    let qdict__: *mut $crate::bindings::QDict = qobj_.cast();
+                    let $qdictvar = unsafe { &*qdict__ };
+                    $qdict
+                },)?
+                _ => ()
+            };
+            $(break $other;)?
+            #[allow(unreachable_code)]
+            {
+                panic!("unreachable");
+            }
+        }
+    };
+
+    // first cleanup the syntax a bit, checking that there's at least
+    // one pattern and always adding a trailing comma
+    (($qobj:expr) =>
+        $($type:tt$(($val:tt))? => $code:expr ),+ $(,)?) => {
+            match_qobject!(@internal ($qobj) =>
+                $($type $(($val))? => $code,)+)
+    };
+}
+#[allow(unused_imports)]
+use match_qobject;
-- 
2.51.0


Re: [PATCH 04/19] rust/qobject: add basic bindings
Posted by Markus Armbruster 2 months ago
Learning opportunity for me, I ask for your patience...

Paolo Bonzini <pbonzini@redhat.com> writes:

> This is only a basic API, intended to be used by the serde traits.
>
> Co-authored-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/util/wrapper.h          |   7 +
>  rust/util/meson.build        |   6 +-
>  rust/util/src/lib.rs         |   2 +
>  rust/util/src/qobject/mod.rs | 317 +++++++++++++++++++++++++++++++++++
>  4 files changed, 330 insertions(+), 2 deletions(-)
>  create mode 100644 rust/util/src/qobject/mod.rs
>
> diff --git a/rust/util/wrapper.h b/rust/util/wrapper.h
> index b9ed68a01d8..0907dd59142 100644
> --- a/rust/util/wrapper.h
> +++ b/rust/util/wrapper.h
> @@ -30,3 +30,10 @@ typedef enum memory_order {
>  #include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
> +#include "qobject/qnull.h"
> +#include "qobject/qbool.h"
> +#include "qobject/qnum.h"
> +#include "qobject/qstring.h"
> +#include "qobject/qobject.h"
> +#include "qobject/qlist.h"
> +#include "qobject/qdict.h"
> diff --git a/rust/util/meson.build b/rust/util/meson.build
> index 8ad344dccbd..ce468ea5227 100644
> --- a/rust/util/meson.build
> +++ b/rust/util/meson.build
> @@ -36,8 +36,10 @@ _util_rs = static_library(
>        'src/module.rs',
>        'src/timer.rs',
>      ],
> -    {'.': _util_bindings_inc_rs}
> -  ),
> +    {'.': _util_bindings_inc_rs,
> +    'qobject': [
> +      'src/qobject/mod.rs',
> +    ]}),
>    override_options: ['rust_std=2021', 'build.rust_std=2021'],
>    rust_abi: 'rust',
>    dependencies: [anyhow_rs, libc_rs, foreign_rs, glib_sys_rs, common_rs, qom, qemuutil],
> diff --git a/rust/util/src/lib.rs b/rust/util/src/lib.rs
> index 16c89b95174..fe0128103c8 100644
> --- a/rust/util/src/lib.rs
> +++ b/rust/util/src/lib.rs
> @@ -4,6 +4,8 @@
>  pub mod error;
>  pub mod log;
>  pub mod module;
> +#[macro_use]
> +pub mod qobject;
>  pub mod timer;
>  
>  pub use error::{Error, Result};
> diff --git a/rust/util/src/qobject/mod.rs b/rust/util/src/qobject/mod.rs
> new file mode 100644
> index 00000000000..9c6f168d6e1
> --- /dev/null
> +++ b/rust/util/src/qobject/mod.rs
> @@ -0,0 +1,317 @@
> +//! `QObject` bindings
> +//!
> +//! This module implements bindings for QEMU's `QObject` data structure.
> +//! The bindings integrate with `serde`, which take the role of visitors
> +//! in Rust code.
> +
> +#![deny(clippy::unwrap_used)]
> +
> +use std::{
> +    cell::UnsafeCell,
> +    ffi::{c_char, CString},
> +    mem::ManuallyDrop,
> +    ptr::{addr_of, addr_of_mut},
> +    sync::atomic::{AtomicUsize, Ordering},
> +};
> +
> +use common::assert_field_type;
> +
> +use crate::bindings;
> +
> +/// 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?

> +
> +// 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?

> +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?

> +    /// The caller cedes its reference to the returned struct.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The `QObjectBase` must not be changed from C code while
> +    /// the Rust `QObject` lives
> +    const unsafe fn from_base(p: *const bindings::QObjectBase_) -> Self {
> +        QObject(unsafe { &*p.cast() })
> +    }
> +
> +    /// Construct a [`QObject`] from a C `QObject` pointer.
> +    /// The caller cedes its reference to the returned struct.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The `QObject` must not be changed from C code while
> +    /// the Rust `QObject` lives
> +    pub const unsafe fn from_raw(p: *const bindings::QObject) -> Self {
> +        QObject(unsafe { &*p.cast() })
> +    }
> +
> +    /// 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"?

> +    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?

> +    /// The caller *does not* cede its reference to the returned struct.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The `QObjectBase` must not be changed from C code while
> +    /// the Rust `QObject` lives
> +    unsafe fn cloned_from_base(p: *const bindings::QObjectBase_) -> Self {
> +        let orig = unsafe { ManuallyDrop::new(QObject::from_base(p)) };
> +        (*orig).clone()
> +    }
> +
> +    /// Construct a [`QObject`] from a C `QObject` pointer.
> +    /// The caller *does not* cede its reference to the returned struct.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The `QObject` must not be changed from C code while
> +    /// the Rust `QObject` lives
> +    pub unsafe fn cloned_from_raw(p: *const bindings::QObject) -> Self {
> +        let orig = unsafe { ManuallyDrop::new(QObject::from_raw(p)) };
> +        (*orig).clone()
> +    }
> +
> +    fn refcnt(&self) -> &AtomicUsize {
> +        assert_field_type!(bindings::QObjectBase_, refcnt, usize);
> +        let qobj = self.0.get();
> +        unsafe { AtomicUsize::from_ptr(addr_of_mut!((*qobj).base.refcnt)) }
> +    }
> +}
> +
> +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()?

> +    }
> +}
> +
> +impl<T> From<Option<T>> for QObject
> +where
> +    QObject: From<T>,
> +{
> +    fn from(o: Option<T>) -> Self {
> +        o.map_or_else(|| ().into(), Into::into)
> +    }
> +}
> +
> +impl From<bool> for QObject {
> +    fn from(b: bool) -> Self {
> +        let qobj = unsafe { &*bindings::qbool_from_bool(b) };
> +        unsafe { QObject::from_base(addr_of!(qobj.base)) }
> +    }
> +}
> +
> +macro_rules! from_int {
> +    ($t:ty) => {
> +        impl From<$t> for QObject {
> +            fn from(n: $t) -> Self {
> +                let qobj = unsafe { &*bindings::qnum_from_int(n.into()) };
> +                unsafe { QObject::from_base(addr_of!(qobj.base)) }
> +            }
> +        }
> +    };
> +}
> +
> +from_int!(i8);
> +from_int!(i16);
> +from_int!(i32);
> +from_int!(i64);
> +
> +macro_rules! from_uint {
> +    ($t:ty) => {
> +        impl From<$t> for QObject {
> +            fn from(n: $t) -> Self {
> +                let qobj = unsafe { &*bindings::qnum_from_uint(n.into()) };
> +                unsafe { QObject::from_base(addr_of!(qobj.base)) }
> +            }
> +        }
> +    };
> +}
> +
> +from_uint!(u8);
> +from_uint!(u16);
> +from_uint!(u32);
> +from_uint!(u64);
> +
> +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?

> +from_double!(f64);

Can you briefly explain why we need more than i64, u64, and double?

> +
> +impl From<CString> for QObject {
> +    fn from(s: CString) -> Self {
> +        let qobj = unsafe { &*bindings::qstring_from_str(s.as_ptr()) };
> +        unsafe { QObject::from_base(addr_of!(qobj.base)) }
> +    }
> +}
> +
> +impl<A> FromIterator<A> for QObject
> +where
> +    Self: From<A>,
> +{
> +    fn from_iter<I: IntoIterator<Item = A>>(it: I) -> Self {
> +        let qlist = unsafe { &mut *bindings::qlist_new() };
> +        for elem in it {
> +            let elem: QObject = elem.into();
> +            let elem = elem.into_raw();
> +            unsafe {
> +                bindings::qlist_append_obj(qlist, elem);
> +            }
> +        }
> +        unsafe { QObject::from_base(addr_of!(qlist.base)) }
> +    }
> +}
> +
> +impl<A> FromIterator<(CString, A)> for QObject
> +where
> +    Self: From<A>,
> +{
> +    fn from_iter<I: IntoIterator<Item = (CString, A)>>(it: I) -> Self {
> +        let qdict = unsafe { &mut *bindings::qdict_new() };
> +        for (key, val) in it {
> +            let val: QObject = val.into();
> +            let val = val.into_raw();
> +            unsafe {
> +                bindings::qdict_put_obj(qdict, key.as_ptr().cast::<c_char>(), val);
> +            }
> +        }
> +        unsafe { QObject::from_base(addr_of!(qdict.base)) }
> +    }
> +}
> +
> +impl Clone for QObject {
> +    fn clone(&self) -> Self {
> +        self.refcnt().fetch_add(1, Ordering::Acquire);
> +        QObject(self.0)
> +    }
> +}
> +
> +impl Drop for QObject {
> +    fn drop(&mut self) {
> +        if self.refcnt().fetch_sub(1, Ordering::Release) == 1 {
> +            unsafe {
> +                bindings::qobject_destroy(self.0.get());
> +            }
> +        }
> +    }
> +}
> +

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

> +#[allow(unused)]
> +macro_rules! match_qobject {
> +    (@internal ($qobj:expr) =>
> +        $(() => $unit:expr,)?
> +        $(bool($boolvar:tt) => $bool:expr,)?
> +        $(i64($i64var:tt) => $i64:expr,)?
> +        $(u64($u64var:tt) => $u64:expr,)?
> +        $(f64($f64var:tt) => $f64:expr,)?
> +        $(CStr($cstrvar:tt) => $cstr:expr,)?
> +        $(QList($qlistvar:tt) => $qlist:expr,)?
> +        $(QDict($qdictvar:tt) => $qdict:expr,)?
> +        $(_ => $other:expr,)?
> +    ) => {
> +        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
> +                },)?
> +                $crate::bindings::QTYPE_QNUM => {
> +                    let qnum__: *mut $crate::bindings::QNum = qobj_.cast();
> +                    let qnum__ = unsafe { &*qnum__ };
> +                    match qnum__.kind {
> +                        $crate::bindings::QNUM_I64 |
> +                        $crate::bindings::QNUM_U64 |
> +                        $crate::bindings::QNUM_DOUBLE => {}
> +                        _ => {
> +                            panic!("unreachable");
> +                        }
> +                    }
> +
> +                    match qnum__.kind {
> +                        $($crate::bindings::QNUM_I64 => break {
> +                            let $i64var = unsafe { qnum__.u.i64_ };
> +                            $i64
> +                        },)?
> +                        $($crate::bindings::QNUM_U64 => break {
> +                            let $u64var = unsafe { qnum__.u.u64_ };
> +                            $u64
> +                        },)?
> +                        $($crate::bindings::QNUM_DOUBLE => break {
> +                            let $f64var = unsafe { qnum__.u.dbl };
> +                            $f64
> +                        },)?
> +                        _ => {}
> +                    }
> +                },
> +                $($crate::bindings::QTYPE_QSTRING => break {
> +                    let qstring__: *mut $crate::bindings::QString = qobj_.cast();
> +                    let $cstrvar = unsafe { ::core::ffi::CStr::from_ptr((&*qstring__).string) };
> +                    $cstr
> +                },)?
> +                $($crate::bindings::QTYPE_QLIST => break {
> +                    let qlist__: *mut $crate::bindings::QList = qobj_.cast();
> +                    let $qlistvar = unsafe { &*qlist__ };
> +                    $qlist
> +                },)?
> +                $($crate::bindings::QTYPE_QDICT => break {
> +                    let qdict__: *mut $crate::bindings::QDict = qobj_.cast();
> +                    let $qdictvar = unsafe { &*qdict__ };
> +                    $qdict
> +                },)?
> +                _ => ()
> +            };
> +            $(break $other;)?
> +            #[allow(unreachable_code)]
> +            {
> +                panic!("unreachable");
> +            }
> +        }
> +    };
> +
> +    // first cleanup the syntax a bit, checking that there's at least
> +    // one pattern and always adding a trailing comma
> +    (($qobj:expr) =>
> +        $($type:tt$(($val:tt))? => $code:expr ),+ $(,)?) => {
> +            match_qobject!(@internal ($qobj) =>
> +                $($type $(($val))? => $code,)+)
> +    };
> +}
> +#[allow(unused_imports)]
> +use match_qobject;
Re: [PATCH 04/19] rust/qobject: add basic bindings
Posted by Paolo Bonzini 2 months ago
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
Re: [PATCH 04/19] rust/qobject: add basic bindings
Posted by Markus Armbruster 2 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> 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.

Fair, but we should update comments when we make them wrong :)

> I can however replace uses of from_base with a macro similar to QOBJECT()

Might be cleaner.

>>> +    /// 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`/

Much clearer.

>>> +    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_);
> }

Could we call C qnull() instead?

>>> +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.

Okay.

>> 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

Thanks for your help!