Re: [Qemu-devel] [RFC] error: auto propagated local_err

[Vladimir Sementsov-Ogievskiy]

19.09.2019 11:59, Max Reitz wrote:
> On 18.09.19 15:02, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is a proposal (three of them, actually) of auto propagation for
>> local_err, to not call error_propagate on every exit point, when we
>> deal with local_err.
>>
>> It also may help make Greg's series[1] about error_append_hint smaller.
>>
>> See definitions and examples below.
>>
>> I'm cc-ing to this RFC everyone from series[1] CC list, as if we like
>> it, the idea will touch same code (and may be more).
>>
>> [1]: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg03449.html
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>   include/qapi/error.h | 102 +++++++++++++++++++++++++++++++++++++++++++
>>   block.c              |  63 ++++++++++++--------------
>>   block/backup.c       |   8 +++-
>>   block/gluster.c      |   7 +++
>>   4 files changed, 144 insertions(+), 36 deletions(-)
> 
> If the combination of “if (local_err) { error_propagate(...); ... }” is
> what’s cumbersome, can’t this be done simpler by adding an
> error_propagate() variant with a return value?
> 
> i.e.
> 
> bool has_error_then_propagate(Error **errp, Error *err)
> {
>      if (!err) {
>          return false;
>      }
>      error_propagate(errp, err);
>      return true;
> }
> 
> And then turn all instances of
> 
> if (local_err) {
>      error_propagate(errp, local_err);
>      ...
> }
> 
> into
> 
> if (has_error_then_propagate(errp, local_err)) {
>      ...
> }
> 
> ?
> 
> Max
> 

No, originally cumbersome is introducing local_err in a lot of new places by
Greg's series. MAKE_ERRP_SAFE macro makes it as simple as one macro call
instead (in each function where we need local_err).

Also, auto-propagation seems correct thing to do, which fits good into
g_auto concept, so even without any macro it just allows to drop several 
error_propagate
calls (or may be several goto statements to do one error_propagate call) into
one definitions. It's the same story like with g_autofree vs g_free.

-- 
Best regards,
Vladimir