Re: [PATCH v6 02/11] error: auto propagated local_err

[Vladimir Sementsov-Ogievskiy]

13.01.2020 11:50, Paul Durrant wrote:
> On Fri, 10 Jan 2020 at 19:42, Vladimir Sementsov-Ogievskiy
> <address@hidden> wrote:
> [snip]
>> +/*
>> + * ERRP_AUTO_PROPAGATE
>> + *
>> + * This macro is created to be the first line of a function which use
>> + * Error **errp parameter to report error. It's needed only in cases where 
>> we
>> + * want to use error_prepend, error_append_hint or dereference *errp. It's
>> + * still safe (but useless) in other cases.
>> + *
>> + * If errp is NULL or points to error_fatal, it is rewritten to point to a
>> + * local Error object, which will be automatically propagated to the 
>> original
>> + * errp on function exit (see error_propagator_cleanup).
>> + *
>> + * After invocation of this macro it is always safe to dereference errp
>> + * (as it's not NULL anymore) and to add information (by error_prepend or
>> + * error_append_hint)
>> + * (as, if it was error_fatal, we swapped it with a local_error to be
>> + * propagated on cleanup).
>> + *
>> + * Note: we don't wrap the error_abort case, as we want resulting coredump
>> + * to point to the place where the error happened, not to error_propagate.
>> + */
>> +#define ERRP_AUTO_PROPAGATE()                                  \
>> +    g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp};  \
>> +    errp = ((errp == NULL || *errp == error_fatal)             \
> 
> Perhaps !errp rather than errp == NULL, for brevity.
> 

I mostly prefer !ptr notation.. But may be here, I'd keep it as is,
to stress special-casing NULL in this non-trivial place.. And it is in good
relation with phrasing "If errp is NULL or points to error_fatal".
But !errp is OK for me to. Let it be as Markus prefer, he is maintainer.

> 
>> +            ? &_auto_errp_prop.local_err : errp)
>> +
>>   /*
>>    * Special error destination to abort on error.
>>    * See error_setg() and error_propagate() for details.
>> --
>> 2.21.0
>>


-- 
Best regards,
Vladimir