[PATCH] coccinelle: misc: minmax: Suppress reports for err returns

Ricardo Ribalda posted 1 patch 1 year, 9 months ago
scripts/coccinelle/misc/minmax.cocci | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
[PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Ricardo Ribalda 1 year, 9 months ago
Most of the people prefer:

return ret < 0 ? ret: 0;

than:

return min(ret, 0);

Let's tweak the cocci file to ignore those lines completely.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Following discussion at:
https://lore.kernel.org/linux-media/20240415203304.GA3460978@ragnatech.se/T/#m4dce34572312bd8a02542d798f21af7e4fc05fe8
---
 scripts/coccinelle/misc/minmax.cocci | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/scripts/coccinelle/misc/minmax.cocci b/scripts/coccinelle/misc/minmax.cocci
index fcf908b34f27..ca4830ae3042 100644
--- a/scripts/coccinelle/misc/minmax.cocci
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -50,11 +50,26 @@ func(...)
 	...>
 }
 
+// Ignore errcode returns.
+@errcode@
+position p;
+identifier func;
+expression x;
+binary operator cmp = {<, <=};
+@@
+
+func(...)
+{
+	<...
+	return ((x) cmp@p 0 ? (x) : 0);
+	...>
+}
+
 @rmin depends on !patch@
 identifier func;
 expression x, y;
 binary operator cmp = {<, <=};
-position p;
+position p != errcode.p;
 @@
 
 func(...)
@@ -116,21 +131,6 @@ func(...)
 	...>
 }
 
-// Don't generate patches for errcode returns.
-@errcode depends on patch@
-position p;
-identifier func;
-expression x;
-binary operator cmp = {<, <=};
-@@
-
-func(...)
-{
-	<...
-	return ((x) cmp@p 0 ? (x) : 0);
-	...>
-}
-
 @pmin depends on patch@
 identifier func;
 expression x, y;

---
base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
change-id: 20240415-minimax-1e9110d4697b

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>
Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Julia Lawall 1 year, 9 months ago

On Mon, 15 Apr 2024, Ricardo Ribalda wrote:

> Most of the people prefer:
>
> return ret < 0 ? ret: 0;
>
> than:
>
> return min(ret, 0);
>
> Let's tweak the cocci file to ignore those lines completely.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Applied, thanks. (Coccinelle for-6.10 branch).

julia

> ---
> Following discussion at:
> https://lore.kernel.org/linux-media/20240415203304.GA3460978@ragnatech.se/T/#m4dce34572312bd8a02542d798f21af7e4fc05fe8
> ---
>  scripts/coccinelle/misc/minmax.cocci | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci b/scripts/coccinelle/misc/minmax.cocci
> index fcf908b34f27..ca4830ae3042 100644
> --- a/scripts/coccinelle/misc/minmax.cocci
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -50,11 +50,26 @@ func(...)
>  	...>
>  }
>
> +// Ignore errcode returns.
> +@errcode@
> +position p;
> +identifier func;
> +expression x;
> +binary operator cmp = {<, <=};
> +@@
> +
> +func(...)
> +{
> +	<...
> +	return ((x) cmp@p 0 ? (x) : 0);
> +	...>
> +}
> +
>  @rmin depends on !patch@
>  identifier func;
>  expression x, y;
>  binary operator cmp = {<, <=};
> -position p;
> +position p != errcode.p;
>  @@
>
>  func(...)
> @@ -116,21 +131,6 @@ func(...)
>  	...>
>  }
>
> -// Don't generate patches for errcode returns.
> -@errcode depends on patch@
> -position p;
> -identifier func;
> -expression x;
> -binary operator cmp = {<, <=};
> -@@
> -
> -func(...)
> -{
> -	<...
> -	return ((x) cmp@p 0 ? (x) : 0);
> -	...>
> -}
> -
>  @pmin depends on patch@
>  identifier func;
>  expression x, y;
>
> ---
> base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> change-id: 20240415-minimax-1e9110d4697b
>
> Best regards,
> --
> Ricardo Ribalda <ribalda@chromium.org>
>
>
Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Markus Elfring 1 year, 9 months ago
>> Most of the people prefer:
>>
>> return ret < 0 ? ret: 0;
>>
>> than:
>>
>> return min(ret, 0);
>>
>> Let's tweak the cocci file to ignore those lines completely.
…
> Applied, thanks. (Coccinelle for-6.10 branch).

Was a planned code adjustment published?


…
>> +++ b/scripts/coccinelle/misc/minmax.cocci
>> @@ -50,11 +50,26 @@ func(...)
>>  	...>
>>  }
>>
>> +// Ignore errcode returns.
>> +@errcode@
…
>> -// Don't generate patches for errcode returns.
>> -@errcode depends on patch@
…

How does such a change fit to the usability of the coccicheck operation modes
“context” and “org”?

Should dependencies be reconsidered any more for the desired consistency
of involved rules for scripts of the semantic patch language?

Regards,
Markus
Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Julia Lawall 1 year, 9 months ago

On Fri, 19 Apr 2024, Markus Elfring wrote:

> >> Most of the people prefer:
> >>
> >> return ret < 0 ? ret: 0;
> >>
> >> than:
> >>
> >> return min(ret, 0);
> >>
> >> Let's tweak the cocci file to ignore those lines completely.
> …
> > Applied, thanks. (Coccinelle for-6.10 branch).
>
> Was a planned code adjustment published?

There is no "planned code adjustment" if there is no patch.

I can check the dependencies again.

julia

>
>
> …
> >> +++ b/scripts/coccinelle/misc/minmax.cocci
> >> @@ -50,11 +50,26 @@ func(...)
> >>  	...>
> >>  }
> >>
> >> +// Ignore errcode returns.
> >> +@errcode@
> …
> >> -// Don't generate patches for errcode returns.
> >> -@errcode depends on patch@
> …
>
> How does such a change fit to the usability of the coccicheck operation modes
> “context” and “org”?
>
> Should dependencies be reconsidered any more for the desired consistency
> of involved rules for scripts of the semantic patch language?
>
> Regards,
> Markus
>
Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Markus Elfring 1 year, 9 months ago
…
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -50,11 +50,26 @@ func(...)
>  	...>
>  }
>
> +// Ignore errcode returns.
> +@errcode@
…

I see that you would like to omit the specification “depends on patch”
from the previous SmPL rule location.

Would you really like to influence and adjust SmPL code any more
according to affected coccicheck operation modes?

Regards,
Markus
Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Ricardo Ribalda 1 year, 9 months ago
Hi Markus

On Tue, 16 Apr 2024 at 13:30, Markus Elfring <Markus.Elfring@web.de> wrote:
>
> …
> > +++ b/scripts/coccinelle/misc/minmax.cocci
> > @@ -50,11 +50,26 @@ func(...)
> >       ...>
> >  }
> >
> > +// Ignore errcode returns.
> > +@errcode@
> …
>
> I see that you would like to omit the specification “depends on patch”
> from the previous SmPL rule location.
>
> Would you really like to influence and adjust SmPL code any more
> according to affected coccicheck operation modes?

I probably do not know what I am doing :), it is my first .cocci patch.

If I leave the "depends on patch", then the change is ignored in report mode.

I think errcode needs to be executed in report and in patch mode, but
there might be a better way to do it.

>
> Regards,
> Markus



-- 
Ricardo Ribalda
Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Markus Elfring 1 year, 9 months ago
> I think errcode needs to be executed in report and in patch mode,

Adjustments for functionality of coccicheck operation modes can be clarified further.


> but there might be a better way to do it.

Corresponding design options depend on varying development efforts and resources.

Regards,
Markus
Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Ricardo Ribalda 1 year, 9 months ago
Hi "Markus"

On Tue, 16 Apr 2024 at 13:50, Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > I think errcode needs to be executed in report and in patch mode,
>
> Adjustments for functionality of coccicheck operation modes can be clarified further.

https://lore.kernel.org/lkml/2024041643-unshaven-happiest-1405@gregkh/


>
>
> > but there might be a better way to do it.
>
> Corresponding design options depend on varying development efforts and resources.
>
> Regards,
> Markus



-- 
Ricardo Ribalda
Re: coccinelle: misc: minmax: Suppress reports for err returns
Posted by Markus Elfring 1 year, 9 months ago
>> Adjustments for functionality of coccicheck operation modes can be clarified further.
>
> https://lore.kernel.org/lkml/2024041643-unshaven-happiest-1405@gregkh/

I hope that remaining communication difficulties can be resolved in more constructive ways.


Do you get further development ideas from previous contributions on presented topics?

Regards,
Markus
Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns
Posted by Julia Lawall 1 year, 9 months ago

On Mon, 15 Apr 2024, Ricardo Ribalda wrote:

> Most of the people prefer:
>
> return ret < 0 ? ret: 0;
>
> than:
>
> return min(ret, 0);
>
> Let's tweak the cocci file to ignore those lines completely.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

I agree.  Thanks for the patch.

julia

> ---
> Following discussion at:
> https://lore.kernel.org/linux-media/20240415203304.GA3460978@ragnatech.se/T/#m4dce34572312bd8a02542d798f21af7e4fc05fe8
> ---
>  scripts/coccinelle/misc/minmax.cocci | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/coccinelle/misc/minmax.cocci b/scripts/coccinelle/misc/minmax.cocci
> index fcf908b34f27..ca4830ae3042 100644
> --- a/scripts/coccinelle/misc/minmax.cocci
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -50,11 +50,26 @@ func(...)
>  	...>
>  }
>
> +// Ignore errcode returns.
> +@errcode@
> +position p;
> +identifier func;
> +expression x;
> +binary operator cmp = {<, <=};
> +@@
> +
> +func(...)
> +{
> +	<...
> +	return ((x) cmp@p 0 ? (x) : 0);
> +	...>
> +}
> +
>  @rmin depends on !patch@
>  identifier func;
>  expression x, y;
>  binary operator cmp = {<, <=};
> -position p;
> +position p != errcode.p;
>  @@
>
>  func(...)
> @@ -116,21 +131,6 @@ func(...)
>  	...>
>  }
>
> -// Don't generate patches for errcode returns.
> -@errcode depends on patch@
> -position p;
> -identifier func;
> -expression x;
> -binary operator cmp = {<, <=};
> -@@
> -
> -func(...)
> -{
> -	<...
> -	return ((x) cmp@p 0 ? (x) : 0);
> -	...>
> -}
> -
>  @pmin depends on patch@
>  identifier func;
>  expression x, y;
>
> ---
> base-commit: 71b3ed53b08d87212fbbe51bdc3bf44eb8c462f8
> change-id: 20240415-minimax-1e9110d4697b
>
> Best regards,
> --
> Ricardo Ribalda <ribalda@chromium.org>
>
>