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.
