2021-02-16 08:03:51

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH] coccinelle: misc: add swap script

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <[email protected]>
---
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


2021-02-17 21:34:08

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script

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

2021-02-18 06:35:37

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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

2021-02-18 11:55:29

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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

2021-02-18 12:52:14

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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

2021-02-18 13:20:46

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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
>

2021-02-18 13:32:27

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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

2021-02-18 20:08:53

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] coccinelle: misc: add swap script



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

2021-02-19 09:13:36

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH v2] coccinelle: misc: add minmax script

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

Signed-off-by: Denis Efremov <[email protected]>
---

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

2021-02-19 09:14:35

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v2] coccinelle: misc: add minmax script

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

2021-02-19 09:27:52

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH v2] coccinelle: misc: add swap script

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <[email protected]>
---
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

2021-03-04 12:11:34

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH v2] coccinelle: misc: add swap script



On Fri, 19 Feb 2021, Denis Efremov wrote:

> Check for opencoded swap() implementation.
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> 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
>
>

2021-03-05 08:14:07

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH v3] coccinelle: misc: add swap script

Check for opencoded swap() implementation.

Signed-off-by: Denis Efremov <[email protected]>
---
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

2021-03-28 09:23:13

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v3] coccinelle: misc: add swap script

Ping?

On 3/5/21 1:09 PM, Denis Efremov wrote:
> Check for opencoded swap() implementation.
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> 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()")
>

2021-04-04 12:47:47

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH v3] coccinelle: misc: add swap script



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