2024-04-15 21:15:27

by Ricardo Ribalda

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

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 <[email protected]>
---
Following discussion at:
https://lore.kernel.org/linux-media/[email protected]/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 <[email protected]>



2024-04-16 08:47:52

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns



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 <[email protected]>

I agree. Thanks for the patch.

julia

> ---
> Following discussion at:
> https://lore.kernel.org/linux-media/[email protected]/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 <[email protected]>
>
>

2024-04-16 11:31:12

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns


> +++ 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

2024-04-16 11:33:17

by Ricardo Ribalda

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns

Hi Markus

On Tue, 16 Apr 2024 at 13:30, Markus Elfring <[email protected]> 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

2024-04-16 11:50:39

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns

> 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

2024-04-17 11:27:07

by Ricardo Ribalda

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: minmax: Suppress reports for err returns

Hi "Markus"

On Tue, 16 Apr 2024 at 13:50, Markus Elfring <[email protected]> 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

2024-04-17 13:04:51

by Markus Elfring

[permalink] [raw]
Subject: Re: coccinelle: misc: minmax: Suppress reports for err returns

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

2024-04-18 20:55:41

by Julia Lawall

[permalink] [raw]
Subject: Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns



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 <[email protected]>

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

julia

> ---
> Following discussion at:
> https://lore.kernel.org/linux-media/[email protected]/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 <[email protected]>
>
>

2024-04-19 06:51:40

by Markus Elfring

[permalink] [raw]
Subject: Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns

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

2024-04-19 07:01:57

by Julia Lawall

[permalink] [raw]
Subject: Re: [cocci] [PATCH] coccinelle: misc: minmax: Suppress reports for err returns



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
>