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

2020-03-12 Thread Vladimir Sementsov-Ogievskiy 

12.03.2020 10:23, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


11.03.2020 17:41, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


11.03.2020 12:38, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  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.


Sorry, I didn't say, but I've checked and I was wrong: ___  works fine. But
we need --max-width 80, otherwise coccinelle wraps some lines which fit into
80 characters and produces extra hunks.

So, now I'm preparing v9 with errp->->errp concept. It differs from your 
patch and works on block.c

We don't need to switch all errp into , it's an extra complication. 
Changing name only in function
header is enough. And don't worry about broken compilation: it's broken anyway 
among the rules, and all
is OK after all rules applied.




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,

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

2020-03-12 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  writes:

> 11.03.2020 17:41, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  writes:
>>
>>> 11.03.2020 12:38, Markus Armbruster wrote:
 Vladimir Sementsov-Ogievskiy  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  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, ...);
 |
-

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 17:41, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


11.03.2020 12:38, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  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___
,...




Possible solution is instead

fn(...)
{
+   ___errp_coccinelle_updating___();


but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.

.

So, I'm returning to just a warning.

I think something simple like

@@
identifier rule1.fn;
position p != rule1.p;
@@

fn@p(...) {...}

@ script:python@



should work.


Up to you.




--
Best regards,
Vladimir

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

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

2020-03-11 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  writes:

> 11.03.2020 12:38, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  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  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 ?

> Possible solution is instead
>
> fn(...)
> {
> +   ___errp_coccinelle_updating___();
>
>
> but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.
>
> .
>
> So, I'm returning to just a warning.
>
> I think something simple like
>
> @@
> identifier rule1.fn;
> position p != rule1.p;
> @@
>
> fn@p(...) {...}
>
> @ script:python@
>
> 
>
> should work.

Up to you.


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

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 12:38, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  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.

Possible solution is instead

fn(...)
{
+   ___errp_coccinelle_updating___();


but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.

.

So, I'm returning to just a warning.

I think something simple like

@@
identifier rule1.fn;
position p != rule1.p;
@@

fn@p(...) {...}

@ script:python@



should work.

--
Best regards,
Vladimir

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

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 12:53, Markus Armbruster wrote:

I think a v9 makes sense now.

If any of the improvement ideas should turn into time sinks for you,
let's talk.  We don't need perfection, we only need to get to the point
where we trust the script to do what we believe it does, understand its
limitations, and know how to compensate for them.

Right now I'm optimistic v9 will be ready for merging, perhaps with some
minor tweaking in my tree.



Good. I hope, I'll resend today.

--
Best regards,
Vladimir

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

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

2020-03-11 Thread Markus Armbruster 
I think a v9 makes sense now.

If any of the improvement ideas should turn into time sinks for you,
let's talk.  We don't need perfection, we only need to get to the point
where we trust the script to do what we believe it does, understand its
limitations, and know how to compensate for them.

Right now I'm optimistic v9 will be ready for merging, perhaps with some
minor tweaking in my tree.


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

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 12:33, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

+
+// Convert error clearing functions

Suggest: Ensure @local_err is cleared on free


But there is no local_err after conversion


True.  Hmm.  What about this:

  // Convert calls to error_free(), possibly indirect
  // In addition to replacing @local_err by *errp, we have to clear *errp
  // to avoid use-after-free in the automatic error propagation.



OK


+(
+-error_free(local_err);
++error_free_errp(errp);
+|
+-error_report_err(local_err);
++error_report_errp(errp);
+|
+-error_reportf_err(local_err, args);
++error_reportf_errp(errp, args);
+|
+-warn_report_err(local_err);
++warn_report_errp(errp);
+|
+-warn_reportf_err(local_err, args);





--
Best regards,
Vladimir

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

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

2020-03-11 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  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  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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2020-03-11 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  writes:

> 09.03.2020 12:56, Markus Armbruster wrote:
>>> +
>>> +// Convert error clearing functions
>> Suggest: Ensure @local_err is cleared on free
>
> But there is no local_err after conversion

True.  Hmm.  What about this:

 // Convert calls to error_free(), possibly indirect
 // In addition to replacing @local_err by *errp, we have to clear *errp
 // to avoid use-after-free in the automatic error propagation.

>>> +(
>>> +-error_free(local_err);
>>> ++error_free_errp(errp);
>>> +|
>>> +-error_report_err(local_err);
>>> ++error_report_errp(errp);
>>> +|
>>> +-error_reportf_err(local_err, args);
>>> ++error_reportf_errp(errp, args);
>>> +|
>>> +-warn_report_err(local_err);
>>> ++warn_report_errp(errp);
>>> +|
>>> +-warn_reportf_err(local_err, args);


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

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 12:04, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

include/qapi/error.h  |   3 +
scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
2 files changed, 234 insertions(+)
create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
 * }
 * ...
 * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
 */
  #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]


As our discussion shows, this script is somewhat hard to understand.
That's okay, it solves a somewhat thorny problem, and I don't have
better ideas.  But let me state the intended transformations once more,
so I don't get lost in the details.

Motivation:

1. Make error propagation less error-prone and improve stack backtraces

The "receive error in _error, propagate to @errp" pattern is
tedious, error-prone, and produces unhelpful stack backtraces with
_abort.

ERRP_AUTO_PROPAGATE() removes manual propagation.  It additionally
gives us the stack backtraces we want.

2. Improve error messages with _fatal

Passing @errp to error_append_hint(), error_prepend() or
error_vprepend() is useless when @errp is _fatal.

ERRP_AUTO_PROPAGATE() fixes this by

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

2020-03-11 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  writes:

> 10.03.2020 18:47, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  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  writes:

> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
> does corresponding changes in code (look for details in
> include/qapi/error.h)
>
> Usage example:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]

 Suggest FILES... instead of a specific set of files.

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> Cc: Eric Blake 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Greg Kurz 
> Cc: Christian Schoenebeck 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paul Durrant 
> Cc: Stefan Hajnoczi 
> Cc: "Philippe Mathieu-Daudé" 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Stefan Berger 
> Cc: Markus Armbruster 
> Cc: Michael Roth 
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-de...@nongnu.org
> Cc: xen-devel@lists.xenproject.org
>
>include/qapi/error.h  |   3 +
>scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
>2 files changed, 234 insertions(+)
>create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index bb9bcf02fb..fbfc6f1c0b 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -211,6 +211,9 @@
> * }
> * ...
> * }
> + *
> + * For mass conversion use script

 mass-conversion (we're not converting mass, we're converting en masse)

> + *   scripts/coccinelle/auto-propagated-errp.cocci
> */
>  #ifndef ERROR_H
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
> b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci

 Preface to my review of this script: may aim isn't to make it
 bullet-proof.  I want to (1) make it good enough (explained in a
 jiffie), and (2) automatically identify the spots where it still isn't
 obviously safe for manual review.

 The latter may involve additional scripting.  That's okay.

 The script is good enough when the number of possibly unsafe spots is
 low enough for careful manual review.

 When I ask for improvements that, in your opinion, go beyond "good
 enough", please push back.  I'm sure we can work it out together.

> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.
> +//
> +// This program is free software; you can redistribute it and/or modify
> +// it under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 2 of the License, or
> +// (at your option) any later version.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License
> +// along with this program.  If not, see .
> +//
> +// Usage example:
> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> +//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
> +//  --max-width 80 blockdev-nbd.c qemu-nbd.c \

 You have --max-width 80 here, but not in the commit message.  Default
 seems to be 78.  Any particular reason to change it to 80?
>>>
>>> Hmm. As I remember, without this parameter, reindenting doesn't work 
>>> correctly.
>>> So, I'm OK with "--max-width 78", but I doubt that it will work without a 
>>> parameter.
>>> Still, may be I'm wrong, we can check it.
>>
>> If you can point to an example where --max-width helps, keep it, and
>> update the commit message to match.  Else, drop it.
>>

> +//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]

As our discussion shows, this script is somewhat hard to understand.
That's okay, it solves a somewhat thorny problem, and I don't have
better ideas.  But let me state the intended transformations once more,
so I

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

09.03.2020 12:56, Markus Armbruster wrote:

+
+// Convert error clearing functions

Suggest: Ensure @local_err is cleared on free


But there is no local_err after conversion




+(
+-error_free(local_err);
++error_free_errp(errp);
+|
+-error_report_err(local_err);
++error_report_errp(errp);
+|
+-error_reportf_err(local_err, args);
++error_reportf_errp(errp, args);
+|
+-warn_report_err(local_err);
++warn_report_errp(errp);
+|
+-warn_reportf_err(local_err, args);



--
Best regards,
Vladimir

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

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

11.03.2020 9:55, Vladimir Sementsov-Ogievskiy wrote:

10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
   --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
   blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

   include/qapi/error.h  |   3 +
   scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
   2 files changed, 234 insertions(+)
   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
    * }
    * ...
    * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
    */
 #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

 // Skip functions with "assert(_errp && *_errp)" statement, because that
 // signals unusual semantics, and the parameter name may well

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

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  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

  include/qapi/error.h  |   3 +
  scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
  2 files changed, 234 insertions(+)
  create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
   * }
   * ...
   * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
   */
  
  #ifndef ERROR_H

diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

// Skip functions with "assert(_errp && *_errp)" statement, because that
// signals unusual semantics, and the parameter name may well serve a
// purpose.


+// (like nbd_iter_channel_error()).
+//
+// Skip util/error.c to not touch, for example, error_propagate and
+// error_propagate_prepend().


error_propagate()

I much appreciate your meticulous explanation of what you skip and why.


+@ depends on !(file in "util/error.c") disable optional_qualifier@
+identifier fn;
+identifier _errp != errp;
+@@
+
+ fn(...,
+-   Error **_errp
++   Error **errp
+,...)
+ {
+(
+ ... when != assert(_errp && *_errp)
+&

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

2020-03-11 Thread Vladimir Sementsov-Ogievskiy 

10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  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  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
   --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
   blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

   include/qapi/error.h  |   3 +
   scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
   2 files changed, 234 insertions(+)
   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
* }
* ...
* }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
*/
 #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

 // Skip functions with "assert(_errp && *_errp)" statement, because that
 // signals unusual semantics, and the parameter name may well serve a
 // purpose.


Sounds good.




+// (like

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

2020-03-10 Thread Markus Armbruster 
Vladimir Sementsov-Ogievskiy  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  writes:
>>
>>> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
>>> does corresponding changes in code (look for details in
>>> include/qapi/error.h)
>>>
>>> Usage example:
>>> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>>>   --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>>>   blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
>>
>> Suggest FILES... instead of a specific set of files.
>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>
>>> Cc: Eric Blake 
>>> Cc: Kevin Wolf 
>>> Cc: Max Reitz 
>>> Cc: Greg Kurz 
>>> Cc: Christian Schoenebeck 
>>> Cc: Stefano Stabellini 
>>> Cc: Anthony Perard 
>>> Cc: Paul Durrant 
>>> Cc: Stefan Hajnoczi 
>>> Cc: "Philippe Mathieu-Daudé" 
>>> Cc: Laszlo Ersek 
>>> Cc: Gerd Hoffmann 
>>> Cc: Stefan Berger 
>>> Cc: Markus Armbruster 
>>> Cc: Michael Roth 
>>> Cc: qemu-bl...@nongnu.org
>>> Cc: qemu-de...@nongnu.org
>>> Cc: xen-devel@lists.xenproject.org
>>>
>>>   include/qapi/error.h  |   3 +
>>>   scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
>>>   2 files changed, 234 insertions(+)
>>>   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>>>
>>> diff --git a/include/qapi/error.h b/include/qapi/error.h
>>> index bb9bcf02fb..fbfc6f1c0b 100644
>>> --- a/include/qapi/error.h
>>> +++ b/include/qapi/error.h
>>> @@ -211,6 +211,9 @@
>>>* }
>>>* ...
>>>* }
>>> + *
>>> + * For mass conversion use script
>>
>> mass-conversion (we're not converting mass, we're converting en masse)
>>
>>> + *   scripts/coccinelle/auto-propagated-errp.cocci
>>>*/
>>> #ifndef ERROR_H
>>> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
>>> b/scripts/coccinelle/auto-propagated-errp.cocci
>>> new file mode 100644
>>> index 00..bff274bd6d
>>> --- /dev/null
>>> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
>>
>> Preface to my review of this script: may aim isn't to make it
>> bullet-proof.  I want to (1) make it good enough (explained in a
>> jiffie), and (2) automatically identify the spots where it still isn't
>> obviously safe for manual review.
>>
>> The latter may involve additional scripting.  That's okay.
>>
>> The script is good enough when the number of possibly unsafe spots is
>> low enough for careful manual review.
>>
>> When I ask for improvements that, in your opinion, go beyond "good
>> enough", please push back.  I'm sure we can work it out together.
>>
>>> @@ -0,0 +1,231 @@
>>> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
>>> +//
>>> +// Copyright (c) 2020 Virtuozzo International GmbH.
>>> +//
>>> +// This program is free software; you can redistribute it and/or modify
>>> +// it under the terms of the GNU General Public License as published by
>>> +// the Free Software Foundation; either version 2 of the License, or
>>> +// (at your option) any later version.
>>> +//
>>> +// This program is distributed in the hope that it will be useful,
>>> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +// GNU General Public License for more details.
>>> +//
>>> +// You should have received a copy of the GNU General Public License
>>> +// along with this program.  If not, see .
>>> +//
>>> +// Usage example:
>>> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>>> +//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>>> +//  --max-width 80 blockdev-nbd.c qemu-nbd.c \
>>
>> You have --max-width 80 here, but not in the commit message.  Default
>> seems to be 78.  Any particular reason to change it to 80?
>
> Hmm. As I remember, without this parameter, reindenting doesn't work 
> correctly.
> So, I'm OK with "--max-width 78", but I doubt that it will work without a 
> parameter.
> Still, may be I'm wrong, we can check it.

If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.

>>
>>> +//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
>>> +
>>> +// Switch unusual (Error **) parameter names to errp
>>
>> Let's drop the parenthesis around Error **
>>
>>> +// (this is necessary to use ERRP_AUTO_PROPAGATE).
>>
>> Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
>> make the fact we're messing with @errp more obvious.  Too late; I
>> shouldn't rock the boat that much now.
>>
>>> +//
>>> +// Disable optional_qualifier to skip functions with "Error *const *errp"
>>> +// parameter.
>>> +//
>>> +// Skip functions with "assert(_errp && *_errp)" statement, as they have
>>> +//

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

2020-03-10 Thread Vladimir Sementsov-Ogievskiy 

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  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

  include/qapi/error.h  |   3 +
  scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
  2 files changed, 234 insertions(+)
  create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
   * }
   * ...
   * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
   */
  
  #ifndef ERROR_H

diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

// Skip functions with "assert(_errp && *_errp)" statement, because that
// signals unusual semantics, and the parameter name may well serve a
// purpose.


Sounds good.




+// (like nbd_iter_channel_error()).
+//
+// Skip util/error.c to not touch, for example, error_propagate and
+// error_propagate_prepend().


error_propagate()

I much appreciate your meticulous explanation of what you skip and why.

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

2020-03-10 Thread Vladimir Sementsov-Ogievskiy 

08.03.2020 22:09, Christian Schoenebeck wrote:

On Freitag, 6. März 2020 06:15:28 CET Vladimir Sementsov-Ogievskiy wrote:

diff --git a/scripts/coccinelle/auto-propagated-errp.cocci
b/scripts/coccinelle/auto-propagated-errp.cocci new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci
@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.


Just in case:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?


Hmm, seems this, and some other coccinelle scripts should be added to "Error 
reporting"
section.



Best regards,
Christian Schoenebeck






--
Best regards,
Vladimir

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

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

2020-03-09 Thread Markus Armbruster 
Suggest

scripts: Coccinelle script to use auto-propagated errp

or

scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:

> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
> does corresponding changes in code (look for details in
> include/qapi/error.h)
>
> Usage example:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]

Suggest FILES... instead of a specific set of files.

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> Cc: Eric Blake 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Greg Kurz 
> Cc: Christian Schoenebeck 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paul Durrant 
> Cc: Stefan Hajnoczi 
> Cc: "Philippe Mathieu-Daudé" 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Stefan Berger 
> Cc: Markus Armbruster 
> Cc: Michael Roth 
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-de...@nongnu.org
> Cc: xen-devel@lists.xenproject.org
>
>  include/qapi/error.h  |   3 +
>  scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
>  2 files changed, 234 insertions(+)
>  create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index bb9bcf02fb..fbfc6f1c0b 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -211,6 +211,9 @@
>   * }
>   * ...
>   * }
> + *
> + * For mass conversion use script

mass-conversion (we're not converting mass, we're converting en masse)

> + *   scripts/coccinelle/auto-propagated-errp.cocci
>   */
>  
>  #ifndef ERROR_H
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
> b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci

Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.

> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.
> +//
> +// This program is free software; you can redistribute it and/or modify
> +// it under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 2 of the License, or
> +// (at your option) any later version.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License
> +// along with this program.  If not, see .
> +//
> +// Usage example:
> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> +//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
> +//  --max-width 80 blockdev-nbd.c qemu-nbd.c \

You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?

> +//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
> +
> +// Switch unusual (Error **) parameter names to errp

Let's drop the parenthesis around Error **

> +// (this is necessary to use ERRP_AUTO_PROPAGATE).

Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.

> +//
> +// Disable optional_qualifier to skip functions with "Error *const *errp"
> +// parameter.
> +//
> +// Skip functions with "assert(_errp && *_errp)" statement, as they have
> +// non generic semantics and may have unusual Error ** argument name for 
> purpose

non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

   // Skip functions with "assert(_errp && *_errp)" statement, because that
   // signals unusual semantics, and the parameter name may well serve a
   // purpose.

> +// (like nbd_iter_channel_error()).
> +//
> +// Skip util/error.c to not touch, for example, error_propagate and
> +// error_propagate_prepend().

error_propagate()

I much appreciate your meticulous explanation of what you skip and why.

> +@ depends on !(file in "util/error.c") disable optional_qualifier@
> +identifier fn;
> +identifier _errp != errp;
> +@@
> +

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

2020-03-08 Thread Christian Schoenebeck 
On Freitag, 6. März 2020 06:15:28 CET Vladimir Sementsov-Ogievskiy wrote:
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci
> b/scripts/coccinelle/auto-propagated-errp.cocci new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.

Just in case:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

Best regards,
Christian Schoenebeck



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

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

2020-03-06 Thread Eric Blake 

On 3/5/20 11:15 PM, Vladimir Sementsov-Ogievskiy wrote:

Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---



I'll let Markus do the final review of this, but my personal take is 
that if the subsequent patches created by using this script are 
reasonable, then this script was good enough.



+// Always use the same patter for checking error


pattern


+@@
+identifier rule1.fn;
+symbol errp;
+@@
+
+ fn(...)
+ {
+ <...
+-*errp != NULL
++*errp
+ ...>
+ }



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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