Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma use that should be semicolon)
On Fri, 2020-08-21 at 23:35 -0400, Valdis Klētnieks wrote:
> On Fri, 21 Aug 2020 18:08:08 -0700, Joe Perches said:
> > (forwarding on to kernel-janitors/mentees and kernelnewbies)
> >
> > Just fyi for anyone that cares:
> >
> > A janitorial task for someone might be to use Julia's coccinelle
> > script below to convert the existing instances of commas that
> > separate statements into semicolons.
>
> Note that you need to *really* check for possible changes in semantics.
> It's *usually* OK to do that, but sometimes it's not...
>
> for (i=0; i++, last++; !last) {
>
> changing that comma to a ; will break the compile. In other cases, it can
> introduce subtle bugs.
True enough for a general statement, though the coccinelle
script Julia provided does not change a single instance of
for loop expressions with commas.
As far as I can tell, no logic defect is introduced by the
script at all.
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma use that should be semicolon)
(forwarding on to kernel-janitors/mentees and kernelnewbies)
Just fyi for anyone that cares:
A janitorial task for someone might be to use Julia's coccinelle
script below to convert the existing instances of commas that
separate statements into semicolons.
https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/
On Thu, 2020-08-20 at 19:03 +0200, Julia Lawall wrote:
> > > I have a bunch of variations of this that are more complicated than I
> > > would have expected. One shorter variant that I have is:
> > >
> > > @@
> > > expression e1,e2;
> > > statement S;
> > > @@
> > >
> > > S
> > > e1
> > > -,
> > > +;
> > > (<+... e2 ...+>);
> > >
> > > This will miss cases where the first statement is the comma thing. But I
> > > think it is possible to improve this now. I will check.
> >
> > Hi Julia.
> >
> > Right, thanks, this adds a dependency on a statement
> > before the expression. Any stragglers would be easily
> > found using slightly different form.
> > There are not very many of these in linux kernel.
> >
> > Another nicety would be to allow the s/,/;/ conversion to
> > find both b and c in this sequence:
> > a = 1;
> > b = 2,
> > c = 3,
> > d = 4;
> > without running the script multiple times.
> > There are many dozen uses of this style in linux kernel.
> >
> > I tried variants of adding a comma after the e2 expression,
> > but cocci seems to have parsing problems with:
> >
> > @@
> > expression e1;
> > expression e2;
> > @@
> > e1
> > - ,
> > + ;
> > e2,
>
> This doesn't work because it's not a valid expression.
>
> The problem is solved by doing:
>
> e1
> - ,
> + ;
> e2
> ... when any
>
> But that doesn't work in the current version of Coccinelle. I have fixed
> the problem, though and it will work shortly.
>
> > I do appreciate that coccinelle adds braces for multiple
> > expression comma use after an if.
> >
> > i.e.:
> > if (foo)
> > a = 1, b = 2;
> > becomes
> > if (foo) {
> > a = 1; b = 2;
> > }
>
> I wasn't sure what was wanted for such things. Should b = 2 now be on a
> separate line?
>
> I took the strategy of avoiding the problem and leaving those cases as is.
> That also solves the LIST_HEAD problem. But if it is wanted to change
> these commas under ifs, then that is probably possible too.
>
> My current complete solution is as follows. The first rule avoids changing
> commas in macros, where thebody of the macro is just an expression. The
> second rule uses position variables to ensure that the two expression are
> on different lines.
>
> @r@
> expression e1,e2;
> statement S;
> position p;
> @@
>
> e1 ,@S@p e2;
>
> @@
> expression e1,e2;
> position p1;
> position p2 :
> script:ocaml(p1) { not((List.hd p1).line_end = (List.hd p2).line) };
> statement S;
> position r.p;
> @@
>
> e1@p1
> -,@S@p
> +;
> e2@p2
> ... when any
>
> The generated patch is below.
>
> julia
>
> diff -u -p a/drivers/reset/hisilicon/reset-hi3660.c
> b/drivers/reset/hisilicon/reset-hi3660.c
> --- a/drivers/reset/hisilicon/reset-hi3660.c
> +++ b/drivers/reset/hisilicon/reset-hi3660.c
> @@ -89,7 +89,7 @@ static int hi3660_reset_probe(struct pla
> return PTR_ERR(rc->map);
> }
>
> - rc->rst.ops = &hi3660_reset_ops,
> + rc->rst.ops = &hi3660_reset_ops;
> rc->rst.of_node = np;
> rc->rst.of_reset_n_cells = 2;
> rc->rst.of_xlate = hi3660_reset_xlate;
The rest of the changes are in the link above...
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: api: add kobj_to_dev.cocci script
On Fri, 21 Aug 2020, Denis Efremov wrote: > Use kobj_to_dev() instead of container_of(). > > Signed-off-by: Denis Efremov Applied, thanks. julia > --- > Changes in v2: > - "symbol kobj;" added to the rule r > > scripts/coccinelle/api/kobj_to_dev.cocci | 45 > 1 file changed, 45 insertions(+) > create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci > > diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci > b/scripts/coccinelle/api/kobj_to_dev.cocci > new file mode 100644 > index ..cd5d31c6fe76 > --- /dev/null > +++ b/scripts/coccinelle/api/kobj_to_dev.cocci > @@ -0,0 +1,45 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Use kobj_to_dev() instead of container_of() > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > +// Keywords: kobj_to_dev, container_of > +// > + > +virtual context > +virtual report > +virtual org > +virtual patch > + > + > +@r depends on !patch@ > +expression ptr; > +symbol kobj; > +position p; > +@@ > + > +* container_of(ptr, struct device, kobj)@p > + > + > +@depends on patch@ > +expression ptr; > +@@ > + > +- container_of(ptr, struct device, kobj) > ++ kobj_to_dev(ptr) > + > + > +@script:python depends on report@ > +p << r.p; > +@@ > + > +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") > + > +@script:python depends on org@ > +p << r.p; > +@@ > + > +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") > -- > 2.26.2 > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: api: add kobj_to_dev.cocci script
Use kobj_to_dev() instead of container_of(). Signed-off-by: Denis Efremov --- Changes in v2: - "symbol kobj;" added to the rule r scripts/coccinelle/api/kobj_to_dev.cocci | 45 1 file changed, 45 insertions(+) create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci b/scripts/coccinelle/api/kobj_to_dev.cocci new file mode 100644 index ..cd5d31c6fe76 --- /dev/null +++ b/scripts/coccinelle/api/kobj_to_dev.cocci @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Use kobj_to_dev() instead of container_of() +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// +// Keywords: kobj_to_dev, container_of +// + +virtual context +virtual report +virtual org +virtual patch + + +@r depends on !patch@ +expression ptr; +symbol kobj; +position p; +@@ + +* container_of(ptr, struct device, kobj)@p + + +@depends on patch@ +expression ptr; +@@ + +- container_of(ptr, struct device, kobj) ++ kobj_to_dev(ptr) + + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add kobj_to_dev.cocci script
On Fri, 21 Aug 2020, Denis Efremov wrote: > Use kobj_to_dev() instead of container_of(). Thanks for the patch and the example commits. Could you add symbol kobj; to the first rule? That's enough to get rid of the question about whether kobj should be a metavariable in all cases. thanks, julia > > Signed-off-by: Denis Efremov > --- > Examples of such patches: > 893c3d82b425 watchdog: Use kobj_to_dev() API > 23fd63a44460 hwmon: (nct6683) Replace container_of() with kobj_to_dev() > 224941c9424f power: supply: use kobj_to_dev > a9b9b2af40c7 backlight: lm3533_bl: Use kobj_to_dev() instead > 0acb47a3a093 qlcnic: Use kobj_to_dev() instead > 97cd738c44c8 gpiolib: sysfs: use kobj_to_dev > d06f9e6c8960 hwmon: (nct7802) Replace container_of() API > 036855a4c3b3 hwmon : (nct6775) Use kobj_to_dev() API > baf1d9c18293 driver/base/soc: Use kobj_to_dev() API > ae243ef0afbc rtc: sysfs: use kobj_to_dev > 6b060d8a09e9 i2c: use kobj_to_dev() API > 9e7bd945b9a9 scsi: core: use kobj_to_dev > 0d730b57b95f s390/cio: use kobj_to_dev() API > 0616ca73fd35 usb: use kobj_to_dev() API > 8c9b839c0b80 alpha: use kobj_to_dev() > 016c0bbae1d1 netxen: Use kobj_to_dev() > 6908b45eafc4 GenWQE: use kobj_to_dev() > 85f4f39c80e9 pch_phub: use kobj_to_dev() > 47679cde604d misc: c2port: use kobj_to_dev() > 85016ff33f35 misc: cxl: use kobj_to_dev() > 092462c2b522 misc: eeprom: use kobj_to_dev() > a9c9d9aca4e7 zorro: Use kobj_to_dev() > a253f1eee6c4 rapidio: use kobj_to_dev() > e3837b00b6bb drm/radeon: use kobj_to_dev() > cc29ec874b37 drm/amdgpu: use kobj_to_dev() > d122cbf1a310 drm/sysfs: use kobj_to_dev() > 657fb5fbadb3 drm/i915: use kobj_to_dev() > 554a60379aaa PCI: Use kobj_to_dev() instead of open-coding it > 2cf83833fc9c HID: use kobj_to_dev() > aeb7ed14fe5d bridge: use kobj_to_dev instead of to_dev > 8e3829c61b48 staging:iio:adis16220: Use kobj_to_dev instead of open-coding it > b0d1f807f340 driver-core: Use kobj_to_dev instead of re-implementing it > > scripts/coccinelle/api/kobj_to_dev.cocci | 44 > 1 file changed, 44 insertions(+) > create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci > > diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci > b/scripts/coccinelle/api/kobj_to_dev.cocci > new file mode 100644 > index ..e2cdd424aeca > --- /dev/null > +++ b/scripts/coccinelle/api/kobj_to_dev.cocci > @@ -0,0 +1,44 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/// > +/// Use kobj_to_dev() instead of container_of() > +/// > +// Confidence: High > +// Copyright: (C) 2020 Denis Efremov ISPRAS > +// Options: --no-includes --include-headers > +// > +// Keywords: kobj_to_dev, container_of > +// > + > +virtual context > +virtual report > +virtual org > +virtual patch > + > + > +@r depends on !patch@ > +expression ptr; > +position p; > +@@ > + > +* container_of(ptr, struct device, kobj)@p > + > + > +@depends on patch@ > +expression ptr; > +@@ > + > +- container_of(ptr, struct device, kobj) > ++ kobj_to_dev(ptr) > + > + > +@script:python depends on report@ > +p << r.p; > +@@ > + > +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") > + > +@script:python depends on org@ > +p << r.p; > +@@ > + > +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") > -- > 2.26.2 > > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: add patch rule for dma_alloc_coherent
On Fri, 21 Aug 2020, Alex Dewar wrote:
> Commit dfd32cad146e ("dma-mapping: remove dma_zalloc_coherent()")
> removed the definition of dma_zalloc_coherent() and also removed the
> corresponding patch rule for replacing instances of dma_alloc_coherent +
> memset in zalloc-simple.cocci (though left the report rule).
>
> Add a new patch rule to remove unnecessary calls to memset after
> allocating with dma_alloc_coherent. While we're at it, fix a couple of
> typos.
Applied, thanks!
julia
>
> Fixes: dfd32cad146e ("dma-mapping: remove dma_zalloc_coherent()")
> Signed-off-by: Alex Dewar
> ---
> scripts/coccinelle/api/alloc/zalloc-simple.cocci | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> index 26cda3f48f01..b3d0c3c230c1 100644
> --- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> +++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> @@ -127,6 +127,16 @@ statement S;
>if ((x==NULL) || ...) S
> - memset((T2)x,0,E1);
>
> +@depends on patch@
> +type T, T2;
> +expression x;
> +expression E1,E2,E3,E4;
> +statement S;
> +@@
> + x = (T)dma_alloc_coherent(E1, E2, E3, E4);
> + if ((x==NULL) || ...) S
> +- memset((T2)x, 0, E2);
> +
> //--
> // For org mode
> //--
> @@ -199,9 +209,9 @@ statement S;
> position p;
> @@
>
> - x = (T)dma_alloc_coherent@p(E2,E1,E3,E4);
> + x = (T)dma_alloc_coherent@p(E1,E2,E3,E4);
> if ((x==NULL) || ...) S
> - memset((T2)x,0,E1);
> + memset((T2)x,0,E2);
>
> @script:python depends on org@
> p << r2.p;
> @@ -217,7 +227,7 @@ p << r2.p;
> x << r2.x;
> @@
>
> -msg="WARNING: dma_alloc_coherent use in %s already zeroes out memory, so
> memset is not needed" % (x)
> +msg="WARNING: dma_alloc_coherent used in %s already zeroes out memory, so
> memset is not needed" % (x)
> coccilib.report.print_report(p[0], msg)
>
> //-
> --
> 2.28.0
>
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: api: add kobj_to_dev.cocci script
Use kobj_to_dev() instead of container_of(). Signed-off-by: Denis Efremov --- Examples of such patches: 893c3d82b425 watchdog: Use kobj_to_dev() API 23fd63a44460 hwmon: (nct6683) Replace container_of() with kobj_to_dev() 224941c9424f power: supply: use kobj_to_dev a9b9b2af40c7 backlight: lm3533_bl: Use kobj_to_dev() instead 0acb47a3a093 qlcnic: Use kobj_to_dev() instead 97cd738c44c8 gpiolib: sysfs: use kobj_to_dev d06f9e6c8960 hwmon: (nct7802) Replace container_of() API 036855a4c3b3 hwmon : (nct6775) Use kobj_to_dev() API baf1d9c18293 driver/base/soc: Use kobj_to_dev() API ae243ef0afbc rtc: sysfs: use kobj_to_dev 6b060d8a09e9 i2c: use kobj_to_dev() API 9e7bd945b9a9 scsi: core: use kobj_to_dev 0d730b57b95f s390/cio: use kobj_to_dev() API 0616ca73fd35 usb: use kobj_to_dev() API 8c9b839c0b80 alpha: use kobj_to_dev() 016c0bbae1d1 netxen: Use kobj_to_dev() 6908b45eafc4 GenWQE: use kobj_to_dev() 85f4f39c80e9 pch_phub: use kobj_to_dev() 47679cde604d misc: c2port: use kobj_to_dev() 85016ff33f35 misc: cxl: use kobj_to_dev() 092462c2b522 misc: eeprom: use kobj_to_dev() a9c9d9aca4e7 zorro: Use kobj_to_dev() a253f1eee6c4 rapidio: use kobj_to_dev() e3837b00b6bb drm/radeon: use kobj_to_dev() cc29ec874b37 drm/amdgpu: use kobj_to_dev() d122cbf1a310 drm/sysfs: use kobj_to_dev() 657fb5fbadb3 drm/i915: use kobj_to_dev() 554a60379aaa PCI: Use kobj_to_dev() instead of open-coding it 2cf83833fc9c HID: use kobj_to_dev() aeb7ed14fe5d bridge: use kobj_to_dev instead of to_dev 8e3829c61b48 staging:iio:adis16220: Use kobj_to_dev instead of open-coding it b0d1f807f340 driver-core: Use kobj_to_dev instead of re-implementing it scripts/coccinelle/api/kobj_to_dev.cocci | 44 1 file changed, 44 insertions(+) create mode 100644 scripts/coccinelle/api/kobj_to_dev.cocci diff --git a/scripts/coccinelle/api/kobj_to_dev.cocci b/scripts/coccinelle/api/kobj_to_dev.cocci new file mode 100644 index ..e2cdd424aeca --- /dev/null +++ b/scripts/coccinelle/api/kobj_to_dev.cocci @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0-only +/// +/// Use kobj_to_dev() instead of container_of() +/// +// Confidence: High +// Copyright: (C) 2020 Denis Efremov ISPRAS +// Options: --no-includes --include-headers +// +// Keywords: kobj_to_dev, container_of +// + +virtual context +virtual report +virtual org +virtual patch + + +@r depends on !patch@ +expression ptr; +position p; +@@ + +* container_of(ptr, struct device, kobj)@p + + +@depends on patch@ +expression ptr; +@@ + +- container_of(ptr, struct device, kobj) ++ kobj_to_dev(ptr) + + +@script:python depends on report@ +p << r.p; +@@ + +coccilib.report.print_report(p[0], "WARNING opportunity for kobj_to_dev()") + +@script:python depends on org@ +p << r.p; +@@ + +coccilib.org.print_todo(p[0], "WARNING opportunity for kobj_to_dev()") -- 2.26.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] coccinelle: add patch rule for dma_alloc_coherent
Commit dfd32cad146e ("dma-mapping: remove dma_zalloc_coherent()")
removed the definition of dma_zalloc_coherent() and also removed the
corresponding patch rule for replacing instances of dma_alloc_coherent +
memset in zalloc-simple.cocci (though left the report rule).
Add a new patch rule to remove unnecessary calls to memset after
allocating with dma_alloc_coherent. While we're at it, fix a couple of
typos.
Fixes: dfd32cad146e ("dma-mapping: remove dma_zalloc_coherent()")
Signed-off-by: Alex Dewar
---
scripts/coccinelle/api/alloc/zalloc-simple.cocci | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
index 26cda3f48f01..b3d0c3c230c1 100644
--- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
@@ -127,6 +127,16 @@ statement S;
if ((x==NULL) || ...) S
- memset((T2)x,0,E1);
+@depends on patch@
+type T, T2;
+expression x;
+expression E1,E2,E3,E4;
+statement S;
+@@
+ x = (T)dma_alloc_coherent(E1, E2, E3, E4);
+ if ((x==NULL) || ...) S
+- memset((T2)x, 0, E2);
+
//--
// For org mode
//--
@@ -199,9 +209,9 @@ statement S;
position p;
@@
- x = (T)dma_alloc_coherent@p(E2,E1,E3,E4);
+ x = (T)dma_alloc_coherent@p(E1,E2,E3,E4);
if ((x==NULL) || ...) S
- memset((T2)x,0,E1);
+ memset((T2)x,0,E2);
@script:python depends on org@
p << r2.p;
@@ -217,7 +227,7 @@ p << r2.p;
x << r2.x;
@@
-msg="WARNING: dma_alloc_coherent use in %s already zeroes out memory, so
memset is not needed" % (x)
+msg="WARNING: dma_alloc_coherent used in %s already zeroes out memory, so
memset is not needed" % (x)
coccilib.report.print_report(p[0], msg)
//-
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: Convert comma to semicolon
> > Add an imperfect test to detect these comma uses. > > > > No false positives were found in testing, but many types of false negatives > > are possible. > > > > e.g.: > > foo = bar() + 1,/* comma use, but not direct assignment */ > > bar = baz(); > > Hi. > > I recently added a test for this condition to linux's checkpatch. > > A similar coccinelle script might be: I find it interesting that you present another transformation approach for the semantic patch language. > $ cat comma.cocci > @@ > expression e1; > expression e2; > @@ > > e1 > - , > + ; > e2; Such a tiny SmPL script looks so simple. > This works reasonably well but it has several false positives > for declarations like: Would you like to filter the usage of code like “LIST_HEAD(list)” out? Are there any more software development challenges to consider for special assignment statements? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
