On 6/4/25 07:01, Markus Armbruster wrote:
This is what your FOO_or_propagate() functions are for.
The rule glosses over a subtle detail: the difference between
error_setg() and error_propagate() isn't just create a new error vs. use
an existing one, namely error_setg() makes the precondition violation
mentioned above a programming error, whereas error_propagate() does not,
it instead *ignores* the error it's supposed to propagate.
I consider this difference a design mistake. Note that GError avoids
this mistake: g_error_propagate() requieres the destination to NULL or
point to NULL. We deviated from GError, because we thought we were
smarter. We weren't.
Mostly harmless in practice, as behavior is identical for callers that
satisfy the preconditions.
[...]
So here's the bottom line. We want a Rust function to use C Error
according to its written rules. Due to a design mistake, C functions
can behave in two different ways when their caller violates a certain
precondition, depending on how the function transmits the error to the
caller. For Rust functions, we can
* Always behave the more common way, i.e. like a C function using
error_setg() to transmit.
* Always behave the less common way, i.e. like a C function using
error_propagate() to transmit.
* Sometimes one way, sometimes the other way.
This is actually in order of decreasing personal preference. But what
do *you* think?
I agree that there are arguments for both. The reason to use
error_setg() is that, even though these functions "propagate" a
qemu_api::Error into a C Error**, the error is born in the Rust callback
and therefore there is no error_setg() anywhere that could check for
non-NULL abort(). There is a bigger risk of triggering
error_propagate()'s weird behavior.
The reason to use error_propagate() is that these functions do look a
lot more like error_propagate() than error_setg(). I'm undecided. I
think I'll keep the error_setg() semantics, which is essentially
assert_eq!(unsafe { *errp }, ptr::null_mut());
followed by calling bindings::error_propagate().
Paolo
