[Top][All Lists]

[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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]