Re: [PATCH 06/14] rust: qemu-api: add bindings to Error

[Paolo Bonzini <pbonzini@redhat.com>]

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