cryptospore / rpms / qemu-kvm

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