Eric Blake <ebl...@redhat.com> writes:
> 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/
Yes.
>> + * 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,
Yes, there are such functions.
> 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:
What about
- 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);
>>
Thanks!
