yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
709dde
From d931195ef5cccd6a4e6fceeba37809b1712c97ad Mon Sep 17 00:00:00 2001
709dde
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>
709dde
Date: Fri, 8 Jan 2021 07:40:55 -0500
709dde
Subject: [PATCH 04/10] error: Document Error API usage rules
709dde
MIME-Version: 1.0
709dde
Content-Type: text/plain; charset=UTF-8
709dde
Content-Transfer-Encoding: 8bit
709dde
709dde
RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>
709dde
Message-id: <20210108074101.290008-5-marcandre.lureau@redhat.com>
709dde
Patchwork-id: 100523
709dde
O-Subject: [RHEL-8.3.0.z qemu-kvm PATCH 04/10] error: Document Error API usage rules
709dde
Bugzilla: 1913818
709dde
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
709dde
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
709dde
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
709dde
709dde
From: Marc-André Lureau <marcandre.lureau@redhat.com>
709dde
709dde
From: Markus Armbruster <armbru@redhat.com>
709dde
709dde
This merely codifies existing practice, with one exception: the rule
709dde
advising against returning void, where existing practice is mixed.
709dde
709dde
When the Error API was created, we adopted the (unwritten) rule to
709dde
return void when the function returns no useful value on success,
709dde
unlike GError, which recommends to return true on success and false on
709dde
error then.
709dde
709dde
When a function returns a distinct error value, say false, a checked
709dde
call that passes the error up looks like
709dde
709dde
    if (!frobnicate(..., errp)) {
709dde
        handle the error...
709dde
    }
709dde
709dde
When it returns void, we need
709dde
709dde
    Error *err = NULL;
709dde
709dde
    frobnicate(..., &err;;
709dde
    if (err) {
709dde
        handle the error...
709dde
        error_propagate(errp, err);
709dde
    }
709dde
709dde
Not only is this more verbose, it also creates an Error object even
709dde
when @errp is null, &error_abort or &error_fatal.
709dde
709dde
People got tired of the additional boilerplate, and started to ignore
709dde
the unwritten rule.  The result is confusion among developers about
709dde
the preferred usage.
709dde
709dde
Make the rule advising against returning void official by putting it
709dde
in writing.  This will hopefully reduce confusion.
709dde
709dde
Update the examples accordingly.
709dde
709dde
The remainder of this series will update a substantial amount of code
709dde
to honor the rule.
709dde
709dde
Signed-off-by: Markus Armbruster <armbru@redhat.com>
709dde
Reviewed-by: Eric Blake <eblake@redhat.com>
709dde
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
709dde
Reviewed-by: Greg Kurz <groug@kaod.org>
709dde
Message-Id: <20200707160613.848843-4-armbru@redhat.com>
709dde
709dde
(cherry picked from commit e3fe3988d7851cac30abffae06d2f555ff7bee62)
709dde
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
709dde
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
709dde
---
709dde
 include/qapi/error.h | 52 +++++++++++++++++++++++++++++++++++++++-----
709dde
 1 file changed, 46 insertions(+), 6 deletions(-)
709dde
709dde
diff --git a/include/qapi/error.h b/include/qapi/error.h
709dde
index 3351fe76368..08d48e74836 100644
709dde
--- a/include/qapi/error.h
709dde
+++ b/include/qapi/error.h
709dde
@@ -15,6 +15,33 @@
709dde
 /*
709dde
  * Error reporting system loosely patterned after Glib's GError.
709dde
  *
709dde
+ * = Rules =
709dde
+ *
709dde
+ * - Functions that use Error to report errors have an Error **errp
709dde
+ *   parameter.  It should be the last parameter, except for functions
709dde
+ *   taking variable arguments.
709dde
+ *
709dde
+ * - You may pass NULL to not receive the error, &error_abort to abort
709dde
+ *   on error, &error_fatal to exit(1) on error, or a pointer to a
709dde
+ *   variable containing NULL to receive the error.
709dde
+ *
709dde
+ * - Separation of concerns: the function is responsible for detecting
709dde
+ *   errors and failing cleanly; handling the error is its caller's
709dde
+ *   job.  Since the value of @errp is about handling the error, the
709dde
+ *   function should not examine it.
709dde
+ *
709dde
+ * - On success, the function should not touch *errp.  On failure, it
709dde
+ *   should set a new error, e.g. with error_setg(errp, ...), or
709dde
+ *   propagate an existing one, e.g. with error_propagate(errp, ...).
709dde
+ *
709dde
+ * - Whenever practical, also return a value that indicates success /
709dde
+ *   failure.  This can make the error checking more concise, and can
709dde
+ *   avoid useless error object creation and destruction.  Note that
709dde
+ *   we still have many functions returning void.  We recommend
709dde
+ *   • bool-valued functions return true on success / false on failure,
709dde
+ *   • pointer-valued functions return non-null / null pointer, and
709dde
+ *   • integer-valued functions return non-negative / negative.
709dde
+ *
709dde
  * = Creating errors =
709dde
  *
709dde
  * Create an error:
709dde
@@ -95,14 +122,13 @@
709dde
  * Create a new error and pass it to the caller:
709dde
  *     error_setg(errp, "situation normal, all fouled up");
709dde
  *
709dde
- * Call a function and receive an error from it:
709dde
- *     Error *err = NULL;
709dde
- *     foo(arg, &err;;
709dde
- *     if (err) {
709dde
+ * Call a function, receive an error from it, and pass it to the caller
709dde
+ * - when the function returns a value that indicates failure, say
709dde
+ *   false:
709dde
+ *     if (!foo(arg, errp)) {
709dde
  *         handle the error...
709dde
  *     }
709dde
- *
709dde
- * Receive an error and pass it on to the caller:
709dde
+ * - when it does not, say because it is a void function:
709dde
  *     Error *err = NULL;
709dde
  *     foo(arg, &err;;
709dde
  *     if (err) {
709dde
@@ -120,6 +146,20 @@
709dde
  *     foo(arg, errp);
709dde
  * for readability.
709dde
  *
709dde
+ * Receive an error, and handle it locally
709dde
+ * - when the function returns a value that indicates failure, say
709dde
+ *   false:
709dde
+ *     Error *err = NULL;
709dde
+ *     if (!foo(arg, &err)) {
709dde
+ *         handle the error...
709dde
+ *     }
709dde
+ * - when it does not, say because it is a void function:
709dde
+ *     Error *err = NULL;
709dde
+ *     foo(arg, &err;;
709dde
+ *     if (err) {
709dde
+ *         handle the error...
709dde
+ *     }
709dde
+ *
709dde
  * Receive and accumulate multiple errors (first one wins):
709dde
  *     Error *err = NULL, *local_err = NULL;
709dde
  *     foo(arg, &err;;
709dde
-- 
709dde
2.27.0
709dde