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

Paolo Bonzini <[email protected]> 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.