Paolo Bonzini <pbonz...@redhat.com> writes:
> Provide an implementation of std::error::Error that bridges the Rust
> anyhow::Error and std::panic::Location types with QEMU's Error*.
>
> It also has several utility methods, analogous to error_propagate(),
> that convert a Result into a return value + Error** pair. One important
> difference is that these propagation methods *panic* if *errp is NULL,
> unlike error_propagate() which eats subsequent errors[1]. The reason
> for this is that in C you have an error_set*() call at the site where
> the error is created, and calls to error_propagate() are relatively rare.
>
> In Rust instead, even though these functions do "propagate" a
> qemu_api::Error into a C Error**, there is no error_setg() anywhere that
> could check for non-NULL errp and call abort(). error_propagate()'s
> behavior of ignoring subsequent errors is generally considered weird,
> and there would be a bigger risk of triggering it from Rust code.
>
> [1] This is actually a violation of the preconditions of error_propagate(),
> so it should not happen. But you never know...
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
[...]
> diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
> new file mode 100644
> index 00000000000..80157f6ea1b
> --- /dev/null
> +++ b/rust/qemu-api/src/error.rs
[...]
> + /// Equivalent of the C function `error_propagate`. Fill `*errp`
> + /// with the information container in `self` if `errp` is not NULL;
> + /// then consume it.
> + ///
> + /// This is similar to the C API `error_propagate`, but it panics if
> + /// `*errp` is not `NULL`.
> + ///
> + /// # Safety
> + ///
> + /// `errp` must be a valid argument to `error_propagate`; it can be
> + /// `NULL` or it can point to any of:
> + /// * `error_abort`
> + /// * `error_fatal`
> + /// * a local variable of (C) type `Error *`
This local variable must contain NULL.
> + ///
> + /// Typically `errp` 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) {
> + if errp.is_null() {
> + return;
> + }
> +
> + // SAFETY: caller guarantees that errp and *errp are valid
> + unsafe {
> + assert_eq!(*errp, ptr::null_mut());
> + bindings::error_propagate(errp, self.clone_to_foreign_ptr());
> + }
> + }
[...]
With the comment tightened:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
The commit message and comment improvements are lovely!
