[PATCH 0/4] improve platform_get_irq.cocci

[PATCH 0/4] improve platform_get_irq.cocci

[Julia Lawall]

Various cleanups in platform_get_irq.cocci to give more useful
patches and to give a few more results.

[PATCH 1/4] coccinelle: platform_get_irq: simplify context case

[Julia Lawall]

Simplify to just report on the first dev_err, whereever it occurs.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 scripts/coccinelle/api/platform_get_irq.cocci |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci
index 06b6a95..c6ac755 100644
--- a/scripts/coccinelle/api/platform_get_irq.cocci
+++ b/scripts/coccinelle/api/platform_get_irq.cocci
@@ -23,16 +23,9 @@ platform_get_irq_byname
 
 if ( \( ret < 0 \| ret <= 0 \) )
 {
-(
-if (ret != -EPROBE_DEFER)
-{ ...
-*dev_err(...);
-... }
-|
 ...
 *dev_err(...);
-)
-...
+... when any
 }
 
 @depends on patch@

[PATCH 2/4] coccinelle: platform_get_irq: handle 2-statement branches

[Julia Lawall]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

Treat separately the case where there is only one other
statement in the branch, to be able to remove the outer
{} as well.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 scripts/coccinelle/api/platform_get_irq.cocci |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci
index c6ac755..7ac32ee 100644
--- a/scripts/coccinelle/api/platform_get_irq.cocci
+++ b/scripts/coccinelle/api/platform_get_irq.cocci
@@ -31,6 +31,25 @@ if ( \( ret < 0 \| ret <= 0 \) )
 @depends on patch@
 expression ret;
 struct platform_device *E;
+statement S;
+@@
+
+ret =
+(
+platform_get_irq
+|
+platform_get_irq_byname
+)(E, ...);
+
+if ( \( ret < 0 \| ret <= 0 \) )
+-{
+-dev_err(...);
+S
+-}
+
+@depends on patch@
+expression ret;
+struct platform_device *E;
 @@
 
 ret =

Re: [PATCH 2/4] coccinelle: platform_get_irq: handle 2-statement branches

[Markus Elfring]

> From: Masahiro Yamada <yamada.masahiro@socionext.com>

I wonder about this information.
Would you like to use the tag “Suggested-by” instead?


…
> +++ b/scripts/coccinelle/api/platform_get_irq.cocci
> @@ -31,6 +31,25 @@ if ( \( ret < 0 \| ret <= 0 \) )
…
> +ret =
> +(
> +platform_get_irq
> +|
> +platform_get_irq_byname
> +)(E, ...);
> +
> +if ( \( ret < 0 \| ret <= 0 \) )
> +-{
> +-dev_err(...);
> +S
> +-}

How do you think about to use the following SmPL code variant?

+ ret =
+(platform_get_irq
+|platform_get_irq_byname
+)(E, ...);
+
+ if ( \( ret < 0 \| ret <= 0 \) )
+-{
+-dev_err(...);
+ S
+-}

Regards,
Markus

Re: [PATCH 2/4] coccinelle: platform_get_irq: handle 2-statement branches

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]



On Tue, 19 Nov 2019, Markus Elfring wrote:

> > From: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> I wonder about this information.
> Would you like to use the tag “Suggested-by” instead?

Sorry, I seem to have done something quite wrong on this patch.  I will
fix it.

>
>
> …
> > +++ b/scripts/coccinelle/api/platform_get_irq.cocci
> > @@ -31,6 +31,25 @@ if ( \( ret < 0 \| ret <= 0 \) )
> …
> > +ret =
> > +(
> > +platform_get_irq
> > +|
> > +platform_get_irq_byname
> > +)(E, ...);
> > +
> > +if ( \( ret < 0 \| ret <= 0 \) )
> > +-{
> > +-dev_err(...);
> > +S
> > +-}
>
> How do you think about to use the following SmPL code variant?

And the benefit is what?

julia

> + ret =
> +(platform_get_irq
> +|platform_get_irq_byname
> +)(E, ...);
> +
> + if ( \( ret < 0 \| ret <= 0 \) )
> +-{
> +-dev_err(...);
> + S
> +-}
>
> Regards,
> Markus
>

Re: [2/4] coccinelle: platform_get_irq: handle 2-statement branches

[Markus Elfring]

> Sorry, I seem to have done something quite wrong on this patch.

Interesting …


> I will fix it.

Thanks.

Development will be continued:
https://lkml.org/lkml/2019/11/19/1681
https://lore.kernel.org/patchwork/patch/1156089/
https://lore.kernel.org/cocci/1574197705-31132-3-git-send-email-Julia.Lawall@lip6.fr/


>> How do you think about to use the following SmPL code variant?
>
> And the benefit is what?
…
>> + ret =
>> +(platform_get_irq
>> +|platform_get_irq_byname
>> +)(E, ...);
>> +
>> + if ( \( ret < 0 \| ret <= 0 \) )
>> +-{
>> +-dev_err(...);
>> + S
>> +-}

* I suggest to use a different coding style for the specification of
  two function names in the SmPL disjunction.

* Would you like to avoid the mixing of code items in the first text column?

Regards,
Markus

Re: [PATCH 2/4] coccinelle: platform_get_irq: handle 2-statement branches

[Markus Elfring]

> From: Masahiro Yamada <yamada.masahiro@socionext.com>

I wonder about this information.
Would you like to use the tag “Suggested-by” instead?


…
> +++ b/scripts/coccinelle/api/platform_get_irq.cocci
> @@ -31,6 +31,25 @@ if ( \( ret < 0 \| ret <= 0 \) )
…
> +ret =
> +(
> +platform_get_irq
> +|
> +platform_get_irq_byname
> +)(E, ...);
> +
> +if ( \( ret < 0 \| ret <= 0 \) )
> +-{
> +-dev_err(...);
> +S
> +-}

How do you think about to use the following SmPL code variant?

+ ret =
+(platform_get_irq
+|platform_get_irq_byname
+)(E, ...);
+
+ if ( \( ret < 0 \| ret <= 0 \) )
+-{
+-dev_err(...);
+ S
+-}

Regards,
Markus

[PATCH 3/4] coccinelle: platform_get_irq: allow multiple dev_errs

[Julia Lawall]

Allow multiple calls to dev_err in the if branch under the
call to the platform_get_irq function.  Remove only the
first one.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 scripts/coccinelle/api/platform_get_irq.cocci |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci
index 7ac32ee..e5d04fb 100644
--- a/scripts/coccinelle/api/platform_get_irq.cocci
+++ b/scripts/coccinelle/api/platform_get_irq.cocci
@@ -70,7 +70,7 @@ if ( \( ret < 0 \| ret <= 0 \) )
 ...
 -dev_err(...);
 )
-...
+... when any
 }
 
 @r depends on org || report@

[PATCH 4/4] coccinelle: platform_get_irq: drop unneeded metavariable

[Julia Lawall]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

Having E as a metavariable adds the requirement to verify its type.
This seems like an unnecessary constraint, so drop it.
There is currently no impact on the set of results.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 scripts/coccinelle/api/platform_get_irq.cocci |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci
index e5d04fb..17e5895 100644
--- a/scripts/coccinelle/api/platform_get_irq.cocci
+++ b/scripts/coccinelle/api/platform_get_irq.cocci
@@ -11,7 +11,6 @@ virtual report
 
 @depends on context@
 expression ret;
-struct platform_device *E;
 @@
 
 ret =
@@ -19,7 +18,7 @@ ret =
 platform_get_irq
 |
 platform_get_irq_byname
-)(E, ...);
+)(...);
 
 if ( \( ret < 0 \| ret <= 0 \) )
 {
@@ -30,7 +29,6 @@ if ( \( ret < 0 \| ret <= 0 \) )
 
 @depends on patch@
 expression ret;
-struct platform_device *E;
 statement S;
 @@
 
@@ -39,7 +37,7 @@ ret =
 platform_get_irq
 |
 platform_get_irq_byname
-)(E, ...);
+)(...);
 
 if ( \( ret < 0 \| ret <= 0 \) )
 -{
@@ -49,7 +47,6 @@ S
 
 @depends on patch@
 expression ret;
-struct platform_device *E;
 @@
 
 ret =
@@ -57,7 +54,7 @@ ret =
 platform_get_irq
 |
 platform_get_irq_byname
-)(E, ...);
+)(...);
 
 if ( \( ret < 0 \| ret <= 0 \) )
 {
@@ -76,7 +73,6 @@ if ( \( ret < 0 \| ret <= 0 \) )
 @r depends on org || report@
 position p1;
 expression ret;
-struct platform_device *E;
 @@
 
 ret =
@@ -84,7 +80,7 @@ ret =
 platform_get_irq
 |
 platform_get_irq_byname
-)(E, ...);
+)(...);
 
 if ( \( ret < 0 \| ret <= 0 \) )
 {