> > + /// Equivalent of the C function `error_propagate`. Fill `*errp`
>
> Uh, is it? Let's see...
>
> > + /// with the information container in `self` if `errp` is not NULL;
> > + /// then consume it.
> > + ///
> > + /// # Safety
> > + ///
> > + /// `errp` must be a valid argument to `error_propagate`;
>
> Reminder for later: the valid @errp arguments for C error_propagate()
> are
>
> * NULL
>
> * &error_abort
>
> * &error_fatal
>
> * Address of some Error * variable containing NULL
>
> * Address of some Error * variable containing non-NULL
>
> The last one is *not* valid with error_setg().
>
> > + /// typically it is received from C code and need not be
> > + /// checked further at the Rust↔C boundary.
> > + pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
>
> Reminder, just to avoid confusion: C error_propagate() has the arguments
> in the opposite order.
>
> > + if errp.is_null() {
> > + return;
> > + }
> > +
> > + let err = self.clone_to_foreign_ptr();
> > +
> > + // SAFETY: caller guarantees errp is valid
> > + unsafe {
> > + errp.write(err);
> > + }
> > + }
>
> In C, we have two subtly different ways to store into some Error **errp
> argument: error_setg() and error_propagate().
>
> Their obvious difference is that error_setg() creates the Error object
> to store, while error_propagate() stores an existing Error object if
> any, else does nothing.
>
> Their unobvious difference is behavior when the destination already
> contains an Error. With error_setg(), this must not happen.
> error_propagate() instead throws away the new error. This permits
> "first one wins" error accumulation. Design mistake if you ask me.
>
> Your Rust propagate() also stores an existing bindings::Error. Note
> that "else does nothing" doesn't apply, because we always have an
> existing error object here, namely @self. In the error_propagate() camp
> so far.
>
> Let's examine the other aspect: how exactly "storing" behaves.
>
> error_setg() according to its contract:
>
> If @errp is NULL, the error is ignored. [...]
>
> If @errp is &error_abort, print a suitable message and abort().
>
> If @errp is &error_fatal, print a suitable message and exit(1).
>
> If @errp is anything else, *@errp must be NULL.
>
> error_propagate() according to its contract:
>
> [...] if @dst_errp is NULL, errors are being ignored. Free the
> error object.
>
> Else, if @dst_errp is &error_abort, print a suitable message and
> abort().
>
> Else, if @dst_errp is &error_fatal, print a suitable message and
> exit(1).
>
> Else, if @dst_errp already contains an error, ignore this one: free
> the error object.
>
> Else, move the error object from @local_err to *@dst_errp.
>
> The second to last clause is where its storing differs from
> error_setg().
>
> What does errp.write(err) do? I *guess* it simply stores @err in @errp.
> Matches neither behavior.
>
> If that's true, then passing &error_abort or &error_fatal to Rust does
> not work, and neither does error accumulation. Not equivalent of C
> error_propagate().
I did some simple tests. yes, &error_abort or &error_fatal doesn't work.
Current @errp of realize() can work because @errp points to @local_err
in device_set_realized().
> Is "propagate" semantics what you want here?
>
> If not, use another name.
I guess here we should call C version's error_propagate() instead of
write():
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
index a91ce6fefaf4..56622065ad22 100644
--- a/rust/qemu-api/src/error.rs
+++ b/rust/qemu-api/src/error.rs
@@ -205,7 +205,7 @@ pub unsafe fn propagate(self, errp: *mut *mut
bindings::Error) {
// SAFETY: caller guarantees errp is valid
unsafe {
- errp.write(err);
+ bindings::error_propagate(errp, err);
}
}
---
Then Rust's propagate has the same behavior as C (Of course, here Rust
is actually using C's error_propagate, so the two are equivalent.)
Regards,
Zhao
