On Wed, 24 Jun 2020 18:43:00 +0200
Markus Armbruster <arm...@redhat.com> wrote:
> This merely codifies existing practice, with one exception: the rule
> advising against returning void, where existing practice is mixed.
>
> When the Error API was created, we adopted the (unwritten) rule to
> return void when the function returns no useful value on success,
> unlike GError, which recommends to return true on success and false on
> error then.
>
> When a function returns a distinct error value, say false, a checked
> call that passes the error up looks like
>
> if (!frobnicate(..., errp)) {
> handle the error...
> }
>
> When it returns void, we need
>
> Error *err = NULL;
>
> frobnicate(..., &err);
> if (err) {
> handle the error...
> error_propagate(errp, err);
> }
>
> Not only is this more verbose, it also creates an Error object even
> when @errp is null, &error_abort or &error_fatal.
>
> People got tired of the additional boilerplate, and started to ignore
> the unwritten rule. The result is confusion among developers about
> the preferred usage.
>
This confusion is reinforced by the fact that the standard pattern:
error_setg(errp, ...);
error_append_hint(errp, ...);
doesn't work when errp is &error_fatal, which is a typical case of
an invalid command line argument, where it is valuable to suggest
something sensible to the user but error_setg() exits before we
could do so.
Fortunately, Vladimir's work will address that and eliminate the
temptation to workaround the issue with more boilerplate :)
> The written rule will hopefully reduce the confusion.
>
> The remainder of this series will update a substantial amount of code
> to honor the rule.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
Reviewed-by: Greg Kurz <gr...@kaod.org>
> include/qapi/error.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index 1a5ea25e12..c3d84d610a 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -15,6 +15,32 @@
> /*
> * Error reporting system loosely patterned after Glib's GError.
> *
> + * Rules:
> + *
> + * - Functions that use Error to report errors have an Error **errp
> + * parameter. It should be the last parameter, except for functions
> + * taking variable arguments.
> + *
> + * - You may pass NULL to not receive the error, &error_abort to abort
> + * on error, &error_fatal to exit(1) on error, or a pointer to a
> + * variable containing NULL to receive the error.
> + *
> + * - The value of @errp should not affect control flow.
> + *
> + * - On success, the function should not use @errp. On failure, it
> + * should set a new error, e.g. with error_setg(errp, ...), or
> + * propagate an existing one, e.g. with error_propagate(errp, ...).
> + *
> + * - Whenever practical, also return a value that indicates success /
> + * failure. This can make the error checking more concise, and can
> + * avoid useless error object creation and destruction. Note that
> + * we still have many functions returning void. We recommend
> + * • bool-valued functions return true on success / false on failure,
> + * • pointer-valued functions return non-null / null pointer, and
> + * • integer-valued functions return non-negative / negative.
> + *
> + * How to:
> + *
> * Create an error:
> * error_setg(errp, "situation normal, all fouled up");
> *
