Re: [PATCH v3 05/25] scripts: add coccinelle script to fix error_append_hint usage

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
>> ---
> 
> 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 <address@hidden>
> 


-- 
Best regards,
Vladimir