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 <arm...@redhat.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Reviewed-by: Greg Kurz <gr...@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