Coccinelle Archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] [PATCH] coccinelle: misc: add swap script
@ 2021-02-16  8:01 Denis Efremov
  2021-02-16 18:05 ` Markus Elfring
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Denis Efremov @ 2021-02-16  8:01 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 scripts/coccinelle/misc/swap.cocci | 77 ++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 scripts/coccinelle/misc/swap.cocci

diff --git a/scripts/coccinelle/misc/swap.cocci b/scripts/coccinelle/misc/swap.cocci
new file mode 100644
index 000000000000..38227a5d0855
--- /dev/null
+++ b/scripts/coccinelle/misc/swap.cocci
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded swap() implementation.
+///
+// Confidence: High
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: swap
+//
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@r depends on !patch@
+identifier tmp;
+expression a, b;
+type T;
+position p;
+@@
+
+(
+* T tmp;
+|
+* T tmp = 0;
+|
+* T *tmp = NULL;
+)
+... when != tmp
+* tmp = a;
+* a = b;@p
+* b = tmp;
+... when != tmp
+
+@depends on patch@
+identifier tmp;
+expression a, b;
+type T;
+@@
+
+(
+- T tmp;
+|
+- T tmp = 0;
+|
+- T *tmp = NULL;
+)
+... when != tmp
+- tmp = a;
+- a = b;
+- b = tmp;
++ swap(a, b);
+... when != tmp
+
+@depends on patch@
+identifier tmp;
+expression a, b;
+@@
+
+- tmp = a;
+- a = b;
+- b = tmp;
++ swap(a, b);
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
-- 
2.26.2

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
@ 2021-02-16 18:05 ` Markus Elfring
  2021-02-17 21:31 ` Julia Lawall
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Markus Elfring @ 2021-02-16 18:05 UTC (permalink / raw)
  To: Denis Efremov, Gilles Muller, Julia Lawall, Michal Marek,
	Nicolas Palix, Coccinelle
  Cc: kernel-janitors, linux-kernel

> +- tmp = a;
> +- a = b;
> +- b = tmp;
> ++ swap(a, b);

How do you think about to use the following SmPL code variant?
* Omission of a few leading space characters
* Keeping a semicolon unmodified in a line

+-tmp = a;
+-a = b;
+-b = tmp
++swap(a, b)
+;


Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
  2021-02-16 18:05 ` Markus Elfring
@ 2021-02-17 21:31 ` Julia Lawall
  2021-02-18  6:13   ` Denis Efremov
  2021-02-19  9:05 ` [Cocci] [PATCH v2] coccinelle: misc: add minmax script Denis Efremov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Julia Lawall @ 2021-02-17 21:31 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel

> +@depends on patch@
> +identifier tmp;
> +expression a, b;
> +type T;
> +@@
> +
> +(
> +- T tmp;
> +|
> +- T tmp = 0;
> +|
> +- T *tmp = NULL;
> +)
> +... when != tmp
> +- tmp = a;
> +- a = b;
> +- b = tmp;
> ++ swap(a, b);
> +... when != tmp

In this rule and the next one, if you remove the final ; from the b = tmp
line and from the swap line, and put it into context code afterwards, them
the generated code looks better on cases like fs/xfs/xfs_inode.c in the
function xfs_lock_two_inodes where two successive swap calls are
generated.

There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
the function ath5k_hw_get_median_noise_floor where the swap code makes up
a whole if branch.  In this cases it would be good to remove the {}.

julia

> +
> +@depends on patch@
> +identifier tmp;
> +expression a, b;
> +@@
> +
> +- tmp = a;
> +- a = b;
> +- b = tmp;
> ++ swap(a, b);
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> --
> 2.26.2
>
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-17 21:31 ` Julia Lawall
@ 2021-02-18  6:13   ` Denis Efremov
  2021-02-18 10:17     ` Julia Lawall
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Efremov @ 2021-02-18  6:13 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel



On 2/18/21 12:31 AM, Julia Lawall wrote:
>> +@depends on patch@
>> +identifier tmp;
>> +expression a, b;
>> +type T;
>> +@@
>> +
>> +(
>> +- T tmp;
>> +|
>> +- T tmp = 0;
>> +|
>> +- T *tmp = NULL;
>> +)
>> +... when != tmp
>> +- tmp = a;
>> +- a = b;
>> +- b = tmp;
>> ++ swap(a, b);
>> +... when != tmp
> 
> In this rule and the next one, if you remove the final ; from the b = tmp
> line and from the swap line, and put it into context code afterwards, them
> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
> function xfs_lock_two_inodes where two successive swap calls are
> generated.
> 
> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
> the function ath5k_hw_get_median_noise_floor where the swap code makes up
> a whole if branch. 

> In this cases it would be good to remove the {}.

How this can be handled?

If I use this pattern:
@depends on patch@
identifier tmp;
expression a, b;
@@

(
  if (...)
- {
-       tmp = a;
-       a = b;
-       b = tmp
+       swap(a, b)
        ;
- }
|
-       tmp = a;
-       a = b;
-       b = tmp
+       swap(a, b)
        ;
)

The tool fails with error:

EXN: Failure("rule starting on line 58: already tagged token:\nC code context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574, column 4, charpos = 41650\n  around = 'sort',\n  whole content = \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c

Thanks,
Denis
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-18  6:13   ` Denis Efremov
@ 2021-02-18 10:17     ` Julia Lawall
  2021-02-18 11:03       ` Denis Efremov
       [not found]       ` <50eb8319-a552-c749-6143-7e24a8778a04@web.de>
  0 siblings, 2 replies; 15+ messages in thread
From: Julia Lawall @ 2021-02-18 10:17 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel



On Thu, 18 Feb 2021, Denis Efremov wrote:

>
>
> On 2/18/21 12:31 AM, Julia Lawall wrote:
> >> +@depends on patch@
> >> +identifier tmp;
> >> +expression a, b;
> >> +type T;
> >> +@@
> >> +
> >> +(
> >> +- T tmp;
> >> +|
> >> +- T tmp = 0;
> >> +|
> >> +- T *tmp = NULL;
> >> +)
> >> +... when != tmp
> >> +- tmp = a;
> >> +- a = b;
> >> +- b = tmp;
> >> ++ swap(a, b);
> >> +... when != tmp
> >
> > In this rule and the next one, if you remove the final ; from the b = tmp
> > line and from the swap line, and put it into context code afterwards, them
> > the generated code looks better on cases like fs/xfs/xfs_inode.c in the
> > function xfs_lock_two_inodes where two successive swap calls are
> > generated.
> >
> > There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
> > the function ath5k_hw_get_median_noise_floor where the swap code makes up
> > a whole if branch.
>
> > In this cases it would be good to remove the {}.
>
> How this can be handled?
>
> If I use this pattern:
> @depends on patch@
> identifier tmp;
> expression a, b;
> @@
>
> (
>   if (...)
> - {
> -       tmp = a;
> -       a = b;
> -       b = tmp
> +       swap(a, b)
>         ;
> - }
> |
> -       tmp = a;
> -       a = b;
> -       b = tmp
> +       swap(a, b)
>         ;
> )
>
> The tool fails with error:
>
> EXN: Failure("rule starting on line 58: already tagged token:\nC code
> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
> column 4, charpos = 41650\n around = 'sort',\n whole content =
> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c

A disjunction basically says "at this node in the cfg, can I match the
first patter, or can I match the second pattern, etc."  Unfortunately in
this case the two branches start matching at different nodes, so the short
circuit aspect of a disjunction isn't used, and it matches both patterns.

The solution is to just make two rules.  The first for the if case and the
second for everything else.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-18 10:17     ` Julia Lawall
@ 2021-02-18 11:03       ` Denis Efremov
  2021-02-18 11:29         ` Julia Lawall
       [not found]       ` <50eb8319-a552-c749-6143-7e24a8778a04@web.de>
  1 sibling, 1 reply; 15+ messages in thread
From: Denis Efremov @ 2021-02-18 11:03 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel



On 2/18/21 1:17 PM, Julia Lawall wrote:
> 
> 
> On Thu, 18 Feb 2021, Denis Efremov wrote:
> 
>>
>>
>> On 2/18/21 12:31 AM, Julia Lawall wrote:
>>>> +@depends on patch@
>>>> +identifier tmp;
>>>> +expression a, b;
>>>> +type T;
>>>> +@@
>>>> +
>>>> +(
>>>> +- T tmp;
>>>> +|
>>>> +- T tmp = 0;
>>>> +|
>>>> +- T *tmp = NULL;
>>>> +)
>>>> +... when != tmp
>>>> +- tmp = a;
>>>> +- a = b;
>>>> +- b = tmp;
>>>> ++ swap(a, b);
>>>> +... when != tmp
>>>
>>> In this rule and the next one, if you remove the final ; from the b = tmp
>>> line and from the swap line, and put it into context code afterwards, them
>>> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
>>> function xfs_lock_two_inodes where two successive swap calls are
>>> generated.
>>>
>>> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
>>> the function ath5k_hw_get_median_noise_floor where the swap code makes up
>>> a whole if branch.
>>
>>> In this cases it would be good to remove the {}.
>>
>> How this can be handled?
>>
>> If I use this pattern:
>> @depends on patch@
>> identifier tmp;
>> expression a, b;
>> @@
>>
>> (
>>   if (...)
>> - {
>> -       tmp = a;
>> -       a = b;
>> -       b = tmp
>> +       swap(a, b)
>>         ;
>> - }
>> |
>> -       tmp = a;
>> -       a = b;
>> -       b = tmp
>> +       swap(a, b)
>>         ;
>> )
>>
>> The tool fails with error:
>>
>> EXN: Failure("rule starting on line 58: already tagged token:\nC code
>> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
>> column 4, charpos = 41650\n around = 'sort',\n whole content =
>> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
> 
> A disjunction basically says "at this node in the cfg, can I match the
> first patter, or can I match the second pattern, etc."  Unfortunately in
> this case the two branches start matching at different nodes, so the short
> circuit aspect of a disjunction isn't used, and it matches both patterns.
> 
> The solution is to just make two rules.  The first for the if case and the
> second for everything else.
> 

  if (...)
- {
-       tmp = a;
-       a = b;
-       b = tmp
+       swap(a, b)
        ;
- }


This produces "single-line ifs".
Maybe generated patches can be improved somehow?
Moving -+; doesn't help in this case.

diff -u -p a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
--- a/drivers/net/wireless/ath/ath9k/calib.c
+++ b/drivers/net/wireless/ath/ath9k/calib.c
@@ -32,11 +32,7 @@ static int16_t ath9k_hw_get_nf_hist_mid(
 
        for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
                for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
-                       if (sort[j] > sort[j - 1]) {
-                               nfval = sort[j];
-                               sort[j] = sort[j - 1];
-                               sort[j - 1] = nfval;
-                       }
+                       if (sort[j] > sort[j - 1]) swap(sort[j], sort[j - 1]);
                }
        }
        nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
diff -u -p a/drivers/net/wireless/ath/ath9k/ar9003_calib.c b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
--- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
@@ -1011,19 +1011,11 @@ static void __ar955x_tx_iq_cal_sort(stru
                for (ix = 0; ix < MAXIQCAL - 1; ix++) {
                        for (iy = ix + 1; iy <= MAXIQCAL - 1; iy++) {
                                if (coeff->mag_coeff[i][im][iy] <
-                                   coeff->mag_coeff[i][im][ix]) {
-                                       temp = coeff->mag_coeff[i][im][ix];
-                                       coeff->mag_coeff[i][im][ix] =
-                                               coeff->mag_coeff[i][im][iy];
-                                       coeff->mag_coeff[i][im][iy] = temp;
-                               }
+                                   coeff->mag_coeff[i][im][ix]) swap(coeff->mag_coeff[i][im][ix],
+                                                                     coeff->mag_coeff[i][im][iy]);
                                if (coeff->phs_coeff[i][im][iy] <
-                                   coeff->phs_coeff[i][im][ix]) {
-                                       temp = coeff->phs_coeff[i][im][ix];
-                                       coeff->phs_coeff[i][im][ix] =
-                                               coeff->phs_coeff[i][im][iy];
-                                       coeff->phs_coeff[i][im][iy] = temp;
-                               }
+                                   coeff->phs_coeff[i][im][ix]) swap(coeff->phs_coeff[i][im][ix],
+                                                                     coeff->phs_coeff[i][im][iy]);

Thanks,
Denis
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-18 11:03       ` Denis Efremov
@ 2021-02-18 11:29         ` Julia Lawall
  2021-02-18 11:34           ` Denis Efremov
  0 siblings, 1 reply; 15+ messages in thread
From: Julia Lawall @ 2021-02-18 11:29 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel



On Thu, 18 Feb 2021, Denis Efremov wrote:

>
>
> On 2/18/21 1:17 PM, Julia Lawall wrote:
> >
> >
> > On Thu, 18 Feb 2021, Denis Efremov wrote:
> >
> >>
> >>
> >> On 2/18/21 12:31 AM, Julia Lawall wrote:
> >>>> +@depends on patch@
> >>>> +identifier tmp;
> >>>> +expression a, b;
> >>>> +type T;
> >>>> +@@
> >>>> +
> >>>> +(
> >>>> +- T tmp;
> >>>> +|
> >>>> +- T tmp = 0;
> >>>> +|
> >>>> +- T *tmp = NULL;
> >>>> +)
> >>>> +... when != tmp
> >>>> +- tmp = a;
> >>>> +- a = b;
> >>>> +- b = tmp;
> >>>> ++ swap(a, b);
> >>>> +... when != tmp
> >>>
> >>> In this rule and the next one, if you remove the final ; from the b = tmp
> >>> line and from the swap line, and put it into context code afterwards, them
> >>> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
> >>> function xfs_lock_two_inodes where two successive swap calls are
> >>> generated.
> >>>
> >>> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
> >>> the function ath5k_hw_get_median_noise_floor where the swap code makes up
> >>> a whole if branch.
> >>
> >>> In this cases it would be good to remove the {}.
> >>
> >> How this can be handled?
> >>
> >> If I use this pattern:
> >> @depends on patch@
> >> identifier tmp;
> >> expression a, b;
> >> @@
> >>
> >> (
> >>   if (...)
> >> - {
> >> -       tmp = a;
> >> -       a = b;
> >> -       b = tmp
> >> +       swap(a, b)
> >>         ;
> >> - }
> >> |
> >> -       tmp = a;
> >> -       a = b;
> >> -       b = tmp
> >> +       swap(a, b)
> >>         ;
> >> )
> >>
> >> The tool fails with error:
> >>
> >> EXN: Failure("rule starting on line 58: already tagged token:\nC code
> >> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
> >> column 4, charpos = 41650\n around = 'sort',\n whole content =
> >> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
> >
> > A disjunction basically says "at this node in the cfg, can I match the
> > first patter, or can I match the second pattern, etc."  Unfortunately in
> > this case the two branches start matching at different nodes, so the short
> > circuit aspect of a disjunction isn't used, and it matches both patterns.
> >
> > The solution is to just make two rules.  The first for the if case and the
> > second for everything else.
> >
>
>   if (...)
> - {
> -       tmp = a;
> -       a = b;
> -       b = tmp
> +       swap(a, b)
>         ;
> - }
>
>
> This produces "single-line ifs".
> Maybe generated patches can be improved somehow?
> Moving -+; doesn't help in this case.

There is clearly some problem with the management of newlines...

The other alternative is to just have one rule for introducing swap and
another for removing the braces around a swap, ie

if (...)
- {
  swap(...);
- }

I don't think it would be motivated to remove the newline in that case.

julia

>
> diff -u -p a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
> --- a/drivers/net/wireless/ath/ath9k/calib.c
> +++ b/drivers/net/wireless/ath/ath9k/calib.c
> @@ -32,11 +32,7 @@ static int16_t ath9k_hw_get_nf_hist_mid(
>
>         for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
>                 for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
> -                       if (sort[j] > sort[j - 1]) {
> -                               nfval = sort[j];
> -                               sort[j] = sort[j - 1];
> -                               sort[j - 1] = nfval;
> -                       }
> +                       if (sort[j] > sort[j - 1]) swap(sort[j], sort[j - 1]);
>                 }
>         }
>         nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
> diff -u -p a/drivers/net/wireless/ath/ath9k/ar9003_calib.c b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> @@ -1011,19 +1011,11 @@ static void __ar955x_tx_iq_cal_sort(stru
>                 for (ix = 0; ix < MAXIQCAL - 1; ix++) {
>                         for (iy = ix + 1; iy <= MAXIQCAL - 1; iy++) {
>                                 if (coeff->mag_coeff[i][im][iy] <
> -                                   coeff->mag_coeff[i][im][ix]) {
> -                                       temp = coeff->mag_coeff[i][im][ix];
> -                                       coeff->mag_coeff[i][im][ix] =
> -                                               coeff->mag_coeff[i][im][iy];
> -                                       coeff->mag_coeff[i][im][iy] = temp;
> -                               }
> +                                   coeff->mag_coeff[i][im][ix]) swap(coeff->mag_coeff[i][im][ix],
> +                                                                     coeff->mag_coeff[i][im][iy]);
>                                 if (coeff->phs_coeff[i][im][iy] <
> -                                   coeff->phs_coeff[i][im][ix]) {
> -                                       temp = coeff->phs_coeff[i][im][ix];
> -                                       coeff->phs_coeff[i][im][ix] =
> -                                               coeff->phs_coeff[i][im][iy];
> -                                       coeff->phs_coeff[i][im][iy] = temp;
> -                               }
> +                                   coeff->phs_coeff[i][im][ix]) swap(coeff->phs_coeff[i][im][ix],
> +                                                                     coeff->phs_coeff[i][im][iy]);
>
> Thanks,
> Denis
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
  2021-02-18 11:29         ` Julia Lawall
@ 2021-02-18 11:34           ` Denis Efremov
  0 siblings, 0 replies; 15+ messages in thread
From: Denis Efremov @ 2021-02-18 11:34 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel



On 2/18/21 2:29 PM, Julia Lawall wrote:
> 
> 
> On Thu, 18 Feb 2021, Denis Efremov wrote:
> 
>>
>>
>> On 2/18/21 1:17 PM, Julia Lawall wrote:
>>>
>>>
>>> On Thu, 18 Feb 2021, Denis Efremov wrote:
>>>
>>>>
>>>>
>>>> On 2/18/21 12:31 AM, Julia Lawall wrote:
>>>>>> +@depends on patch@
>>>>>> +identifier tmp;
>>>>>> +expression a, b;
>>>>>> +type T;
>>>>>> +@@
>>>>>> +
>>>>>> +(
>>>>>> +- T tmp;
>>>>>> +|
>>>>>> +- T tmp = 0;
>>>>>> +|
>>>>>> +- T *tmp = NULL;
>>>>>> +)
>>>>>> +... when != tmp
>>>>>> +- tmp = a;
>>>>>> +- a = b;
>>>>>> +- b = tmp;
>>>>>> ++ swap(a, b);
>>>>>> +... when != tmp
>>>>>
>>>>> In this rule and the next one, if you remove the final ; from the b = tmp
>>>>> line and from the swap line, and put it into context code afterwards, them
>>>>> the generated code looks better on cases like fs/xfs/xfs_inode.c in the
>>>>> function xfs_lock_two_inodes where two successive swap calls are
>>>>> generated.
>>>>>
>>>>> There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in
>>>>> the function ath5k_hw_get_median_noise_floor where the swap code makes up
>>>>> a whole if branch.
>>>>
>>>>> In this cases it would be good to remove the {}.
>>>>
>>>> How this can be handled?
>>>>
>>>> If I use this pattern:
>>>> @depends on patch@
>>>> identifier tmp;
>>>> expression a, b;
>>>> @@
>>>>
>>>> (
>>>>   if (...)
>>>> - {
>>>> -       tmp = a;
>>>> -       a = b;
>>>> -       b = tmp
>>>> +       swap(a, b)
>>>>         ;
>>>> - }
>>>> |
>>>> -       tmp = a;
>>>> -       a = b;
>>>> -       b = tmp
>>>> +       swap(a, b)
>>>>         ;
>>>> )
>>>>
>>>> The tool fails with error:
>>>>
>>>> EXN: Failure("rule starting on line 58: already tagged token:\nC code
>>>> context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574,
>>>> column 4, charpos = 41650\n around = 'sort',\n whole content =
>>>> \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
>>>
>>> A disjunction basically says "at this node in the cfg, can I match the
>>> first patter, or can I match the second pattern, etc."  Unfortunately in
>>> this case the two branches start matching at different nodes, so the short
>>> circuit aspect of a disjunction isn't used, and it matches both patterns.
>>>
>>> The solution is to just make two rules.  The first for the if case and the
>>> second for everything else.
>>>
>>
>>   if (...)
>> - {
>> -       tmp = a;
>> -       a = b;
>> -       b = tmp
>> +       swap(a, b)
>>         ;
>> - }
>>
>>
>> This produces "single-line ifs".
>> Maybe generated patches can be improved somehow?
>> Moving -+; doesn't help in this case.
> 
> There is clearly some problem with the management of newlines...
> 
> The other alternative is to just have one rule for introducing swap and
> another for removing the braces around a swap, ie
> 
> if (...)
> - {
>   swap(...);
> - }
> 
> I don't think it would be motivated to remove the newline in that case.

Yes, this works. I'll send v2.

Thanks
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: misc: add swap script
       [not found]       ` <50eb8319-a552-c749-6143-7e24a8778a04@web.de>
@ 2021-02-18 20:03         ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2021-02-18 20:03 UTC (permalink / raw)
  To: Markus Elfring; +Cc: kernel-janitors, cocci, linux-kernel



On Thu, 18 Feb 2021, Markus Elfring wrote:

> > A disjunction basically says "at this node in the cfg, can I match the
> > first patter, or can I match the second pattern, etc."  Unfortunately in
> > this case the two branches start matching at different nodes, so the short
> > circuit aspect of a disjunction isn't used, and it matches both patterns.
> >
> > The solution is to just make two rules.  The first for the if case and the
> > second for everything else.
>
> Will such feedback trigger further software development considerations?

No.  This is never going to change, until someone completely redesigns
Coccinelle.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cocci] [PATCH v2] coccinelle: misc: add minmax script
  2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
  2021-02-16 18:05 ` Markus Elfring
  2021-02-17 21:31 ` Julia Lawall
@ 2021-02-19  9:05 ` Denis Efremov
  2021-02-19  9:07   ` Denis Efremov
  2021-02-19  9:24 ` [Cocci] [PATCH v2] coccinelle: misc: add swap script Denis Efremov
  2021-03-05 10:09 ` [Cocci] [PATCH v3] " Denis Efremov
  4 siblings, 1 reply; 15+ messages in thread
From: Denis Efremov @ 2021-02-19  9:05 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel

Check for opencoded min(), max() implementations.

Signed-off-by: Denis Efremov <efremov@linux.com>
---

Changes in v2:
 - <... ...> instead of ... when any
 - org mode reports fixed
 - patch rule to drop excessive ()

 scripts/coccinelle/misc/minmax.cocci | 224 +++++++++++++++++++++++++++
 1 file changed, 224 insertions(+)
 create mode 100644 scripts/coccinelle/misc/minmax.cocci

diff --git a/scripts/coccinelle/misc/minmax.cocci b/scripts/coccinelle/misc/minmax.cocci
new file mode 100644
index 000000000000..61d6b61fd82c
--- /dev/null
+++ b/scripts/coccinelle/misc/minmax.cocci
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded min(), max() implementations.
+/// Generated patches sometimes require adding a cast to fix compile warning.
+/// Warnings/patches scope intentionally limited to a function body.
+///
+// Confidence: Medium
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: min, max
+//
+
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@rmax depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+	<...
+*	x cmp@p y ? x : y
+	...>
+}
+
+@rmaxif depends on !patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>, >=};
+position p;
+@@
+
+func(...)
+{
+	<...
+*	if (x cmp@p y) {
+*		max_val = x;
+*	} else {
+*		max_val = y;
+*	}
+	...>
+}
+
+@rmin depends on !patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+	<...
+*	x cmp@p y ? x : y
+	...>
+}
+
+@rminif depends on !patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<, <=};
+position p;
+@@
+
+func(...)
+{
+	<...
+*	if (x cmp@p y) {
+*		min_val = x;
+*	} else {
+*		min_val = y;
+*	}
+	...>
+}
+
+@pmax depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+	<...
+-	x cmp y ? x : y
++	max(x, y)
+	...>
+}
+
+@pmaxif depends on patch@
+identifier func;
+expression x, y;
+expression max_val;
+binary operator cmp = {>=, >};
+position p;
+@@
+
+func@p(...)
+{
+	<...
+-	if (x cmp y) {
+-		max_val = x;
+-	} else {
+-		max_val = y;
+-	}
++	max_val = max(x, y);
+	...>
+}
+
+@pmin depends on patch@
+identifier func;
+expression x, y;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+	<...
+-	x cmp y ? x : y
++	min(x, y)
+	...>
+}
+
+@pminif depends on patch@
+identifier func;
+expression x, y;
+expression min_val;
+binary operator cmp = {<=, <};
+position p;
+@@
+
+func@p(...)
+{
+	<...
+-	if (x cmp y) {
+-		min_val = x;
+-	} else {
+-		min_val = y;
+-	}
++	min_val = min(x, y);
+	...>
+}
+
+@depends on (pmax || pmaxif || pmin || pminif)@
+identifier func;
+expression x, y;
+position p;
+// FIXME: Coccinelle consumes all available ram and
+// and timeouts on every file.
+// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
+@@
+
+func@p(...)
+{
+	<...
+(
+-	(min((x), (y)))
++	min(x, y)
+|
+-	(max((x), (y)))
++	max(x, y)
+)
+	...>
+}
+
+@script:python depends on report@
+p << rmax.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmax.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmaxif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for max()")
+
+@script:python depends on org@
+p << rmaxif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
+
+@script:python depends on report@
+p << rmin.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rmin.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
+
+@script:python depends on report@
+p << rminif.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for min()")
+
+@script:python depends on org@
+p << rminif.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
-- 
2.26.2

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH v2] coccinelle: misc: add minmax script
  2021-02-19  9:05 ` [Cocci] [PATCH v2] coccinelle: misc: add minmax script Denis Efremov
@ 2021-02-19  9:07   ` Denis Efremov
  0 siblings, 0 replies; 15+ messages in thread
From: Denis Efremov @ 2021-02-19  9:07 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel

Sorry for wrong thread, I'll resend v2 to the right one.

Denis

On 2/19/21 12:05 PM, Denis Efremov wrote:
> Check for opencoded min(), max() implementations.
> 
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> 
> Changes in v2:
>  - <... ...> instead of ... when any
>  - org mode reports fixed
>  - patch rule to drop excessive ()
> 
>  scripts/coccinelle/misc/minmax.cocci | 224 +++++++++++++++++++++++++++
>  1 file changed, 224 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/minmax.cocci
> 
> diff --git a/scripts/coccinelle/misc/minmax.cocci b/scripts/coccinelle/misc/minmax.cocci
> new file mode 100644
> index 000000000000..61d6b61fd82c
> --- /dev/null
> +++ b/scripts/coccinelle/misc/minmax.cocci
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded min(), max() implementations.
> +/// Generated patches sometimes require adding a cast to fix compile warning.
> +/// Warnings/patches scope intentionally limited to a function body.
> +///
> +// Confidence: Medium
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: min, max
> +//
> +
> +
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
> +
> +@rmax depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> +	<...
> +*	x cmp@p y ? x : y
> +	...>
> +}
> +
> +@rmaxif depends on !patch@
> +identifier func;
> +expression x, y;
> +expression max_val;
> +binary operator cmp = {>, >=};
> +position p;
> +@@
> +
> +func(...)
> +{
> +	<...
> +*	if (x cmp@p y) {
> +*		max_val = x;
> +*	} else {
> +*		max_val = y;
> +*	}
> +	...>
> +}
> +
> +@rmin depends on !patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {<, <=};
> +position p;
> +@@
> +
> +func(...)
> +{
> +	<...
> +*	x cmp@p y ? x : y
> +	...>
> +}
> +
> +@rminif depends on !patch@
> +identifier func;
> +expression x, y;
> +expression min_val;
> +binary operator cmp = {<, <=};
> +position p;
> +@@
> +
> +func(...)
> +{
> +	<...
> +*	if (x cmp@p y) {
> +*		min_val = x;
> +*	} else {
> +*		min_val = y;
> +*	}
> +	...>
> +}
> +
> +@pmax depends on patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {>=, >};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> +	<...
> +-	x cmp y ? x : y
> ++	max(x, y)
> +	...>
> +}
> +
> +@pmaxif depends on patch@
> +identifier func;
> +expression x, y;
> +expression max_val;
> +binary operator cmp = {>=, >};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> +	<...
> +-	if (x cmp y) {
> +-		max_val = x;
> +-	} else {
> +-		max_val = y;
> +-	}
> ++	max_val = max(x, y);
> +	...>
> +}
> +
> +@pmin depends on patch@
> +identifier func;
> +expression x, y;
> +binary operator cmp = {<=, <};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> +	<...
> +-	x cmp y ? x : y
> ++	min(x, y)
> +	...>
> +}
> +
> +@pminif depends on patch@
> +identifier func;
> +expression x, y;
> +expression min_val;
> +binary operator cmp = {<=, <};
> +position p;
> +@@
> +
> +func@p(...)
> +{
> +	<...
> +-	if (x cmp y) {
> +-		min_val = x;
> +-	} else {
> +-		min_val = y;
> +-	}
> ++	min_val = min(x, y);
> +	...>
> +}
> +
> +@depends on (pmax || pmaxif || pmin || pminif)@
> +identifier func;
> +expression x, y;
> +position p;
> +// FIXME: Coccinelle consumes all available ram and
> +// and timeouts on every file.
> +// position p = { pmin.p, pminif.p, pmax.p, pmaxif.p };
> +@@
> +
> +func@p(...)
> +{
> +	<...
> +(
> +-	(min((x), (y)))
> ++	min(x, y)
> +|
> +-	(max((x), (y)))
> ++	max(x, y)
> +)
> +	...>
> +}
> +
> +@script:python depends on report@
> +p << rmax.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on org@
> +p << rmax.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on report@
> +p << rmaxif.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on org@
> +p << rmaxif.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for max()")
> +
> +@script:python depends on report@
> +p << rmin.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for min()")
> +
> +@script:python depends on org@
> +p << rmin.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
> +
> +@script:python depends on report@
> +p << rminif.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for min()")
> +
> +@script:python depends on org@
> +p << rminif.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for min()")
> 
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cocci] [PATCH v2] coccinelle: misc: add swap script
  2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
                   ` (2 preceding siblings ...)
  2021-02-19  9:05 ` [Cocci] [PATCH v2] coccinelle: misc: add minmax script Denis Efremov
@ 2021-02-19  9:24 ` Denis Efremov
  2021-03-03 16:37   ` Julia Lawall
  2021-03-05 10:09 ` [Cocci] [PATCH v3] " Denis Efremov
  4 siblings, 1 reply; 15+ messages in thread
From: Denis Efremov @ 2021-02-19  9:24 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
Changes in v2:
 - additional patch rule to drop excessive {}
 - fix indentation in patch mode by anchoring ;

 scripts/coccinelle/misc/swap.cocci | 101 +++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 scripts/coccinelle/misc/swap.cocci

diff --git a/scripts/coccinelle/misc/swap.cocci b/scripts/coccinelle/misc/swap.cocci
new file mode 100644
index 000000000000..d5da9888c222
--- /dev/null
+++ b/scripts/coccinelle/misc/swap.cocci
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded swap() implementation.
+///
+// Confidence: High
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: swap
+//
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@r depends on !patch@
+identifier tmp;
+expression a, b;
+type T;
+position p;
+@@
+
+(
+* T tmp;
+|
+* T tmp = 0;
+|
+* T *tmp = NULL;
+)
+... when != tmp
+* tmp = a;
+* a = b;@p
+* b = tmp;
+... when != tmp
+
+@rpvar depends on patch@
+identifier tmp;
+expression a, b;
+type T;
+@@
+
+(
+- T tmp;
+|
+- T tmp = 0;
+|
+- T *tmp = NULL;
+)
+... when != tmp
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+  ;
+... when != tmp
+
+
+@rp depends on patch@
+identifier tmp;
+expression a, b;
+@@
+
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+  ;
+
+@depends on (rpvar || rp)@
+@@
+
+(
+  for (...;...;...)
+- {
+	swap(...);
+- }
+|
+  while (...)
+- {
+	swap(...);
+- }
+|
+  if (...)
+- {
+	swap(...);
+- }
+)
+
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
-- 
2.26.2

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH v2] coccinelle: misc: add swap script
  2021-02-19  9:24 ` [Cocci] [PATCH v2] coccinelle: misc: add swap script Denis Efremov
@ 2021-03-03 16:37   ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2021-03-03 16:37 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel



On Fri, 19 Feb 2021, Denis Efremov wrote:

> Check for opencoded swap() implementation.
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> Changes in v2:
>  - additional patch rule to drop excessive {}
>  - fix indentation in patch mode by anchoring ;
>
>  scripts/coccinelle/misc/swap.cocci | 101 +++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/swap.cocci
>
> diff --git a/scripts/coccinelle/misc/swap.cocci b/scripts/coccinelle/misc/swap.cocci
> new file mode 100644
> index 000000000000..d5da9888c222
> --- /dev/null
> +++ b/scripts/coccinelle/misc/swap.cocci
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for opencoded swap() implementation.
> +///
> +// Confidence: High
> +// Copyright: (C) 2021 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: swap
> +//
> +
> +virtual patch
> +virtual org
> +virtual report
> +virtual context
> +
> +@r depends on !patch@
> +identifier tmp;
> +expression a, b;
> +type T;
> +position p;
> +@@
> +
> +(
> +* T tmp;
> +|
> +* T tmp = 0;
> +|
> +* T *tmp = NULL;
> +)
> +... when != tmp
> +* tmp = a;
> +* a = b;@p
> +* b = tmp;
> +... when != tmp
> +
> +@rpvar depends on patch@
> +identifier tmp;
> +expression a, b;
> +type T;
> +@@
> +
> +(
> +- T tmp;
> +|
> +- T tmp = 0;
> +|
> +- T *tmp = NULL;
> +)
> +... when != tmp
> +- tmp = a;
> +- a = b;
> +- b = tmp
> ++ swap(a, b)
> +  ;
> +... when != tmp
> +
> +
> +@rp depends on patch@
> +identifier tmp;
> +expression a, b;
> +@@
> +
> +- tmp = a;
> +- a = b;
> +- b = tmp
> ++ swap(a, b)
> +  ;

A rule like the above should also appear in the non-patch case.

> +
> +@depends on (rpvar || rp)@

This needs to be depends on patch && (rpvar || rp).  It doesn't make much
sense, because rpvar and rp both depend on patch, but at the moment that
information isn't propagate everywhere that it is needed.

thanks,
julia

> +@@
> +
> +(
> +  for (...;...;...)
> +- {
> +	swap(...);
> +- }
> +|
> +  while (...)
> +- {
> +	swap(...);
> +- }
> +|
> +  if (...)
> +- {
> +	swap(...);
> +- }
> +)
> +
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> --
> 2.26.2
>
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cocci] [PATCH v3] coccinelle: misc: add swap script
  2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
                   ` (3 preceding siblings ...)
  2021-02-19  9:24 ` [Cocci] [PATCH v2] coccinelle: misc: add swap script Denis Efremov
@ 2021-03-05 10:09 ` Denis Efremov
       [not found]   ` <afb06bbc-5c28-8a92-f205-c9a1c87c707c@linux.com>
  4 siblings, 1 reply; 15+ messages in thread
From: Denis Efremov @ 2021-03-05 10:09 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, linux-kernel

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
Changes in v2:
 - additional patch rule to drop excessive {}
 - fix indentation in patch mode by anchoring ;
Changes in v3:
 - Rule added for simple (without var init) swap highlighting in !patch mode 
 - "depends on patch && (rpvar || rp)" fixed

 scripts/coccinelle/misc/swap.cocci | 122 +++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 scripts/coccinelle/misc/swap.cocci

diff --git a/scripts/coccinelle/misc/swap.cocci b/scripts/coccinelle/misc/swap.cocci
new file mode 100644
index 000000000000..c5e71b7ef7f5
--- /dev/null
+++ b/scripts/coccinelle/misc/swap.cocci
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for opencoded swap() implementation.
+///
+// Confidence: High
+// Copyright: (C) 2021 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+// Keywords: swap
+//
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@rvar depends on !patch@
+identifier tmp;
+expression a, b;
+type T;
+position p;
+@@
+
+(
+* T tmp;
+|
+* T tmp = 0;
+|
+* T *tmp = NULL;
+)
+... when != tmp
+* tmp = a;
+* a = b;@p
+* b = tmp;
+... when != tmp
+
+@r depends on !patch@
+identifier tmp;
+expression a, b;
+position p != rvar.p;
+@@
+
+* tmp = a;
+* a = b;@p
+* b = tmp;
+
+@rpvar depends on patch@
+identifier tmp;
+expression a, b;
+type T;
+@@
+
+(
+- T tmp;
+|
+- T tmp = 0;
+|
+- T *tmp = NULL;
+)
+... when != tmp
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+  ;
+... when != tmp
+
+@rp depends on patch@
+identifier tmp;
+expression a, b;
+@@
+
+- tmp = a;
+- a = b;
+- b = tmp
++ swap(a, b)
+  ;
+
+@depends on patch && (rpvar || rp)@
+@@
+
+(
+  for (...;...;...)
+- {
+	swap(...);
+- }
+|
+  while (...)
+- {
+	swap(...);
+- }
+|
+  if (...)
+- {
+	swap(...);
+- }
+)
+
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on report@
+p << rvar.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
+
+@script:python depends on org@
+p << rvar.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
-- 
2.26.2

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Cocci] [PATCH v3] coccinelle: misc: add swap script
       [not found]   ` <afb06bbc-5c28-8a92-f205-c9a1c87c707c@linux.com>
@ 2021-04-04 12:33     ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2021-04-04 12:33 UTC (permalink / raw)
  To: Denis Efremov; +Cc: cocci, linux-kernel



On Sun, 28 Mar 2021, Denis Efremov wrote:

> Ping?

Applied.  Thanks.

>
> On 3/5/21 1:09 PM, Denis Efremov wrote:
> > Check for opencoded swap() implementation.
> >
> > Signed-off-by: Denis Efremov <efremov@linux.com>
> > ---
> > Changes in v2:
> >   - additional patch rule to drop excessive {}
> >   - fix indentation in patch mode by anchoring ;
> > Changes in v3:
> >   - Rule added for simple (without var init) swap highlighting in !patch
> > mode
> >   - "depends on patch && (rpvar || rp)" fixed
> >
> >   scripts/coccinelle/misc/swap.cocci | 122 +++++++++++++++++++++++++++++
> >   1 file changed, 122 insertions(+)
> >   create mode 100644 scripts/coccinelle/misc/swap.cocci
> >
> > diff --git a/scripts/coccinelle/misc/swap.cocci
> > b/scripts/coccinelle/misc/swap.cocci
> > new file mode 100644
> > index 000000000000..c5e71b7ef7f5
> > --- /dev/null
> > +++ b/scripts/coccinelle/misc/swap.cocci
> > @@ -0,0 +1,122 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +///
> > +/// Check for opencoded swap() implementation.
> > +///
> > +// Confidence: High
> > +// Copyright: (C) 2021 Denis Efremov ISPRAS
> > +// Options: --no-includes --include-headers
> > +//
> > +// Keywords: swap
> > +//
> > +
> > +virtual patch
> > +virtual org
> > +virtual report
> > +virtual context
> > +
> > +@rvar depends on !patch@
> > +identifier tmp;
> > +expression a, b;
> > +type T;
> > +position p;
> > +@@
> > +
> > +(
> > +* T tmp;
> > +|
> > +* T tmp = 0;
> > +|
> > +* T *tmp = NULL;
> > +)
> > +... when != tmp
> > +* tmp = a;
> > +* a = b;@p
> > +* b = tmp;
> > +... when != tmp
> > +
> > +@r depends on !patch@
> > +identifier tmp;
> > +expression a, b;
> > +position p != rvar.p;
> > +@@
> > +
> > +* tmp = a;
> > +* a = b;@p
> > +* b = tmp;
> > +
> > +@rpvar depends on patch@
> > +identifier tmp;
> > +expression a, b;
> > +type T;
> > +@@
> > +
> > +(
> > +- T tmp;
> > +|
> > +- T tmp = 0;
> > +|
> > +- T *tmp = NULL;
> > +)
> > +... when != tmp
> > +- tmp = a;
> > +- a = b;
> > +- b = tmp
> > ++ swap(a, b)
> > +  ;
> > +... when != tmp
> > +
> > +@rp depends on patch@
> > +identifier tmp;
> > +expression a, b;
> > +@@
> > +
> > +- tmp = a;
> > +- a = b;
> > +- b = tmp
> > ++ swap(a, b)
> > +  ;
> > +
> > +@depends on patch && (rpvar || rp)@
> > +@@
> > +
> > +(
> > +  for (...;...;...)
> > +- {
> > +	swap(...);
> > +- }
> > +|
> > +  while (...)
> > +- {
> > +	swap(...);
> > +- }
> > +|
> > +  if (...)
> > +- {
> > +	swap(...);
> > +- }
> > +)
> > +
> > +
> > +@script:python depends on report@
> > +p << r.p;
> > +@@
> > +
> > +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on org@
> > +p << r.p;
> > +@@
> > +
> > +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on report@
> > +p << rvar.p;
> > +@@
> > +
> > +coccilib.report.print_report(p[0], "WARNING opportunity for swap()")
> > +
> > +@script:python depends on org@
> > +p << rvar.p;
> > +@@
> > +
> > +coccilib.org.print_todo(p[0], "WARNING opportunity for swap()")
> >
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16  8:01 [Cocci] [PATCH] coccinelle: misc: add swap script Denis Efremov
2021-02-16 18:05 ` Markus Elfring
2021-02-17 21:31 ` Julia Lawall
2021-02-18  6:13   ` Denis Efremov
2021-02-18 10:17     ` Julia Lawall
2021-02-18 11:03       ` Denis Efremov
2021-02-18 11:29         ` Julia Lawall
2021-02-18 11:34           ` Denis Efremov
     [not found]       ` <50eb8319-a552-c749-6143-7e24a8778a04@web.de>
2021-02-18 20:03         ` Julia Lawall
2021-02-19  9:05 ` [Cocci] [PATCH v2] coccinelle: misc: add minmax script Denis Efremov
2021-02-19  9:07   ` Denis Efremov
2021-02-19  9:24 ` [Cocci] [PATCH v2] coccinelle: misc: add swap script Denis Efremov
2021-03-03 16:37   ` Julia Lawall
2021-03-05 10:09 ` [Cocci] [PATCH v3] " Denis Efremov
     [not found]   ` <afb06bbc-5c28-8a92-f205-c9a1c87c707c@linux.com>
2021-04-04 12:33     ` Julia Lawall

Coccinelle Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git