Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

[Markus Armbruster <armbru@xxxxxxxxxx>]

Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:

> 09.03.2020 12:56, Markus Armbruster wrote:
>> Suggest
>>
>>      scripts: Coccinelle script to use auto-propagated errp
>>
>> or
>>
>>      scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()
>>
>> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:
[...]
>>> +// Note, that we update everything related to matched by rule1 function 
>>> name
>>> +// and local_err name. We may match something not related to the pattern
>>> +// matched by rule1. For example, local_err may be defined with the same 
>>> name
>>> +// in different blocks inside one function, and in one block follow the
>>> +// propagation pattern and in other block doesn't. Or we may have several
>>> +// functions with the same name (for different configurations).
>>
>> Context: rule1 matches functions that have all three of
>>
>> * an Error **errp parameter
>>
>> * an Error *local_err = NULL variable declaration
>>
>> * an error_propagate(errp, local_err) or error_propagate_prepend(errp,
>>    local_err, ...) expression, where @errp is the parameter and
>>    @local_err is the variable.
>>
>> If I understand you correctly, you're pointing out two potential issues:
>>
>> 1. This rule can match functions rule1 does not match if there is
>> another function with the same name that rule1 does match.
>>
>> 2. This rule matches in the entire function matched by rule1, even when
>> parts of that function use a different @errp or @local_err.
>>
>> I figure these apply to all rules with identifier rule1.fn, not just
>> this one.  Correct?
>>
>> Regarding 1.  There must be a better way to chain rules together, but I
>> don't know it.
>
> Hmm, what about something like this:
>
> @rule1 disable optional_qualifier exists@
> identifier fn, local_err;
> symbol errp;
> @@
>
>  fn(..., Error **
> - errp
> + ___errp_coccinelle_updating___
>     , ...)
>  {
>      ...
>      Error *local_err = NULL;
>      ...
> (
>     error_propagate_prepend(errp, local_err, ...);
> |
>     error_propagate(errp, local_err);
> )
>      ...
>  }
>
>
> [..]
>
> match symbol ___errp_coccinelle_updating___ in following rules in function 
> header
>
> [..]
>
>
> @ disable optional_qualifier@
> identifier fn, local_err;
> symbol errp;
> @@
>
>  fn(..., Error **
> - ___errp_coccinelle_updating___
> + errp
>     , ...)
>  {
>      ...
>  }
>
>
> - hacky, but seems not more hacky than python detection, and should work 
> better

As simple, forceful and unsubtle as a sledgehammer.  I like it :)

[...]


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel