yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
a19a21
From b2ac3e491eb7f18a421e2b1132e527d484681767 Mon Sep 17 00:00:00 2001
709dde
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>
a19a21
Date: Wed, 16 Dec 2020 16:06:09 -0500
a19a21
Subject: [PATCH 08/14] 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>
a19a21
Message-id: <20201216160615.324213-5-marcandre.lureau@redhat.com>
a19a21
Patchwork-id: 100477
a19a21
O-Subject: [RHEL-8.4.0 qemu-kvm PATCH v2 04/10] error: Document Error API usage rules
a19a21
Bugzilla: 1859494
a19a21
RH-Acked-by: Danilo de Paula <ddepaula@redhat.com>
a19a21
RH-Acked-by: Sergio Lopez Pascual <slp@redhat.com>
a19a21
RH-Acked-by: Stefan Hajnoczi <stefanha@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