On 12/4/25 15:18, Paolo Bonzini wrote:
On 12/4/25 15:15, Zhao Liu wrote:
diff --git a/rust/util/src/qobject/mod.rs b/rust/util/src/qobject/mod.rs
new file mode 100644
index 00000000000..f43d87a3b66
--- /dev/null
+++ b/rust/util/src/qobject/mod.rs
@@ -0,0 +1,309 @@
+//! `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)]
Are there are any specific considerations for this lint rule?
It ensures that all conversion errors (e.g. from try_into()) are
propagated.
+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".
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>.
These 2 methods are the clone, but it seems they're actually similar to
Owned<>, i.e., increase refcnt when Rust side wants to "own" or ensure
to use this safely.
However, there indeed isn't a non-object version of Owned for now, so I
think this kind of clone should be okay. Hmm, for long-term, is it
valuable to consider a more generic Owned<>?
Linux has ARef<> I think. But it seems overkill for now.
+impl<A> FromIterator<A> for QObject
+where
+ Self: From<A>,
+{
+ fn from_iter<I: IntoIterator<Item = A>>(it: I) -> Self {
nit: maybe the name "iter" is better than "it"?
Sure.
+ 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
+ },)?
+ _ => {}
I think this doesn't handle a special case: qobj's QNUM type doesn't
macth the value type, for example, qobj is QNUM_U64 but value is i64.
Though external code won't use this macro, I think panic may be better
than {}?> Thurther, there're 2 match blocks. Could we merge them as one?
Just one thing: it's not returning {}, it's returning the value
expressed in $(_ => $other:expr,)?. So I think it's better to keep it
this way.
Paolo
