Re: [PATCH v4 01/31] errp: rename errp to errp_in where it is IN-argument

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <address@hidden> writes:

> 08.10.2019 12:08, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
[...]
>>> diff --git a/util/error.c b/util/error.c
>>> index d4532ce318..b3ff3832d6 100644
>>> --- a/util/error.c
>>> +++ b/util/error.c
>>> @@ -271,11 +271,11 @@ void error_free(Error *err)
>>>       }
>>>   }
>>>   
>>> -void error_free_or_abort(Error **errp)
>>> +void error_free_or_abort(Error **errp_in)
>>>   {
>>> -    assert(errp && *errp);
>>> -    error_free(*errp);
>>> -    *errp = NULL;
>>> +    assert(errp_in && *errp_in);
>>> +    error_free(*errp_in);
>>> +    *errp_in = NULL;
>>>   }
>>>   
>>>   void error_propagate(Error **dst_errp, Error *local_err)
>> 
>> This functions frees the error.  It leaves nothing for the caller to
>> clean up.
>> 
>> All callers pass &ERR, where ERR is a local variable.  We can peel off
>> an indirection.
>
>
> But if we drop indirection, we'll have to set local variable to NULL by
> hand. Is it good?
>
> Look at test_keyval_parse_list() for example: it uses local err object
> several times, so it depends on the fact that error_free_or_abort
> sets pointer to NULL.

You're right, peeling off the indirection would make
error_free_or_abort() worse.

It's a convenience function for tests.  Confusion seems unlikely to me.
Let's not worry about it.