[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagat
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp |
Date: |
Thu, 12 Mar 2020 08:23:05 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> 11.03.2020 17:41, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
>>
>>> 11.03.2020 12:38, Markus Armbruster wrote:
>>>> Vladimir Sementsov-Ogievskiy <address@hidden> 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 <address@hidden> 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 :)
>>>>
>>>
>>>
>>> Hmm, not so simple.
>>>
>>> It leads to reindenting of function header, which is bad.
>>
>> Because ___errp_coccinelle_updating___ is longer than errp, I guess.
>> Try ____?
>
> I'm afraid not. It's because it just adds \n, when I do
>
> ...,
>
> - errp
> + ___errp_coccinelle_updating___
> ,...
I was thinking of something like the appended patch, which in my
(superficial!) testing leaves alone newlines unless lines are long, but
hangs for block.c. Oh well.
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci
b/scripts/coccinelle/auto-propagated-errp.cocci
index bff274bd6d..492a4db826 100644
--- a/scripts/coccinelle/auto-propagated-errp.cocci
+++ b/scripts/coccinelle/auto-propagated-errp.cocci
@@ -35,12 +35,12 @@
// error_propagate_prepend().
@ depends on !(file in "util/error.c") disable optional_qualifier@
identifier fn;
-identifier _errp != errp;
+identifier _errp;
@@
fn(...,
- Error **_errp
-+ Error **errp
++ Error **____
,...)
{
(
@@ -48,7 +48,7 @@ identifier _errp != errp;
&
<...
- _errp
-+ errp
++ ____
...>
)
}
@@ -63,26 +63,26 @@ identifier _errp != errp;
// all possible control flows (otherwise, it will not match standard pattern
// when error_propagate() call is in if branch).
@ disable optional_qualifier exists@
-identifier fn, local_err, errp;
+identifier fn, local_err;
@@
- fn(..., Error **errp, ...)
+ fn(..., Error **____, ...)
{
+ ERRP_AUTO_PROPAGATE();
... when != ERRP_AUTO_PROPAGATE();
(
- error_append_hint(errp, ...);
+ error_append_hint(____, ...);
|
- error_prepend(errp, ...);
+ error_prepend(____, ...);
|
- error_vprepend(errp, ...);
+ error_vprepend(____, ...);
|
Error *local_err = NULL;
...
(
- error_propagate_prepend(errp, local_err, ...);
+ error_propagate_prepend(____, local_err, ...);
|
- error_propagate(errp, local_err);
+ error_propagate(____, local_err);
)
)
... when any
@@ -92,18 +92,17 @@ identifier fn, local_err, errp;
// Match scenarios with propagation of local error to errp.
@rule1 disable optional_qualifier exists@
identifier fn, local_err;
-symbol errp;
@@
- fn(..., Error **errp, ...)
+ fn(..., Error **____, ...)
{
...
Error *local_err = NULL;
...
(
- error_propagate_prepend(errp, local_err, ...);
+ error_propagate_prepend(____, local_err, ...);
|
- error_propagate(errp, local_err);
+ error_propagate(____, local_err);
)
...
}
@@ -118,7 +117,6 @@ symbol errp;
// without error_propagate() call), coccinelle fails to match this "out: }".
@@
identifier rule1.fn, rule1.local_err, out;
-symbol errp;
@@
fn(...)
@@ -128,7 +126,7 @@ symbol errp;
+ return;
...>
- out:
-- error_propagate(errp, local_err);
+- error_propagate(____, local_err);
}
// Convert most of local_err related staff.
@@ -159,7 +157,6 @@ symbol errp;
@ exists@
identifier rule1.fn, rule1.local_err;
expression list args;
-symbol errp;
@@
fn(...)
@@ -172,30 +169,30 @@ symbol errp;
// Convert error clearing functions
(
- error_free(local_err);
-+ error_free_errp(errp);
++ error_free_errp(____);
|
- error_report_err(local_err);
-+ error_report_errp(errp);
++ error_report_errp(____);
|
- error_reportf_err(local_err, args);
-+ error_reportf_errp(errp, args);
++ error_reportf_errp(____, args);
|
- warn_report_err(local_err);
-+ warn_report_errp(errp);
++ warn_report_errp(____);
|
- warn_reportf_err(local_err, args);
-+ warn_reportf_errp(errp, args);
++ warn_reportf_errp(____, args);
)
?- local_err = NULL;
|
-- error_propagate_prepend(errp, local_err, args);
-+ error_prepend(errp, args);
+- error_propagate_prepend(____, local_err, args);
++ error_prepend(____, args);
|
-- error_propagate(errp, local_err);
+- error_propagate(____, local_err);
|
- &local_err
-+ errp
++ ____
)
...>
}
@@ -205,27 +202,43 @@ symbol errp;
// conflicts with other substitutions in it (at least with "- local_err =
NULL").
@@
identifier rule1.fn, rule1.local_err;
-symbol errp;
@@
fn(...)
{
<...
- local_err
-+ *errp
++ *____
...>
}
// Always use the same patter for checking error
@@
identifier rule1.fn;
-symbol errp;
@@
fn(...)
{
<...
-- *errp != NULL
-+ *errp
+- *____ != NULL
++ *____
...>
}
+
+@@
+identifier fn;
+symbol errp;
+@@
+
+ fn(...,
+- Error **____
++ Error **errp
+ ,...)
+ {
+ ...
+ }
+
+@@
+@@
+-____
++errp
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, (continued)
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/10
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp,
Markus Armbruster <=
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/12
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/11
- Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Vladimir Sementsov-Ogievskiy, 2020/03/11
Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp, Markus Armbruster, 2020/03/11
[PATCH v8 01/10] error: auto propagated local_err, Vladimir Sementsov-Ogievskiy, 2020/03/06