[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 03/45] error: Document Error API usage rules
From: |
Eric Blake |
Subject: |
Re: [PATCH v4 03/45] error: Document Error API usage rules |
Date: |
Tue, 7 Jul 2020 13:46:28 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 7/7/20 11:05 AM, Markus Armbruster 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.
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>
---
@@ -95,14 +122,12 @@
* 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 caller
maybe s/to caller/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 doesn't, say a void function:
Hmm. It looks like you have a single sentence "Call a function... when
the function returns", but this line now makes it obvious that you have
a single prefix: "Call a function, ...and pass it to the caller:"
with two choices "when the function returns" and "when it doesn't". I'm
not sure if there is a nicer way to typeset it, adding yet another ":"
at the end of the line looks odd. The idea behind the text is fine, I'm
just trying to paint the bikeshed to see if there is a better presentation.
* Error *err = NULL;
* foo(arg, &err);
* if (err) {
@@ -120,6 +145,19 @@
* 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 doesn't, say a void function:
It helps that you have repeated the same pattern as above. But that
means if you change the layout, both groupings should have the same
layout. Maybe:
Intro for a task:
- when the function returns...
- when it doesn't
Also, are there functions that have a return type other than void, but
where the return value is not an indication of error? If there are,
then the "say a void function" clause makes sense (but we should
probably recommend against such functions); if there are not, then "say
a void function" reads awkwardly. Maybe:
- when it does not, 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);
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v4 00/45] Less clumsy error checking, Markus Armbruster, 2020/07/07
- [PATCH v4 11/45] qemu-option: Factor out helper opt_create(), Markus Armbruster, 2020/07/07
- [PATCH v4 04/45] qdev: Use returned bool to check for qdev_realize() etc. failure, Markus Armbruster, 2020/07/07
- [PATCH v4 12/45] qemu-option: Replace opt_set() by cleaner opt_validate(), Markus Armbruster, 2020/07/07
- [PATCH v4 08/45] qemu-option: Make uses of find_desc_by_name() more similar, Markus Armbruster, 2020/07/07
- [PATCH v4 15/45] block: Avoid error accumulation in bdrv_img_create(), Markus Armbruster, 2020/07/07
- [PATCH v4 09/45] qemu-option: Factor out helper find_default_by_name(), Markus Armbruster, 2020/07/07
- [PATCH v4 23/45] qom: Crash more nicely on object_property_get_link() failure, Markus Armbruster, 2020/07/07
- [PATCH v4 13/45] qemu-option: Make functions taking Error ** return bool, not void, Markus Armbruster, 2020/07/07