24.09.2019 23:48, Eric Blake wrote:
> On 9/24/19 3:08 PM, Vladimir Sementsov-Ogievskiy wrote:
>> error_append_hint will not work, if errp == &fatal_error, as program
>> will exit before error_append_hint call. Fix this by use of special
>> macro ERRP_FUNCTION_BEGIN.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> ---
>
> With the approach of a partial cleanup (rather than globally enforcing
> it for all functions with errp parameter), we'll probably be rerunning
> this Coccinelle script regularly, to track down any regressions.
>
>
>> +++ b/scripts/coccinelle/fix-error_append_hint-usage.cocci
>> @@ -0,0 +1,25 @@
>> +@rule0@
>> +// Add invocation to errp-functions
>> +identifier fn;
>> +@@
>> +
>> + fn(..., Error **errp, ...)
>> + {
>> ++ ERRP_FUNCTION_BEGIN();
>> + <+...
>> + error_append_hint(errp, ...);
>> + ...+>
>> + }
>
> Does not catch the case that we want to also use the macro for any use
> of *errp, but we can augment that later.
I don't want to include *errp here, as actually a lot of *errp invocations in
code are correct: they do it if errp is not NULL. So, it's not related to plan
B.
Still, I think we forget about error_prepend :)))
I've checked, if I include error_prepend here, series becomes 30 patches, which
is
not significantly larger. So I think, I'll cover error_prepend in v4.
>
>> +
>> +@@
>> +// Drop doubled invocation
>> +identifier rule0.fn;
>> +@@
>> +
>> + fn(...)
>> +{
>> + ERRP_FUNCTION_BEGIN();
>> +- ERRP_FUNCTION_BEGIN();
>> + ...
>> +}
>
> This is smaller than the script you posted in v2, and thus I'm a bit
> more confident in stating that it looks correct and idempotent.
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
--
Best regards,
Vladimir
