[PATCH 06/19] rust/qobject: add Serialize implementation

Paolo Bonzini posted 19 patches 3 months, 4 weeks 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 06/19] rust/qobject: add Serialize implementation
Posted by Paolo Bonzini 3 months, 4 weeks ago
This allows QObject to be converted to other formats, for example
JSON via serde_json.

This is not too useful, since QObjects are consumed by
C code or deserialized into structs, but it can be used for testing
and it is part of the full implementation of a serde format.

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/Cargo.lock                    |  1 +
 rust/util/Cargo.toml               |  1 +
 rust/util/meson.build              |  3 +-
 rust/util/src/qobject/mod.rs       |  4 +-
 rust/util/src/qobject/serialize.rs | 59 ++++++++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 rust/util/src/qobject/serialize.rs

diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 11085133490..7c9f85d5728 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -445,6 +445,7 @@ dependencies = [
  "foreign",
  "glib-sys",
  "libc",
+ "serde",
 ]
 
 [[package]]
diff --git a/rust/util/Cargo.toml b/rust/util/Cargo.toml
index 85f91436545..554004816eb 100644
--- a/rust/util/Cargo.toml
+++ b/rust/util/Cargo.toml
@@ -17,6 +17,7 @@ anyhow = { workspace = true }
 foreign = { workspace = true }
 glib-sys = { workspace = true }
 libc = { workspace = true }
+serde = { workspace = true }
 common = { path = "../common" }
 
 [lints]
diff --git a/rust/util/meson.build b/rust/util/meson.build
index ce468ea5227..9fafaf76a37 100644
--- a/rust/util/meson.build
+++ b/rust/util/meson.build
@@ -39,10 +39,11 @@ _util_rs = static_library(
     {'.': _util_bindings_inc_rs,
     'qobject': [
       'src/qobject/mod.rs',
+      'src/qobject/serialize.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],
+  dependencies: [anyhow_rs, libc_rs, foreign_rs, glib_sys_rs, common_rs, serde_rs, qom, qemuutil],
 )
 
 util_rs = declare_dependency(link_with: [_util_rs])
diff --git a/rust/util/src/qobject/mod.rs b/rust/util/src/qobject/mod.rs
index 9c6f168d6e1..0913fabc1ee 100644
--- a/rust/util/src/qobject/mod.rs
+++ b/rust/util/src/qobject/mod.rs
@@ -6,6 +6,8 @@
 
 #![deny(clippy::unwrap_used)]
 
+mod serialize;
+
 use std::{
     cell::UnsafeCell,
     ffi::{c_char, CString},
@@ -230,7 +232,6 @@ fn drop(&mut self) {
     }
 }
 
-#[allow(unused)]
 macro_rules! match_qobject {
     (@internal ($qobj:expr) =>
         $(() => $unit:expr,)?
@@ -313,5 +314,4 @@ macro_rules! match_qobject {
                 $($type $(($val))? => $code,)+)
     };
 }
-#[allow(unused_imports)]
 use match_qobject;
diff --git a/rust/util/src/qobject/serialize.rs b/rust/util/src/qobject/serialize.rs
new file mode 100644
index 00000000000..34ec3847c1d
--- /dev/null
+++ b/rust/util/src/qobject/serialize.rs
@@ -0,0 +1,59 @@
+//! `QObject` serialization
+//!
+//! This module implements the [`Serialize`] trait for `QObject`,
+//! allowing it to be converted to other formats, for example
+//! JSON.
+
+use std::{ffi::CStr, mem::ManuallyDrop, ptr::addr_of};
+
+use serde::ser::{self, Serialize, SerializeMap, SerializeSeq};
+
+use super::{match_qobject, QObject};
+use crate::bindings;
+
+impl Serialize for QObject {
+    #[inline]
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: ::serde::Serializer,
+    {
+        match_qobject! { (self) =>
+            () => serializer.serialize_unit(),
+            bool(b) => serializer.serialize_bool(b),
+            i64(i) => serializer.serialize_i64(i),
+            u64(u) => serializer.serialize_u64(u),
+            f64(f) => serializer.serialize_f64(f),
+            CStr(cstr) => cstr.to_str().map_or_else(
+                |_| Err(ser::Error::custom("invalid UTF-8 in QString")),
+                |s| serializer.serialize_str(s),
+            ),
+            QList(l) => {
+                let mut node_ptr = unsafe { l.head.tqh_first };
+                let mut state = serializer.serialize_seq(None)?;
+                while !node_ptr.is_null() {
+                    let node = unsafe { &*node_ptr };
+                    let elem = unsafe { ManuallyDrop::new(QObject::from_raw(addr_of!(*node.value))) };
+                    state.serialize_element(&*elem)?;
+                    node_ptr = unsafe { node.next.tqe_next };
+                }
+                state.end()
+            },
+            QDict(d) => {
+                let mut state = serializer.serialize_map(Some(d.size))?;
+                let mut e_ptr = unsafe { bindings::qdict_first(d) };
+                while !e_ptr.is_null() {
+                    let e = unsafe { &*e_ptr };
+                    let key = unsafe { CStr::from_ptr(e.key) };
+                    key.to_str().map_or_else(
+                        |_| Err(ser::Error::custom("invalid UTF-8 in key")),
+                        |k| state.serialize_key(k),
+                    )?;
+                    let value = unsafe { ManuallyDrop::new(QObject::from_raw(addr_of!(*e.value))) };
+                    state.serialize_value(&*value)?;
+                    e_ptr = unsafe { bindings::qdict_next(d, e) };
+                }
+                state.end()
+            }
+        }
+    }
+}
-- 
2.51.0


Re: [PATCH 06/19] rust/qobject: add Serialize implementation
Posted by Markus Armbruster 2 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> This allows QObject to be converted to other formats, for example
> JSON via serde_json.
>
> This is not too useful, since QObjects are consumed by
> C code or deserialized into structs, but it can be used for testing
> and it is part of the full implementation of a serde format.
>
> 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/Cargo.lock                    |  1 +
>  rust/util/Cargo.toml               |  1 +
>  rust/util/meson.build              |  3 +-
>  rust/util/src/qobject/mod.rs       |  4 +-
>  rust/util/src/qobject/serialize.rs | 59 ++++++++++++++++++++++++++++++
>  5 files changed, 65 insertions(+), 3 deletions(-)
>  create mode 100644 rust/util/src/qobject/serialize.rs
>
> diff --git a/rust/Cargo.lock b/rust/Cargo.lock
> index 11085133490..7c9f85d5728 100644
> --- a/rust/Cargo.lock
> +++ b/rust/Cargo.lock
> @@ -445,6 +445,7 @@ dependencies = [
>   "foreign",
>   "glib-sys",
>   "libc",
> + "serde",
>  ]
>  
>  [[package]]
> diff --git a/rust/util/Cargo.toml b/rust/util/Cargo.toml
> index 85f91436545..554004816eb 100644
> --- a/rust/util/Cargo.toml
> +++ b/rust/util/Cargo.toml
> @@ -17,6 +17,7 @@ anyhow = { workspace = true }
>  foreign = { workspace = true }
>  glib-sys = { workspace = true }
>  libc = { workspace = true }
> +serde = { workspace = true }
>  common = { path = "../common" }
>  
>  [lints]
> diff --git a/rust/util/meson.build b/rust/util/meson.build
> index ce468ea5227..9fafaf76a37 100644
> --- a/rust/util/meson.build
> +++ b/rust/util/meson.build
> @@ -39,10 +39,11 @@ _util_rs = static_library(
>      {'.': _util_bindings_inc_rs,
>      'qobject': [
>        'src/qobject/mod.rs',
> +      'src/qobject/serialize.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],
> +  dependencies: [anyhow_rs, libc_rs, foreign_rs, glib_sys_rs, common_rs, serde_rs, qom, qemuutil],
>  )
>  
>  util_rs = declare_dependency(link_with: [_util_rs])
> diff --git a/rust/util/src/qobject/mod.rs b/rust/util/src/qobject/mod.rs
> index 9c6f168d6e1..0913fabc1ee 100644
> --- a/rust/util/src/qobject/mod.rs
> +++ b/rust/util/src/qobject/mod.rs
> @@ -6,6 +6,8 @@
>  
>  #![deny(clippy::unwrap_used)]
>  
> +mod serialize;
> +
>  use std::{
>      cell::UnsafeCell,
>      ffi::{c_char, CString},
> @@ -230,7 +232,6 @@ fn drop(&mut self) {
>      }
>  }
>  
> -#[allow(unused)]
>  macro_rules! match_qobject {
>      (@internal ($qobj:expr) =>
>          $(() => $unit:expr,)?
> @@ -313,5 +314,4 @@ macro_rules! match_qobject {
>                  $($type $(($val))? => $code,)+)
>      };
>  }
> -#[allow(unused_imports)]
>  use match_qobject;
> diff --git a/rust/util/src/qobject/serialize.rs b/rust/util/src/qobject/serialize.rs
> new file mode 100644
> index 00000000000..34ec3847c1d
> --- /dev/null
> +++ b/rust/util/src/qobject/serialize.rs
> @@ -0,0 +1,59 @@
> +//! `QObject` serialization
> +//!
> +//! This module implements the [`Serialize`] trait for `QObject`,
> +//! allowing it to be converted to other formats, for example
> +//! JSON.
> +
> +use std::{ffi::CStr, mem::ManuallyDrop, ptr::addr_of};
> +
> +use serde::ser::{self, Serialize, SerializeMap, SerializeSeq};
> +
> +use super::{match_qobject, QObject};
> +use crate::bindings;
> +
> +impl Serialize for QObject {
> +    #[inline]
> +    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
> +    where
> +        S: ::serde::Serializer,
> +    {
> +        match_qobject! { (self) =>
> +            () => serializer.serialize_unit(),
> +            bool(b) => serializer.serialize_bool(b),
> +            i64(i) => serializer.serialize_i64(i),
> +            u64(u) => serializer.serialize_u64(u),
> +            f64(f) => serializer.serialize_f64(f),
> +            CStr(cstr) => cstr.to_str().map_or_else(
> +                |_| Err(ser::Error::custom("invalid UTF-8 in QString")),

Could this be a programming error?  Like flawed input validation?

> +                |s| serializer.serialize_str(s),
> +            ),
> +            QList(l) => {
> +                let mut node_ptr = unsafe { l.head.tqh_first };
> +                let mut state = serializer.serialize_seq(None)?;
> +                while !node_ptr.is_null() {
> +                    let node = unsafe { &*node_ptr };
> +                    let elem = unsafe { ManuallyDrop::new(QObject::from_raw(addr_of!(*node.value))) };
> +                    state.serialize_element(&*elem)?;
> +                    node_ptr = unsafe { node.next.tqe_next };
> +                }
> +                state.end()
> +            },
> +            QDict(d) => {
> +                let mut state = serializer.serialize_map(Some(d.size))?;
> +                let mut e_ptr = unsafe { bindings::qdict_first(d) };
> +                while !e_ptr.is_null() {
> +                    let e = unsafe { &*e_ptr };
> +                    let key = unsafe { CStr::from_ptr(e.key) };
> +                    key.to_str().map_or_else(
> +                        |_| Err(ser::Error::custom("invalid UTF-8 in key")),
> +                        |k| state.serialize_key(k),
> +                    )?;
> +                    let value = unsafe { ManuallyDrop::new(QObject::from_raw(addr_of!(*e.value))) };
> +                    state.serialize_value(&*value)?;
> +                    e_ptr = unsafe { bindings::qdict_next(d, e) };
> +                }
> +                state.end()
> +            }
> +        }
> +    }
> +}
Re: [PATCH 06/19] rust/qobject: add Serialize implementation
Posted by Paolo Bonzini 1 month, 4 weeks ago
On 12/5/25 10:47, Markus Armbruster wrote:
>> +        match_qobject! { (self) =>
>> +            () => serializer.serialize_unit(),
>> +            bool(b) => serializer.serialize_bool(b),
>> +            i64(i) => serializer.serialize_i64(i),
>> +            u64(u) => serializer.serialize_u64(u),
>> +            f64(f) => serializer.serialize_f64(f),
>> +            CStr(cstr) => cstr.to_str().map_or_else(
>> +                |_| Err(ser::Error::custom("invalid UTF-8 in QString")),
> 
> Could this be a programming error?  Like flawed input validation?

Possibly, but given that you have to create a custom error type anyway, 
I'd rather not special case this into the only abort (see the 
"#![deny(clippy::unwrap_used)]" in patch 4).

Paolo
Re: [PATCH 06/19] rust/qobject: add Serialize implementation
Posted by Markus Armbruster 1 month, 3 weeks ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 12/5/25 10:47, Markus Armbruster wrote:
>>> +        match_qobject! { (self) =>
>>> +            () => serializer.serialize_unit(),
>>> +            bool(b) => serializer.serialize_bool(b),
>>> +            i64(i) => serializer.serialize_i64(i),
>>> +            u64(u) => serializer.serialize_u64(u),
>>> +            f64(f) => serializer.serialize_f64(f),
>>> +            CStr(cstr) => cstr.to_str().map_or_else(
>>> +                |_| Err(ser::Error::custom("invalid UTF-8 in QString")),
>> 
>> Could this be a programming error?  Like flawed input validation?

qobject's JSON parser validates, see parse_string().

> Possibly, but given that you have to create a custom error type anyway, 
> I'd rather not special case this into the only abort (see the 
> "#![deny(clippy::unwrap_used)]" in patch 4).

We *should* abort on programming error.

.to_str() fails when its argument is invalid UTF-8.  It returns "an
error with details of where UTF-8 validation failed."[1]

Why are we replacing this error with a custom one?  I guess we add the
clue "in QString".  We also lose the details of where.  Feels like a
questionable trade.

Can we use .expect()?  It panics "if the value is an Err, with a panic
message including the passed message, and the content of the Err."[2]

If we decide this isn't a programming error (because QString may contain
arbitrary zero-terminated byte sequences[3]): can we combine all the
readily available information like .expect() does?


[1] https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.to_str
[2] https://doc.rust-lang.org/std/result/enum.Result.html#method.expect
[3] Feels like a problematic idea to me.