Re: [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp

[Vladimir Sementsov-Ogievskiy]

11.10.2019 20:12, Eric Blake wrote:
> On 10/11/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>
> 
>>   scripts/coccinelle/auto-propagated-errp.cocci | 118 ++++++++++++++++++
>>   1 file changed, 118 insertions(+)
>>   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>>
>> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
>> b/scripts/coccinelle/auto-propagated-errp.cocci
>> new file mode 100644
>> index 0000000000..d9731620aa
>> --- /dev/null
>> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
> 
>> +@rule1@
>> +// Drop local_err
>> +identifier fn, local_err;
>> +symbol errp;
>> +@@
>> +
>> + fn(..., Error **errp, ...)
>> + {
>> +     <...
>> +-    Error *local_err = NULL;
>> +     ...>
>> + }
>> +
> 
> So our goal is to automate removal of all local_err (including when it is 
> spelled err)...
> 
>> +@@
>> +// Handle pattern with goto, otherwise we'll finish up
>> +// with labels at function end which will not compile.
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +identifier OUT;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +-    goto OUT;
>> ++    return;
>> +     ...>
>> +- OUT:
>> +-    error_propagate(errp, local_err);
>> + }
>> +
> 
> this dangling label cleanup makes sense
> 
>> +@@
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +(
>> +-    error_free(local_err);
>> +-    local_err = NULL;
>> ++    error_free_errp(errp);
> 
> This does not make sense - error_free_errp() is not defined prior to this 
> series or anywhere in patches 1-24, if I'm reading it correctly.
> 
>> +|
>> +-    error_free(local_err);
>> ++    error_free_errp(errp);
> 
> and again
> 
>> +|
>> +-    error_report_err(local_err);
>> ++    error_report_errp(errp);
>> +|
>> +-    warn_report_err(local_err);
>> ++    warn_report_errp(errp);
>> +|
>> +-    error_propagate_prepend(errp, local_err,
>> ++    error_prepend(errp,
>> +                              ...);
>> +|
>> +-    error_propagate(errp, local_err);
>> +)
>> +     ...>
>> + }
>> +
> 
> It looks like once this script is run, error_propagate_prepend() will have no 
> clients.

No, it still have a bit, when working with error_copy, and/or moving errors 
from/to structures.

> Is there a non-generated cleanup patch that removes it (and once it is 
> removed, it can also be removed from the .cocci script as no further clients 
> will reappear later)?

Maybe.

> 
> 
>> +@@
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +(
>> +-    &local_err
>> ++    errp
>> +|
>> +-    local_err
>> ++    *errp
>> +)
>> +     ...>
>> + }
>> +
>> +@@
>> +symbol errp;
>> +@@
>> +
>> +- *errp != NULL
>> ++ *errp
>>
> 
> Seems to make sense.
> 


-- 
Best regards,
Vladimir