155 lines
5.4 KiB
Diff
155 lines
5.4 KiB
Diff
From b2ac3e491eb7f18a421e2b1132e527d484681767 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>
|
|
Date: Wed, 16 Dec 2020 16:06:09 -0500
|
|
Subject: [PATCH 08/14] error: Document Error API usage rules
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>
|
|
Message-id: <20201216160615.324213-5-marcandre.lureau@redhat.com>
|
|
Patchwork-id: 100477
|
|
O-Subject: [RHEL-8.4.0 qemu-kvm PATCH v2 04/10] error: Document Error API usage rules
|
|
Bugzilla: 1859494
|
|
RH-Acked-by: Danilo de Paula <ddepaula@redhat.com>
|
|
RH-Acked-by: Sergio Lopez Pascual <slp@redhat.com>
|
|
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
|
From: Markus Armbruster <armbru@redhat.com>
|
|
|
|
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.
|
|
|
|
Make the rule advising against returning void official by putting it
|
|
in writing. This will hopefully reduce confusion.
|
|
|
|
Update the examples accordingly.
|
|
|
|
The remainder of this series will update a substantial amount of code
|
|
to honor the rule.
|
|
|
|
Signed-off-by: Markus Armbruster <armbru@redhat.com>
|
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
|
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
|
|
Reviewed-by: Greg Kurz <groug@kaod.org>
|
|
Message-Id: <20200707160613.848843-4-armbru@redhat.com>
|
|
|
|
(cherry picked from commit e3fe3988d7851cac30abffae06d2f555ff7bee62)
|
|
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
|
|
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
---
|
|
include/qapi/error.h | 52 +++++++++++++++++++++++++++++++++++++++-----
|
|
1 file changed, 46 insertions(+), 6 deletions(-)
|
|
|
|
diff --git a/include/qapi/error.h b/include/qapi/error.h
|
|
index 3351fe76368..08d48e74836 100644
|
|
--- a/include/qapi/error.h
|
|
+++ b/include/qapi/error.h
|
|
@@ -15,6 +15,33 @@
|
|
/*
|
|
* 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.
|
|
+ *
|
|
+ * - Separation of concerns: the function is responsible for detecting
|
|
+ * errors and failing cleanly; handling the error is its caller's
|
|
+ * job. Since the value of @errp is about handling the error, the
|
|
+ * function should not examine it.
|
|
+ *
|
|
+ * - On success, the function should not touch *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.
|
|
+ *
|
|
* = Creating errors =
|
|
*
|
|
* Create an error:
|
|
@@ -95,14 +122,13 @@
|
|
* Create a new error and pass it to the caller:
|
|
* error_setg(errp, "situation normal, all fouled up");
|
|
*
|
|
- * Call a function and receive an error from it:
|
|
- * Error *err = NULL;
|
|
- * foo(arg, &err);
|
|
- * if (err) {
|
|
+ * Call a function, receive an error from it, and pass it to the caller
|
|
+ * - when the function returns a value that indicates failure, say
|
|
+ * false:
|
|
+ * if (!foo(arg, errp)) {
|
|
* handle the error...
|
|
* }
|
|
- *
|
|
- * Receive an error and pass it on to the caller:
|
|
+ * - when it does not, say because it is a void function:
|
|
* Error *err = NULL;
|
|
* foo(arg, &err);
|
|
* if (err) {
|
|
@@ -120,6 +146,20 @@
|
|
* foo(arg, errp);
|
|
* for readability.
|
|
*
|
|
+ * Receive an error, and handle it locally
|
|
+ * - when the function returns a value that indicates failure, say
|
|
+ * false:
|
|
+ * Error *err = NULL;
|
|
+ * if (!foo(arg, &err)) {
|
|
+ * handle the error...
|
|
+ * }
|
|
+ * - when it does not, say because it is a void function:
|
|
+ * Error *err = NULL;
|
|
+ * foo(arg, &err);
|
|
+ * if (err) {
|
|
+ * handle the error...
|
|
+ * }
|
|
+ *
|
|
* Receive and accumulate multiple errors (first one wins):
|
|
* Error *err = NULL, *local_err = NULL;
|
|
* foo(arg, &err);
|
|
--
|
|
2.27.0
|
|
|