| Message ID | 1506861768-5734-1-git-send-email-Julia.Lawall@lip6.fr (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > within a single structure. It is currently specific to the file > drivers/power/supply/bq27xxx_battery.c. > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > --- Applied to linux-kbuild/misc. Thanks.
On Fri, 6 Oct 2017, Masahiro Yamada wrote: > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > within a single structure. It is currently specific to the file > > drivers/power/supply/bq27xxx_battery.c. > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > --- > > Applied to linux-kbuild/misc. Thanks for picking it up. julia > Thanks. > > > -- > Best Regards > Masahiro Yamada >
On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > within a single structure. It is currently specific to the file > > > drivers/power/supply/bq27xxx_battery.c. > > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > > > --- > > > > Applied to linux-kbuild/misc. > > Thanks for picking it up. If it is specific to one file, why not just run it and post the resultant patch? Why have it in tree?
On Thu, 5 Oct 2017, Joe Perches wrote: > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > > within a single structure. It is currently specific to the file > > > > drivers/power/supply/bq27xxx_battery.c. > > > > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > > > > > --- > > > > > > Applied to linux-kbuild/misc. > > > > Thanks for picking it up. > > If it is specific to one file, why not just run it > and post the resultant patch? Why have it in tree? I guess that they anticipate that the data may change in the future? If id-utils is used, Coccinelle will completely ignore files that don't contain BQ27XXX_DATA, so the rule will have essentially no performance impact. If there is no indexing, it will only "grep" for BQ27XXX_DATA, not actually parse the files that don't contain it. So there is not much performance impact even in that case. julia
Hi, sorry for slow reply... Can we patch something to make this script run by default on bq7xxxx_battery_i2c build? If so let's do that. Also maybe the name of the script should include "bq27xxx_data"? Few more comments below... On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > within a single structure. It is currently specific to the file > drivers/power/supply/bq27xxx_battery.c. > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > --- > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ > 1 file changed, 161 insertions(+) > > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci > new file mode 100644 > index 0000000..77c145a > --- /dev/null > +++ b/scripts/coccinelle/api/battery.cocci > @@ -0,0 +1,161 @@ > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or > +/// properties. > +//# Doesn't unfold macros used in register or property fields. > +//# Requires OCaml scripting > +/// > +// Confidence: High > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. > +// URL: http://coccinelle.lip6.fr/ > +// Requires: 1.0.7 > +// Keywords: BQ27XXX_DATA > + > +virtual report > + > +@initialize:ocaml@ > +@@ > + > +let print_report p msg = > + let p = List.hd p in > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg > + > +@str depends on report@ > +type t; > +identifier i,i1,i2; > +expression e1,e2; > +@@ > + > +t i[] = { > + ..., > + [e1] = BQ27XXX_DATA(i1,...), > + ..., > + [e2] = BQ27XXX_DATA(i2,...), > + ..., > +}; > + > +@script:ocaml tocheck@ > +i1 << str.i1; > +i2 << str.i2; > +i1regs; i2regs; > +i1dmregs; i2dmregs; > +i1props; i2props; > +@@ > + > +if not(i1 = i2) > +then > + begin > + i1regs := make_ident (i1 ^ "_regs"); > + i2regs := make_ident (i2 ^ "_regs"); > + i1dmregs := make_ident (i1 ^ "_dm_regs"); > + i2dmregs := make_ident (i2 ^ "_dm_regs"); > + i1props := make_ident (i1 ^ "_props"); > + i2props := make_ident (i2 ^ "_props") > + end > + > +(* ---------------------------------------------------------------- *) > + > +@getregs1@ > +typedef u8; > +identifier tocheck.i1regs; > +initializer list i1regs_vals; > +position p1; > +@@ > + > +u8 i1regs@p1[...] = { i1regs_vals, }; > + > +@getregs2@ > +identifier tocheck.i2regs; > +initializer list i2regs_vals; > +position p2; > +@@ > + > +u8 i2regs@p2[...] = { i2regs_vals, }; > + > +@script:ocaml@ > +(_,i1regs_vals) << getregs1.i1regs_vals; > +(_,i2regs_vals) << getregs2.i2regs_vals; > +i1regs << tocheck.i1regs; > +i2regs << tocheck.i2regs; > +p1 << getregs1.p1; > +p2 << getregs2.p2; > +@@ > + > +if i1regs < i2regs && > + List.sort compare i1regs_vals = List.sort compare i2regs_vals > +then > + let msg = > + Printf.sprintf > + "WARNING %s and %s (line %d) have the same registers\n" "are identical" vs "have the same..." > + i1regs i2regs (List.hd p2).line in > + print_report p1 msg > + > +(* ---------------------------------------------------------------- *) > + > +@getdmregs1@ > +identifier tocheck.i1dmregs; > +initializer list i1dmregs_vals; > +position p1; > +@@ > + > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; > + > +@getdmregs2@ > +identifier tocheck.i2dmregs; > +initializer list i2dmregs_vals; > +position p2; > +@@ > + > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; > + > +@script:ocaml@ > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; > +i1dmregs << tocheck.i1dmregs; > +i2dmregs << tocheck.i2dmregs; > +p1 << getdmregs1.p1; > +p2 << getdmregs2.p2; > +@@ > + > +if i1dmregs < i2dmregs && > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals > +then > + let msg = > + Printf.sprintf > + "WARNING %s and %s (line %d) have the same dm registers\n" "are identical" vs "have the same..." > + i1dmregs i2dmregs (List.hd p2).line in > + print_report p1 msg > + > +(* ---------------------------------------------------------------- *) > + > +@getprops1@ > +identifier tocheck.i1props; > +initializer list[n1] i1props_vals; > +position p1; > +@@ > + > +enum power_supply_property i1props@p1[] = { i1props_vals, }; > + > +@getprops2@ > +identifier tocheck.i2props; > +initializer list[n2] i2props_vals; > +position p2; > +@@ > + > +enum power_supply_property i2props@p2[] = { i2props_vals, }; > + > +@script:ocaml@ > +(_,i1props_vals) << getprops1.i1props_vals; > +(_,i2props_vals) << getprops2.i2props_vals; > +i1props << tocheck.i1props; > +i2props << tocheck.i2props; > +p1 << getprops1.p1; > +p2 << getprops2.p2; > +@@ > + > +if i1props < i2props && > + List.sort compare i1props_vals = List.sort compare i2props_vals > +then > + let msg = > + Printf.sprintf > + "WARNING %s and %s (line %d) have the same properties\n" "are identical" vs "have the same..." > + i1props i2props (List.hd p2).line in > + print_report p1 msg >
Hi Joe, On Thu, Oct 5, 2017 at 12:15 PM, Joe Perches <joe@perches.com> wrote: > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: >> >> On Fri, 6 Oct 2017, Masahiro Yamada wrote: >> >> > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: >> > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA >> > > within a single structure. It is currently specific to the file >> > > drivers/power/supply/bq27xxx_battery.c. >> > > >> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> >> > > >> > > --- >> > >> > Applied to linux-kbuild/misc. >> >> Thanks for picking it up. > > If it is specific to one file, why not just run it > and post the resultant patch? Why have it in tree? The driver which this script analyzes supports more than a dozen different chips, which require different parameters. This script checks for duplicate parameter arrays.
On Thu, 5 Oct 2017, Liam Breck wrote: > Hi, sorry for slow reply... > > Can we patch something to make this script run by default on > bq7xxxx_battery_i2c build? If so let's do that. I don't think anything is set up for that. But any changes to the file should be checked by the 0-day bot. > Also maybe the name of the script should include "bq27xxx_data"? OK > Few more comments below... > > On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > within a single structure. It is currently specific to the file > > drivers/power/supply/bq27xxx_battery.c. > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > --- > > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 161 insertions(+) > > > > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci > > new file mode 100644 > > index 0000000..77c145a > > --- /dev/null > > +++ b/scripts/coccinelle/api/battery.cocci > > @@ -0,0 +1,161 @@ > > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or > > +/// properties. > > +//# Doesn't unfold macros used in register or property fields. > > +//# Requires OCaml scripting > > +/// > > +// Confidence: High > > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. > > +// URL: http://coccinelle.lip6.fr/ > > +// Requires: 1.0.7 > > +// Keywords: BQ27XXX_DATA > > + > > +virtual report > > + > > +@initialize:ocaml@ > > +@@ > > + > > +let print_report p msg = > > + let p = List.hd p in > > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg > > + > > +@str depends on report@ > > +type t; > > +identifier i,i1,i2; > > +expression e1,e2; > > +@@ > > + > > +t i[] = { > > + ..., > > + [e1] = BQ27XXX_DATA(i1,...), > > + ..., > > + [e2] = BQ27XXX_DATA(i2,...), > > + ..., > > +}; > > + > > +@script:ocaml tocheck@ > > +i1 << str.i1; > > +i2 << str.i2; > > +i1regs; i2regs; > > +i1dmregs; i2dmregs; > > +i1props; i2props; > > +@@ > > + > > +if not(i1 = i2) > > +then > > + begin > > + i1regs := make_ident (i1 ^ "_regs"); > > + i2regs := make_ident (i2 ^ "_regs"); > > + i1dmregs := make_ident (i1 ^ "_dm_regs"); > > + i2dmregs := make_ident (i2 ^ "_dm_regs"); > > + i1props := make_ident (i1 ^ "_props"); > > + i2props := make_ident (i2 ^ "_props") > > + end > > + > > +(* ---------------------------------------------------------------- *) > > + > > +@getregs1@ > > +typedef u8; > > +identifier tocheck.i1regs; > > +initializer list i1regs_vals; > > +position p1; > > +@@ > > + > > +u8 i1regs@p1[...] = { i1regs_vals, }; > > + > > +@getregs2@ > > +identifier tocheck.i2regs; > > +initializer list i2regs_vals; > > +position p2; > > +@@ > > + > > +u8 i2regs@p2[...] = { i2regs_vals, }; > > + > > +@script:ocaml@ > > +(_,i1regs_vals) << getregs1.i1regs_vals; > > +(_,i2regs_vals) << getregs2.i2regs_vals; > > +i1regs << tocheck.i1regs; > > +i2regs << tocheck.i2regs; > > +p1 << getregs1.p1; > > +p2 << getregs2.p2; > > +@@ > > + > > +if i1regs < i2regs && > > + List.sort compare i1regs_vals = List.sort compare i2regs_vals > > +then > > + let msg = > > + Printf.sprintf > > + "WARNING %s and %s (line %d) have the same registers\n" > > "are identical" vs "have the same..." OK, I guess identical would be appropriate for regsand dm_regs, but perhaps not for properties because there the same values might be in a different order. julia > > + i1regs i2regs (List.hd p2).line in > > + print_report p1 msg > > + > > +(* ---------------------------------------------------------------- *) > > + > > +@getdmregs1@ > > +identifier tocheck.i1dmregs; > > +initializer list i1dmregs_vals; > > +position p1; > > +@@ > > + > > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; > > + > > +@getdmregs2@ > > +identifier tocheck.i2dmregs; > > +initializer list i2dmregs_vals; > > +position p2; > > +@@ > > + > > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; > > + > > +@script:ocaml@ > > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; > > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; > > +i1dmregs << tocheck.i1dmregs; > > +i2dmregs << tocheck.i2dmregs; > > +p1 << getdmregs1.p1; > > +p2 << getdmregs2.p2; > > +@@ > > + > > +if i1dmregs < i2dmregs && > > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals > > +then > > + let msg = > > + Printf.sprintf > > + "WARNING %s and %s (line %d) have the same dm registers\n" > > "are identical" vs "have the same..." > > > + i1dmregs i2dmregs (List.hd p2).line in > > + print_report p1 msg > > + > > +(* ---------------------------------------------------------------- *) > > + > > +@getprops1@ > > +identifier tocheck.i1props; > > +initializer list[n1] i1props_vals; > > +position p1; > > +@@ > > + > > +enum power_supply_property i1props@p1[] = { i1props_vals, }; > > + > > +@getprops2@ > > +identifier tocheck.i2props; > > +initializer list[n2] i2props_vals; > > +position p2; > > +@@ > > + > > +enum power_supply_property i2props@p2[] = { i2props_vals, }; > > + > > +@script:ocaml@ > > +(_,i1props_vals) << getprops1.i1props_vals; > > +(_,i2props_vals) << getprops2.i2props_vals; > > +i1props << tocheck.i1props; > > +i2props << tocheck.i2props; > > +p1 << getprops1.p1; > > +p2 << getprops2.p2; > > +@@ > > + > > +if i1props < i2props && > > + List.sort compare i1props_vals = List.sort compare i2props_vals > > +then > > + let msg = > > + Printf.sprintf > > + "WARNING %s and %s (line %d) have the same properties\n" > > "are identical" vs "have the same..." > > > > + i1props i2props (List.hd p2).line in > > + print_report p1 msg > > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, 5 Oct 2017, Liam Breck wrote: > Hi Joe, > > On Thu, Oct 5, 2017 at 12:15 PM, Joe Perches <joe@perches.com> wrote: > > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > >> > >> On Fri, 6 Oct 2017, Masahiro Yamada wrote: > >> > >> > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > >> > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > >> > > within a single structure. It is currently specific to the file > >> > > drivers/power/supply/bq27xxx_battery.c. > >> > > > >> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > >> > > > >> > > --- > >> > > >> > Applied to linux-kbuild/misc. > >> > >> Thanks for picking it up. > > > > If it is specific to one file, why not just run it > > and post the resultant patch? Why have it in tree? > > The driver which this script analyzes supports more than a dozen > different chips, which require different parameters. This script > checks for duplicate parameter arrays. I think the question is more how will the code change over time. julia
On Thu, 2017-10-05 at 21:19 +0200, Julia Lawall wrote: > On Thu, 5 Oct 2017, Joe Perches wrote: > > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > > > within a single structure. It is currently specific to the file > > > > > drivers/power/supply/bq27xxx_battery.c. > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > Applied to linux-kbuild/misc. > > > Thanks for picking it up. > > If it is specific to one file, why not just run it > > and post the resultant patch? Why have it in tree? > I guess that they anticipate that the data may change in the future? Aren't you the script author Julia? Who is the "they"? I think having a script for a single file unnecessary. btw: spatch 1.7 doesn't seem to have a tag in git From the script: // Requires: 1.0.7 Assuming this is correct, then this doesn't even run today except maybe on your system. $ git show commit 0bf53049e64295e8ddb625e853134f49568f4bc3 Merge: 446dc66f8b98 d599cc6a75e0 Author: julia <julia.lawall@lip6.fr> Date: Sun Oct 1 22:44:34 2017 +0200 Merge git+ssh://palace.lip6.fr/var/git/coccinelle $ When I build this, the version is 1.0.6 $ /usr/local/bin/spatch --version spatch version 1.0.6-00033-g23cca0a238c2 compiled with OCaml version 4.02.3 Flags passed to the configure script: [none] Python scripting support: yes Syntax of regular expresssions: PCRE > If id-utils is used, Coccinelle will completely ignore files that don't > contain BQ27XXX_DATA, so the rule will have essentially no performance > impact. If there is no indexing, it will only "grep" for BQ27XXX_DATA, > not actually parse the files that don't contain it. So there is not much > performance impact even in that case. I'm not sure that matters.
Hi Julia, On Thu, Oct 5, 2017 at 12:25 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Thu, 5 Oct 2017, Liam Breck wrote: > >> Hi, sorry for slow reply... >> >> Can we patch something to make this script run by default on >> bq7xxxx_battery_i2c build? If so let's do that. > > I don't think anything is set up for that. But any changes to the file > should be checked by the 0-day bot. How do we ask the 0-day bot team to configure their build to run this script? Will they just pick it up? >> Also maybe the name of the script should include "bq27xxx_data"? > > OK > >> Few more comments below... >> >> On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: >> > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA >> > within a single structure. It is currently specific to the file >> > drivers/power/supply/bq27xxx_battery.c. >> > >> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> >> > >> > --- >> > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ >> > 1 file changed, 161 insertions(+) >> > >> > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci >> > new file mode 100644 >> > index 0000000..77c145a >> > --- /dev/null >> > +++ b/scripts/coccinelle/api/battery.cocci >> > @@ -0,0 +1,161 @@ >> > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or >> > +/// properties. >> > +//# Doesn't unfold macros used in register or property fields. >> > +//# Requires OCaml scripting >> > +/// >> > +// Confidence: High >> > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. >> > +// URL: http://coccinelle.lip6.fr/ >> > +// Requires: 1.0.7 >> > +// Keywords: BQ27XXX_DATA >> > + >> > +virtual report >> > + >> > +@initialize:ocaml@ >> > +@@ >> > + >> > +let print_report p msg = >> > + let p = List.hd p in >> > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg >> > + >> > +@str depends on report@ >> > +type t; >> > +identifier i,i1,i2; >> > +expression e1,e2; >> > +@@ >> > + >> > +t i[] = { >> > + ..., >> > + [e1] = BQ27XXX_DATA(i1,...), >> > + ..., >> > + [e2] = BQ27XXX_DATA(i2,...), >> > + ..., >> > +}; >> > + >> > +@script:ocaml tocheck@ >> > +i1 << str.i1; >> > +i2 << str.i2; >> > +i1regs; i2regs; >> > +i1dmregs; i2dmregs; >> > +i1props; i2props; >> > +@@ >> > + >> > +if not(i1 = i2) >> > +then >> > + begin >> > + i1regs := make_ident (i1 ^ "_regs"); >> > + i2regs := make_ident (i2 ^ "_regs"); >> > + i1dmregs := make_ident (i1 ^ "_dm_regs"); >> > + i2dmregs := make_ident (i2 ^ "_dm_regs"); >> > + i1props := make_ident (i1 ^ "_props"); >> > + i2props := make_ident (i2 ^ "_props") >> > + end >> > + >> > +(* ---------------------------------------------------------------- *) >> > + >> > +@getregs1@ >> > +typedef u8; >> > +identifier tocheck.i1regs; >> > +initializer list i1regs_vals; >> > +position p1; >> > +@@ >> > + >> > +u8 i1regs@p1[...] = { i1regs_vals, }; >> > + >> > +@getregs2@ >> > +identifier tocheck.i2regs; >> > +initializer list i2regs_vals; >> > +position p2; >> > +@@ >> > + >> > +u8 i2regs@p2[...] = { i2regs_vals, }; >> > + >> > +@script:ocaml@ >> > +(_,i1regs_vals) << getregs1.i1regs_vals; >> > +(_,i2regs_vals) << getregs2.i2regs_vals; >> > +i1regs << tocheck.i1regs; >> > +i2regs << tocheck.i2regs; >> > +p1 << getregs1.p1; >> > +p2 << getregs2.p2; >> > +@@ >> > + >> > +if i1regs < i2regs && >> > + List.sort compare i1regs_vals = List.sort compare i2regs_vals >> > +then >> > + let msg = >> > + Printf.sprintf >> > + "WARNING %s and %s (line %d) have the same registers\n" >> >> "are identical" vs "have the same..." > > OK, I guess identical would be appropriate for regsand dm_regs, but > perhaps not for properties because there the same values might be in a > different order. The order of the properties has no impact, so a duplicate is functionally identical. > > julia > >> > + i1regs i2regs (List.hd p2).line in >> > + print_report p1 msg >> > + >> > +(* ---------------------------------------------------------------- *) >> > + >> > +@getdmregs1@ >> > +identifier tocheck.i1dmregs; >> > +initializer list i1dmregs_vals; >> > +position p1; >> > +@@ >> > + >> > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; >> > + >> > +@getdmregs2@ >> > +identifier tocheck.i2dmregs; >> > +initializer list i2dmregs_vals; >> > +position p2; >> > +@@ >> > + >> > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; >> > + >> > +@script:ocaml@ >> > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; >> > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; >> > +i1dmregs << tocheck.i1dmregs; >> > +i2dmregs << tocheck.i2dmregs; >> > +p1 << getdmregs1.p1; >> > +p2 << getdmregs2.p2; >> > +@@ >> > + >> > +if i1dmregs < i2dmregs && >> > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals >> > +then >> > + let msg = >> > + Printf.sprintf >> > + "WARNING %s and %s (line %d) have the same dm registers\n" >> >> "are identical" vs "have the same..." >> >> > + i1dmregs i2dmregs (List.hd p2).line in >> > + print_report p1 msg >> > + >> > +(* ---------------------------------------------------------------- *) >> > + >> > +@getprops1@ >> > +identifier tocheck.i1props; >> > +initializer list[n1] i1props_vals; >> > +position p1; >> > +@@ >> > + >> > +enum power_supply_property i1props@p1[] = { i1props_vals, }; >> > + >> > +@getprops2@ >> > +identifier tocheck.i2props; >> > +initializer list[n2] i2props_vals; >> > +position p2; >> > +@@ >> > + >> > +enum power_supply_property i2props@p2[] = { i2props_vals, }; >> > + >> > +@script:ocaml@ >> > +(_,i1props_vals) << getprops1.i1props_vals; >> > +(_,i2props_vals) << getprops2.i2props_vals; >> > +i1props << tocheck.i1props; >> > +i2props << tocheck.i2props; >> > +p1 << getprops1.p1; >> > +p2 << getprops2.p2; >> > +@@ >> > + >> > +if i1props < i2props && >> > + List.sort compare i1props_vals = List.sort compare i2props_vals >> > +then >> > + let msg = >> > + Printf.sprintf >> > + "WARNING %s and %s (line %d) have the same properties\n" >> >> "are identical" vs "have the same..." >> >> >> > + i1props i2props (List.hd p2).line in >> > + print_report p1 msg >> > >> -- >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >>
On Thu, 5 Oct 2017, Joe Perches wrote: > On Thu, 2017-10-05 at 21:19 +0200, Julia Lawall wrote: > > On Thu, 5 Oct 2017, Joe Perches wrote: > > > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > > > > within a single structure. It is currently specific to the file > > > > > > drivers/power/supply/bq27xxx_battery.c. > > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > > Applied to linux-kbuild/misc. > > > > Thanks for picking it up. > > > If it is specific to one file, why not just run it > > > and post the resultant patch? Why have it in tree? > > I guess that they anticipate that the data may change in the future? > > Aren't you the script author Julia? Who is the "they"? Liam, etc. > I think having a script for a single file unnecessary. > > btw: spatch 1.7 doesn't seem to have a tag in git > > From the script: > > // Requires: 1.0.7 > > Assuming this is correct, then this doesn't even run today > except maybe on your system. It runs on the current github version. If you don't have the github version it will suffer from false negatives, but nothing will break. julia > $ git show > commit 0bf53049e64295e8ddb625e853134f49568f4bc3 > Merge: 446dc66f8b98 d599cc6a75e0 > Author: julia <julia.lawall@lip6.fr> > Date: Sun Oct 1 22:44:34 2017 +0200 > > Merge git+ssh://palace.lip6.fr/var/git/coccinelle > > $ > > When I build this, the version is 1.0.6 > > $ /usr/local/bin/spatch --version > spatch version 1.0.6-00033-g23cca0a238c2 compiled with OCaml version 4.02.3 > Flags passed to the configure script: [none] > Python scripting support: yes > Syntax of regular expresssions: PCRE > > > If id-utils is used, Coccinelle will completely ignore files that don't > > contain BQ27XXX_DATA, so the rule will have essentially no performance > > impact. If there is no indexing, it will only "grep" for BQ27XXX_DATA, > > not actually parse the files that don't contain it. So there is not much > > performance impact even in that case. > > I'm not sure that matters. > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, 5 Oct 2017, Liam Breck wrote: > Hi Julia, > > On Thu, Oct 5, 2017 at 12:25 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > On Thu, 5 Oct 2017, Liam Breck wrote: > > > >> Hi, sorry for slow reply... > >> > >> Can we patch something to make this script run by default on > >> bq7xxxx_battery_i2c build? If so let's do that. > > > > I don't think anything is set up for that. But any changes to the file > > should be checked by the 0-day bot. > > How do we ask the 0-day bot team to configure their build to run this > script? Will they just pick it up? I put kbuild in CC. I don't know what is the criterion for including semantic patches. julia > > >> Also maybe the name of the script should include "bq27xxx_data"? > > > > OK > > > >> Few more comments below... > >> > >> On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: > >> > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > >> > within a single structure. It is currently specific to the file > >> > drivers/power/supply/bq27xxx_battery.c. > >> > > >> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > >> > > >> > --- > >> > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 161 insertions(+) > >> > > >> > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci > >> > new file mode 100644 > >> > index 0000000..77c145a > >> > --- /dev/null > >> > +++ b/scripts/coccinelle/api/battery.cocci > >> > @@ -0,0 +1,161 @@ > >> > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or > >> > +/// properties. > >> > +//# Doesn't unfold macros used in register or property fields. > >> > +//# Requires OCaml scripting > >> > +/// > >> > +// Confidence: High > >> > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. > >> > +// URL: http://coccinelle.lip6.fr/ > >> > +// Requires: 1.0.7 > >> > +// Keywords: BQ27XXX_DATA > >> > + > >> > +virtual report > >> > + > >> > +@initialize:ocaml@ > >> > +@@ > >> > + > >> > +let print_report p msg = > >> > + let p = List.hd p in > >> > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg > >> > + > >> > +@str depends on report@ > >> > +type t; > >> > +identifier i,i1,i2; > >> > +expression e1,e2; > >> > +@@ > >> > + > >> > +t i[] = { > >> > + ..., > >> > + [e1] = BQ27XXX_DATA(i1,...), > >> > + ..., > >> > + [e2] = BQ27XXX_DATA(i2,...), > >> > + ..., > >> > +}; > >> > + > >> > +@script:ocaml tocheck@ > >> > +i1 << str.i1; > >> > +i2 << str.i2; > >> > +i1regs; i2regs; > >> > +i1dmregs; i2dmregs; > >> > +i1props; i2props; > >> > +@@ > >> > + > >> > +if not(i1 = i2) > >> > +then > >> > + begin > >> > + i1regs := make_ident (i1 ^ "_regs"); > >> > + i2regs := make_ident (i2 ^ "_regs"); > >> > + i1dmregs := make_ident (i1 ^ "_dm_regs"); > >> > + i2dmregs := make_ident (i2 ^ "_dm_regs"); > >> > + i1props := make_ident (i1 ^ "_props"); > >> > + i2props := make_ident (i2 ^ "_props") > >> > + end > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getregs1@ > >> > +typedef u8; > >> > +identifier tocheck.i1regs; > >> > +initializer list i1regs_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +u8 i1regs@p1[...] = { i1regs_vals, }; > >> > + > >> > +@getregs2@ > >> > +identifier tocheck.i2regs; > >> > +initializer list i2regs_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +u8 i2regs@p2[...] = { i2regs_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1regs_vals) << getregs1.i1regs_vals; > >> > +(_,i2regs_vals) << getregs2.i2regs_vals; > >> > +i1regs << tocheck.i1regs; > >> > +i2regs << tocheck.i2regs; > >> > +p1 << getregs1.p1; > >> > +p2 << getregs2.p2; > >> > +@@ > >> > + > >> > +if i1regs < i2regs && > >> > + List.sort compare i1regs_vals = List.sort compare i2regs_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same registers\n" > >> > >> "are identical" vs "have the same..." > > > > OK, I guess identical would be appropriate for regsand dm_regs, but > > perhaps not for properties because there the same values might be in a > > different order. > > The order of the properties has no impact, so a duplicate is > functionally identical. > > > > > julia > > > >> > + i1regs i2regs (List.hd p2).line in > >> > + print_report p1 msg > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getdmregs1@ > >> > +identifier tocheck.i1dmregs; > >> > +initializer list i1dmregs_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; > >> > + > >> > +@getdmregs2@ > >> > +identifier tocheck.i2dmregs; > >> > +initializer list i2dmregs_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; > >> > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; > >> > +i1dmregs << tocheck.i1dmregs; > >> > +i2dmregs << tocheck.i2dmregs; > >> > +p1 << getdmregs1.p1; > >> > +p2 << getdmregs2.p2; > >> > +@@ > >> > + > >> > +if i1dmregs < i2dmregs && > >> > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same dm registers\n" > >> > >> "are identical" vs "have the same..." > >> > >> > + i1dmregs i2dmregs (List.hd p2).line in > >> > + print_report p1 msg > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getprops1@ > >> > +identifier tocheck.i1props; > >> > +initializer list[n1] i1props_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +enum power_supply_property i1props@p1[] = { i1props_vals, }; > >> > + > >> > +@getprops2@ > >> > +identifier tocheck.i2props; > >> > +initializer list[n2] i2props_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +enum power_supply_property i2props@p2[] = { i2props_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1props_vals) << getprops1.i1props_vals; > >> > +(_,i2props_vals) << getprops2.i2props_vals; > >> > +i1props << tocheck.i1props; > >> > +i2props << tocheck.i2props; > >> > +p1 << getprops1.p1; > >> > +p2 << getprops2.p2; > >> > +@@ > >> > + > >> > +if i1props < i2props && > >> > + List.sort compare i1props_vals = List.sort compare i2props_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same properties\n" > >> > >> "are identical" vs "have the same..." > >> > >> > >> > + i1props i2props (List.hd p2).line in > >> > + print_report p1 msg > >> > > >> -- > >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> >
On Thu, 5 Oct 2017, Liam Breck wrote: > Hi Julia, > > On Thu, Oct 5, 2017 at 12:25 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > On Thu, 5 Oct 2017, Liam Breck wrote: > > > >> Hi, sorry for slow reply... > >> > >> Can we patch something to make this script run by default on > >> bq7xxxx_battery_i2c build? If so let's do that. > > > > I don't think anything is set up for that. But any changes to the file > > should be checked by the 0-day bot. > > How do we ask the 0-day bot team to configure their build to run this > script? Will they just pick it up? > > >> Also maybe the name of the script should include "bq27xxx_data"? > > > > OK > > > >> Few more comments below... > >> > >> On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: > >> > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > >> > within a single structure. It is currently specific to the file > >> > drivers/power/supply/bq27xxx_battery.c. > >> > > >> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > >> > > >> > --- > >> > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 161 insertions(+) > >> > > >> > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci > >> > new file mode 100644 > >> > index 0000000..77c145a > >> > --- /dev/null > >> > +++ b/scripts/coccinelle/api/battery.cocci > >> > @@ -0,0 +1,161 @@ > >> > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or > >> > +/// properties. > >> > +//# Doesn't unfold macros used in register or property fields. > >> > +//# Requires OCaml scripting > >> > +/// > >> > +// Confidence: High > >> > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. > >> > +// URL: http://coccinelle.lip6.fr/ > >> > +// Requires: 1.0.7 > >> > +// Keywords: BQ27XXX_DATA > >> > + > >> > +virtual report > >> > + > >> > +@initialize:ocaml@ > >> > +@@ > >> > + > >> > +let print_report p msg = > >> > + let p = List.hd p in > >> > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg > >> > + > >> > +@str depends on report@ > >> > +type t; > >> > +identifier i,i1,i2; > >> > +expression e1,e2; > >> > +@@ > >> > + > >> > +t i[] = { > >> > + ..., > >> > + [e1] = BQ27XXX_DATA(i1,...), > >> > + ..., > >> > + [e2] = BQ27XXX_DATA(i2,...), > >> > + ..., > >> > +}; > >> > + > >> > +@script:ocaml tocheck@ > >> > +i1 << str.i1; > >> > +i2 << str.i2; > >> > +i1regs; i2regs; > >> > +i1dmregs; i2dmregs; > >> > +i1props; i2props; > >> > +@@ > >> > + > >> > +if not(i1 = i2) > >> > +then > >> > + begin > >> > + i1regs := make_ident (i1 ^ "_regs"); > >> > + i2regs := make_ident (i2 ^ "_regs"); > >> > + i1dmregs := make_ident (i1 ^ "_dm_regs"); > >> > + i2dmregs := make_ident (i2 ^ "_dm_regs"); > >> > + i1props := make_ident (i1 ^ "_props"); > >> > + i2props := make_ident (i2 ^ "_props") > >> > + end > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getregs1@ > >> > +typedef u8; > >> > +identifier tocheck.i1regs; > >> > +initializer list i1regs_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +u8 i1regs@p1[...] = { i1regs_vals, }; > >> > + > >> > +@getregs2@ > >> > +identifier tocheck.i2regs; > >> > +initializer list i2regs_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +u8 i2regs@p2[...] = { i2regs_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1regs_vals) << getregs1.i1regs_vals; > >> > +(_,i2regs_vals) << getregs2.i2regs_vals; > >> > +i1regs << tocheck.i1regs; > >> > +i2regs << tocheck.i2regs; > >> > +p1 << getregs1.p1; > >> > +p2 << getregs2.p2; > >> > +@@ > >> > + > >> > +if i1regs < i2regs && > >> > + List.sort compare i1regs_vals = List.sort compare i2regs_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same registers\n" > >> > >> "are identical" vs "have the same..." > > > > OK, I guess identical would be appropriate for regsand dm_regs, but > > perhaps not for properties because there the same values might be in a > > different order. > > The order of the properties has no impact, so a duplicate is > functionally identical. If you prefer identical, I can put that. julia > > > > > julia > > > >> > + i1regs i2regs (List.hd p2).line in > >> > + print_report p1 msg > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getdmregs1@ > >> > +identifier tocheck.i1dmregs; > >> > +initializer list i1dmregs_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; > >> > + > >> > +@getdmregs2@ > >> > +identifier tocheck.i2dmregs; > >> > +initializer list i2dmregs_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; > >> > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; > >> > +i1dmregs << tocheck.i1dmregs; > >> > +i2dmregs << tocheck.i2dmregs; > >> > +p1 << getdmregs1.p1; > >> > +p2 << getdmregs2.p2; > >> > +@@ > >> > + > >> > +if i1dmregs < i2dmregs && > >> > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same dm registers\n" > >> > >> "are identical" vs "have the same..." > >> > >> > + i1dmregs i2dmregs (List.hd p2).line in > >> > + print_report p1 msg > >> > + > >> > +(* ---------------------------------------------------------------- *) > >> > + > >> > +@getprops1@ > >> > +identifier tocheck.i1props; > >> > +initializer list[n1] i1props_vals; > >> > +position p1; > >> > +@@ > >> > + > >> > +enum power_supply_property i1props@p1[] = { i1props_vals, }; > >> > + > >> > +@getprops2@ > >> > +identifier tocheck.i2props; > >> > +initializer list[n2] i2props_vals; > >> > +position p2; > >> > +@@ > >> > + > >> > +enum power_supply_property i2props@p2[] = { i2props_vals, }; > >> > + > >> > +@script:ocaml@ > >> > +(_,i1props_vals) << getprops1.i1props_vals; > >> > +(_,i2props_vals) << getprops2.i2props_vals; > >> > +i1props << tocheck.i1props; > >> > +i2props << tocheck.i2props; > >> > +p1 << getprops1.p1; > >> > +p2 << getprops2.p2; > >> > +@@ > >> > + > >> > +if i1props < i2props && > >> > + List.sort compare i1props_vals = List.sort compare i2props_vals > >> > +then > >> > + let msg = > >> > + Printf.sprintf > >> > + "WARNING %s and %s (line %d) have the same properties\n" > >> > >> "are identical" vs "have the same..." > >> > >> > >> > + i1props i2props (List.hd p2).line in > >> > + print_report p1 msg > >> > > >> -- > >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi Joe, On Thu, Oct 5, 2017 at 12:30 PM, Joe Perches <joe@perches.com> wrote: > On Thu, 2017-10-05 at 21:19 +0200, Julia Lawall wrote: >> On Thu, 5 Oct 2017, Joe Perches wrote: >> > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: >> > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: >> > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: >> > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA >> > > > > within a single structure. It is currently specific to the file >> > > > > drivers/power/supply/bq27xxx_battery.c. >> > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> >> > > > Applied to linux-kbuild/misc. >> > > Thanks for picking it up. >> > If it is specific to one file, why not just run it >> > and post the resultant patch? Why have it in tree? >> I guess that they anticipate that the data may change in the future? > > Aren't you the script author Julia? Who is the "they"? > > I think having a script for a single file unnecessary. Sebastian Reichel, the power-supply maintainer, requested this script. I previously implemented this as a run-time check within ifdef DEBUG, but that was rejected.
On Thu, 2017-10-05 at 21:40 +0200, Julia Lawall wrote: > On Thu, 5 Oct 2017, Joe Perches wrote: > > btw: spatch 1.7 doesn't seem to have a tag in git > > > > From the script: > > > > // Requires: 1.0.7 > > > > Assuming this is correct, then this doesn't even run today > > except maybe on your system. > > It runs on the current github version. If you don't have the github > version it will suffer from false negatives, but nothing will break. fyi: there's no tag for 1.0.7 there either.
On Thu, 2017-10-05 at 12:46 -0700, Liam Breck wrote: > Hi Joe, > > On Thu, Oct 5, 2017 at 12:30 PM, Joe Perches <joe@perches.com> wrote: > > On Thu, 2017-10-05 at 21:19 +0200, Julia Lawall wrote: > > > On Thu, 5 Oct 2017, Joe Perches wrote: > > > > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > > > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > > > > > within a single structure. It is currently specific to the file > > > > > > > drivers/power/supply/bq27xxx_battery.c. > > > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > > > > > > > > > Applied to linux-kbuild/misc. > > > > > > > > > > Thanks for picking it up. > > > > > > > > If it is specific to one file, why not just run it > > > > and post the resultant patch? Why have it in tree? > > > > > > I guess that they anticipate that the data may change in the future? > > > > Aren't you the script author Julia? Who is the "they"? > > > > I think having a script for a single file unnecessary. > > Sebastian Reichel, the power-supply maintainer, requested this script. > I previously implemented this as a run-time check within ifdef DEBUG, > but that was rejected. Hi Liam. If you are the author of the script, you should also have a sign-off/from here right? This also has a copyright of Inria and Julia Lawall which, if you are the author, doesn't seem appropriate. cheers, Joe
On Thu, 5 Oct 2017, Joe Perches wrote: > On Thu, 2017-10-05 at 21:40 +0200, Julia Lawall wrote: > > On Thu, 5 Oct 2017, Joe Perches wrote: > > > btw: spatch 1.7 doesn't seem to have a tag in git > > > > > > From the script: > > > > > > // Requires: 1.0.7 > > > > > > Assuming this is correct, then this doesn't even run today > > > except maybe on your system. > > > > It runs on the current github version. If you don't have the github > > version it will suffer from false negatives, but nothing will break. > > fyi: there's no tag for 1.0.7 there either. Yes, I know. 1.0.7 doesn't exist. I didn't think it would be meaningful to put the actual version I get with 1.0.6 with the commit id, because commit ids aren't ordered. I hoped that 1.0.7 would be released, but I haven't gotten confirmation that all of the makefile issues have been resolved. julia
On Thu, 5 Oct 2017, Joe Perches wrote: > On Thu, 2017-10-05 at 12:46 -0700, Liam Breck wrote: > > Hi Joe, > > > > On Thu, Oct 5, 2017 at 12:30 PM, Joe Perches <joe@perches.com> wrote: > > > On Thu, 2017-10-05 at 21:19 +0200, Julia Lawall wrote: > > > > On Thu, 5 Oct 2017, Joe Perches wrote: > > > > > On Thu, 2017-10-05 at 21:13 +0200, Julia Lawall wrote: > > > > > > On Fri, 6 Oct 2017, Masahiro Yamada wrote: > > > > > > > 2017-10-01 21:42 GMT+09:00 Julia Lawall <Julia.Lawall@lip6.fr>: > > > > > > > > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA > > > > > > > > within a single structure. It is currently specific to the file > > > > > > > > drivers/power/supply/bq27xxx_battery.c. > > > > > > > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > > > > > > > > > > > > Applied to linux-kbuild/misc. > > > > > > > > > > > > Thanks for picking it up. > > > > > > > > > > If it is specific to one file, why not just run it > > > > > and post the resultant patch? Why have it in tree? > > > > > > > > I guess that they anticipate that the data may change in the future? > > > > > > Aren't you the script author Julia? Who is the "they"? > > > > > > I think having a script for a single file unnecessary. > > > > Sebastian Reichel, the power-supply maintainer, requested this script. > > I previously implemented this as a run-time check within ifdef DEBUG, > > but that was rejected. > > Hi Liam. > > If you are the author of the script, you should also have > a sign-off/from here right? > > This also has a copyright of Inria and Julia Lawall > which, if you are the author, doesn't seem appropriate. I am the author. Liam and Sebastian asked for it to be done and to be put into the kernel. julia
On Thu, Oct 05, 2017 at 09:40:51PM +0200, Julia Lawall wrote: > > >On Thu, 5 Oct 2017, Liam Breck wrote: > >> Hi Julia, >> >> On Thu, Oct 5, 2017 at 12:25 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: >> > >> > >> > On Thu, 5 Oct 2017, Liam Breck wrote: >> > >> >> Hi, sorry for slow reply... >> >> >> >> Can we patch something to make this script run by default on >> >> bq7xxxx_battery_i2c build? If so let's do that. >> > >> > I don't think anything is set up for that. But any changes to the file >> > should be checked by the 0-day bot. >> >> How do we ask the 0-day bot team to configure their build to run this >> script? Will they just pick it up? > >I put kbuild in CC. I don't know what is the criterion for including >semantic patches. We may have 3 options: 1) push this patch to upstream, so that all users can run the check (with obvious cost) 2) try to optimize away the extra overheads by improving script/coccicheck to support conditional run of some cocci scripts 3) let 0-day maintain and run a collection of 3rd-party scripts if (1,2) turns out to be hard or not always applicable for some scripts What do you think? At least it'd be easy and quick for us to implement (3). Thanks, Fengguang >> >> Also maybe the name of the script should include "bq27xxx_data"? >> > >> > OK >> > >> >> Few more comments below... >> >> >> >> On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote: >> >> > This semantic patch detects duplicate arrays declared using BQ27XXX_DATA >> >> > within a single structure. It is currently specific to the file >> >> > drivers/power/supply/bq27xxx_battery.c. >> >> > >> >> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> >> >> > >> >> > --- >> >> > scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ >> >> > 1 file changed, 161 insertions(+) >> >> > >> >> > diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci >> >> > new file mode 100644 >> >> > index 0000000..77c145a >> >> > --- /dev/null >> >> > +++ b/scripts/coccinelle/api/battery.cocci >> >> > @@ -0,0 +1,161 @@ >> >> > +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or >> >> > +/// properties. >> >> > +//# Doesn't unfold macros used in register or property fields. >> >> > +//# Requires OCaml scripting >> >> > +/// >> >> > +// Confidence: High >> >> > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. >> >> > +// URL: http://coccinelle.lip6.fr/ >> >> > +// Requires: 1.0.7 >> >> > +// Keywords: BQ27XXX_DATA >> >> > + >> >> > +virtual report >> >> > + >> >> > +@initialize:ocaml@ >> >> > +@@ >> >> > + >> >> > +let print_report p msg = >> >> > + let p = List.hd p in >> >> > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg >> >> > + >> >> > +@str depends on report@ >> >> > +type t; >> >> > +identifier i,i1,i2; >> >> > +expression e1,e2; >> >> > +@@ >> >> > + >> >> > +t i[] = { >> >> > + ..., >> >> > + [e1] = BQ27XXX_DATA(i1,...), >> >> > + ..., >> >> > + [e2] = BQ27XXX_DATA(i2,...), >> >> > + ..., >> >> > +}; >> >> > + >> >> > +@script:ocaml tocheck@ >> >> > +i1 << str.i1; >> >> > +i2 << str.i2; >> >> > +i1regs; i2regs; >> >> > +i1dmregs; i2dmregs; >> >> > +i1props; i2props; >> >> > +@@ >> >> > + >> >> > +if not(i1 = i2) >> >> > +then >> >> > + begin >> >> > + i1regs := make_ident (i1 ^ "_regs"); >> >> > + i2regs := make_ident (i2 ^ "_regs"); >> >> > + i1dmregs := make_ident (i1 ^ "_dm_regs"); >> >> > + i2dmregs := make_ident (i2 ^ "_dm_regs"); >> >> > + i1props := make_ident (i1 ^ "_props"); >> >> > + i2props := make_ident (i2 ^ "_props") >> >> > + end >> >> > + >> >> > +(* ---------------------------------------------------------------- *) >> >> > + >> >> > +@getregs1@ >> >> > +typedef u8; >> >> > +identifier tocheck.i1regs; >> >> > +initializer list i1regs_vals; >> >> > +position p1; >> >> > +@@ >> >> > + >> >> > +u8 i1regs@p1[...] = { i1regs_vals, }; >> >> > + >> >> > +@getregs2@ >> >> > +identifier tocheck.i2regs; >> >> > +initializer list i2regs_vals; >> >> > +position p2; >> >> > +@@ >> >> > + >> >> > +u8 i2regs@p2[...] = { i2regs_vals, }; >> >> > + >> >> > +@script:ocaml@ >> >> > +(_,i1regs_vals) << getregs1.i1regs_vals; >> >> > +(_,i2regs_vals) << getregs2.i2regs_vals; >> >> > +i1regs << tocheck.i1regs; >> >> > +i2regs << tocheck.i2regs; >> >> > +p1 << getregs1.p1; >> >> > +p2 << getregs2.p2; >> >> > +@@ >> >> > + >> >> > +if i1regs < i2regs && >> >> > + List.sort compare i1regs_vals = List.sort compare i2regs_vals >> >> > +then >> >> > + let msg = >> >> > + Printf.sprintf >> >> > + "WARNING %s and %s (line %d) have the same registers\n" >> >> >> >> "are identical" vs "have the same..." >> > >> > OK, I guess identical would be appropriate for regsand dm_regs, but >> > perhaps not for properties because there the same values might be in a >> > different order. >> >> The order of the properties has no impact, so a duplicate is >> functionally identical. >> >> > >> > julia >> > >> >> > + i1regs i2regs (List.hd p2).line in >> >> > + print_report p1 msg >> >> > + >> >> > +(* ---------------------------------------------------------------- *) >> >> > + >> >> > +@getdmregs1@ >> >> > +identifier tocheck.i1dmregs; >> >> > +initializer list i1dmregs_vals; >> >> > +position p1; >> >> > +@@ >> >> > + >> >> > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; >> >> > + >> >> > +@getdmregs2@ >> >> > +identifier tocheck.i2dmregs; >> >> > +initializer list i2dmregs_vals; >> >> > +position p2; >> >> > +@@ >> >> > + >> >> > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; >> >> > + >> >> > +@script:ocaml@ >> >> > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; >> >> > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; >> >> > +i1dmregs << tocheck.i1dmregs; >> >> > +i2dmregs << tocheck.i2dmregs; >> >> > +p1 << getdmregs1.p1; >> >> > +p2 << getdmregs2.p2; >> >> > +@@ >> >> > + >> >> > +if i1dmregs < i2dmregs && >> >> > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals >> >> > +then >> >> > + let msg = >> >> > + Printf.sprintf >> >> > + "WARNING %s and %s (line %d) have the same dm registers\n" >> >> >> >> "are identical" vs "have the same..." >> >> >> >> > + i1dmregs i2dmregs (List.hd p2).line in >> >> > + print_report p1 msg >> >> > + >> >> > +(* ---------------------------------------------------------------- *) >> >> > + >> >> > +@getprops1@ >> >> > +identifier tocheck.i1props; >> >> > +initializer list[n1] i1props_vals; >> >> > +position p1; >> >> > +@@ >> >> > + >> >> > +enum power_supply_property i1props@p1[] = { i1props_vals, }; >> >> > + >> >> > +@getprops2@ >> >> > +identifier tocheck.i2props; >> >> > +initializer list[n2] i2props_vals; >> >> > +position p2; >> >> > +@@ >> >> > + >> >> > +enum power_supply_property i2props@p2[] = { i2props_vals, }; >> >> > + >> >> > +@script:ocaml@ >> >> > +(_,i1props_vals) << getprops1.i1props_vals; >> >> > +(_,i2props_vals) << getprops2.i2props_vals; >> >> > +i1props << tocheck.i1props; >> >> > +i2props << tocheck.i2props; >> >> > +p1 << getprops1.p1; >> >> > +p2 << getprops2.p2; >> >> > +@@ >> >> > + >> >> > +if i1props < i2props && >> >> > + List.sort compare i1props_vals = List.sort compare i2props_vals >> >> > +then >> >> > + let msg = >> >> > + Printf.sprintf >> >> > + "WARNING %s and %s (line %d) have the same properties\n" >> >> >> >> "are identical" vs "have the same..." >> >> >> >> >> >> > + i1props i2props (List.hd p2).line in >> >> > + print_report p1 msg >> >> > >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >>
> > I put kbuild in CC. I don't know what is the criterion for including > > semantic patches. > > We may have 3 options: > > 1) push this patch to upstream, so that all users can run the check > (with obvious cost) I tried on my laptop with only one core and no indexing. In this case, Coccinelle uses git grep to find relevant files. It finds only one. The time for the whole process is 2 seconds. > 2) try to optimize away the extra overheads by improving > script/coccicheck to support conditional run of some cocci scripts With Coccinelle, you can make virtual rules that are defined or not on the command line and determine what does or not happen. At the moment, putting virtual full at the top of the file and making all processing depend on full has the very undesirable effect of causing all of the .cfiles to be parsed, but I can fix that. Another possibility is adding "depends on file in "drivers/power/supply"" to the first rule, which everything else depends on. I doubt that would affect performance, because the git grep already has a high cost for starting a shell, but it would make clearer what is going on. julia > 3) let 0-day maintain and run a collection of 3rd-party scripts if > (1,2) turns out to be hard or not always applicable for some scripts > > What do you think? At least it'd be easy and quick for us to implement (3). > > Thanks, > Fengguang > > > > >> Also maybe the name of the script should include "bq27xxx_data"? > > > > > > > > OK > > > > > > > >> Few more comments below... > > > >> > > > >> On Sun, Oct 1, 2017 at 5:42 AM, Julia Lawall <Julia.Lawall@lip6.fr> > > > wrote: > > > >> > This semantic patch detects duplicate arrays declared using > > > BQ27XXX_DATA > > > >> > within a single structure. It is currently specific to the file > > > >> > drivers/power/supply/bq27xxx_battery.c. > > > >> > > > > >> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> > > > >> > > > > >> > --- > > > >> > scripts/coccinelle/api/battery.cocci | 161 > > > +++++++++++++++++++++++++++++++++++ > > > >> > 1 file changed, 161 insertions(+) > > > >> > > > > >> > diff --git a/scripts/coccinelle/api/battery.cocci > > > b/scripts/coccinelle/api/battery.cocci > > > >> > new file mode 100644 > > > >> > index 0000000..77c145a > > > >> > --- /dev/null > > > >> > +++ b/scripts/coccinelle/api/battery.cocci > > > >> > @@ -0,0 +1,161 @@ > > > >> > +/// Detect BQ27XXX_DATA structures with identical registers, dm > > > registers or > > > >> > +/// properties. > > > >> > +//# Doesn't unfold macros used in register or property fields. > > > >> > +//# Requires OCaml scripting > > > >> > +/// > > > >> > +// Confidence: High > > > >> > +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. > > > >> > +// URL: http://coccinelle.lip6.fr/ > > > >> > +// Requires: 1.0.7 > > > >> > +// Keywords: BQ27XXX_DATA > > > >> > + > > > >> > +virtual report > > > >> > + > > > >> > +@initialize:ocaml@ > > > >> > +@@ > > > >> > + > > > >> > +let print_report p msg = > > > >> > + let p = List.hd p in > > > >> > + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg > > > >> > + > > > >> > +@str depends on report@ > > > >> > +type t; > > > >> > +identifier i,i1,i2; > > > >> > +expression e1,e2; > > > >> > +@@ > > > >> > + > > > >> > +t i[] = { > > > >> > + ..., > > > >> > + [e1] = BQ27XXX_DATA(i1,...), > > > >> > + ..., > > > >> > + [e2] = BQ27XXX_DATA(i2,...), > > > >> > + ..., > > > >> > +}; > > > >> > + > > > >> > +@script:ocaml tocheck@ > > > >> > +i1 << str.i1; > > > >> > +i2 << str.i2; > > > >> > +i1regs; i2regs; > > > >> > +i1dmregs; i2dmregs; > > > >> > +i1props; i2props; > > > >> > +@@ > > > >> > + > > > >> > +if not(i1 = i2) > > > >> > +then > > > >> > + begin > > > >> > + i1regs := make_ident (i1 ^ "_regs"); > > > >> > + i2regs := make_ident (i2 ^ "_regs"); > > > >> > + i1dmregs := make_ident (i1 ^ "_dm_regs"); > > > >> > + i2dmregs := make_ident (i2 ^ "_dm_regs"); > > > >> > + i1props := make_ident (i1 ^ "_props"); > > > >> > + i2props := make_ident (i2 ^ "_props") > > > >> > + end > > > >> > + > > > >> > +(* ---------------------------------------------------------------- > > > *) > > > >> > + > > > >> > +@getregs1@ > > > >> > +typedef u8; > > > >> > +identifier tocheck.i1regs; > > > >> > +initializer list i1regs_vals; > > > >> > +position p1; > > > >> > +@@ > > > >> > + > > > >> > +u8 i1regs@p1[...] = { i1regs_vals, }; > > > >> > + > > > >> > +@getregs2@ > > > >> > +identifier tocheck.i2regs; > > > >> > +initializer list i2regs_vals; > > > >> > +position p2; > > > >> > +@@ > > > >> > + > > > >> > +u8 i2regs@p2[...] = { i2regs_vals, }; > > > >> > + > > > >> > +@script:ocaml@ > > > >> > +(_,i1regs_vals) << getregs1.i1regs_vals; > > > >> > +(_,i2regs_vals) << getregs2.i2regs_vals; > > > >> > +i1regs << tocheck.i1regs; > > > >> > +i2regs << tocheck.i2regs; > > > >> > +p1 << getregs1.p1; > > > >> > +p2 << getregs2.p2; > > > >> > +@@ > > > >> > + > > > >> > +if i1regs < i2regs && > > > >> > + List.sort compare i1regs_vals = List.sort compare i2regs_vals > > > >> > +then > > > >> > + let msg = > > > >> > + Printf.sprintf > > > >> > + "WARNING %s and %s (line %d) have the same registers\n" > > > >> > > > >> "are identical" vs "have the same..." > > > > > > > > OK, I guess identical would be appropriate for regsand dm_regs, but > > > > perhaps not for properties because there the same values might be in a > > > > different order. > > > > > > The order of the properties has no impact, so a duplicate is > > > functionally identical. > > > > > > > > > > > julia > > > > > > > >> > + i1regs i2regs (List.hd p2).line in > > > >> > + print_report p1 msg > > > >> > + > > > >> > +(* ---------------------------------------------------------------- > > > *) > > > >> > + > > > >> > +@getdmregs1@ > > > >> > +identifier tocheck.i1dmregs; > > > >> > +initializer list i1dmregs_vals; > > > >> > +position p1; > > > >> > +@@ > > > >> > + > > > >> > +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; > > > >> > + > > > >> > +@getdmregs2@ > > > >> > +identifier tocheck.i2dmregs; > > > >> > +initializer list i2dmregs_vals; > > > >> > +position p2; > > > >> > +@@ > > > >> > + > > > >> > +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; > > > >> > + > > > >> > +@script:ocaml@ > > > >> > +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; > > > >> > +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; > > > >> > +i1dmregs << tocheck.i1dmregs; > > > >> > +i2dmregs << tocheck.i2dmregs; > > > >> > +p1 << getdmregs1.p1; > > > >> > +p2 << getdmregs2.p2; > > > >> > +@@ > > > >> > + > > > >> > +if i1dmregs < i2dmregs && > > > >> > + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals > > > >> > +then > > > >> > + let msg = > > > >> > + Printf.sprintf > > > >> > + "WARNING %s and %s (line %d) have the same dm registers\n" > > > >> > > > >> "are identical" vs "have the same..." > > > >> > > > >> > + i1dmregs i2dmregs (List.hd p2).line in > > > >> > + print_report p1 msg > > > >> > + > > > >> > +(* ---------------------------------------------------------------- > > > *) > > > >> > + > > > >> > +@getprops1@ > > > >> > +identifier tocheck.i1props; > > > >> > +initializer list[n1] i1props_vals; > > > >> > +position p1; > > > >> > +@@ > > > >> > + > > > >> > +enum power_supply_property i1props@p1[] = { i1props_vals, }; > > > >> > + > > > >> > +@getprops2@ > > > >> > +identifier tocheck.i2props; > > > >> > +initializer list[n2] i2props_vals; > > > >> > +position p2; > > > >> > +@@ > > > >> > + > > > >> > +enum power_supply_property i2props@p2[] = { i2props_vals, }; > > > >> > + > > > >> > +@script:ocaml@ > > > >> > +(_,i1props_vals) << getprops1.i1props_vals; > > > >> > +(_,i2props_vals) << getprops2.i2props_vals; > > > >> > +i1props << tocheck.i1props; > > > >> > +i2props << tocheck.i2props; > > > >> > +p1 << getprops1.p1; > > > >> > +p2 << getprops2.p2; > > > >> > +@@ > > > >> > + > > > >> > +if i1props < i2props && > > > >> > + List.sort compare i1props_vals = List.sort compare i2props_vals > > > >> > +then > > > >> > + let msg = > > > >> > + Printf.sprintf > > > >> > + "WARNING %s and %s (line %d) have the same properties\n" > > > >> > > > >> "are identical" vs "have the same..." > > > >> > > > >> > > > >> > + i1props i2props (List.hd p2).line in > > > >> > + print_report p1 msg > > > >> > > > > >> -- > > > >> To unsubscribe from this list: send the line "unsubscribe > > > kernel-janitors" in > > > >> the body of a message to majordomo@vger.kernel.org > > > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > >> > > > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/scripts/coccinelle/api/battery.cocci b/scripts/coccinelle/api/battery.cocci new file mode 100644 index 0000000..77c145a --- /dev/null +++ b/scripts/coccinelle/api/battery.cocci @@ -0,0 +1,161 @@ +/// Detect BQ27XXX_DATA structures with identical registers, dm registers or +/// properties. +//# Doesn't unfold macros used in register or property fields. +//# Requires OCaml scripting +/// +// Confidence: High +// Copyright: (C) 2017 Julia Lawall, Inria/LIP6, GPLv2. +// URL: http://coccinelle.lip6.fr/ +// Requires: 1.0.7 +// Keywords: BQ27XXX_DATA + +virtual report + +@initialize:ocaml@ +@@ + +let print_report p msg = + let p = List.hd p in + Printf.printf "%s:%d:%d-%d: %s" p.file p.line p.col p.col_end msg + +@str depends on report@ +type t; +identifier i,i1,i2; +expression e1,e2; +@@ + +t i[] = { + ..., + [e1] = BQ27XXX_DATA(i1,...), + ..., + [e2] = BQ27XXX_DATA(i2,...), + ..., +}; + +@script:ocaml tocheck@ +i1 << str.i1; +i2 << str.i2; +i1regs; i2regs; +i1dmregs; i2dmregs; +i1props; i2props; +@@ + +if not(i1 = i2) +then + begin + i1regs := make_ident (i1 ^ "_regs"); + i2regs := make_ident (i2 ^ "_regs"); + i1dmregs := make_ident (i1 ^ "_dm_regs"); + i2dmregs := make_ident (i2 ^ "_dm_regs"); + i1props := make_ident (i1 ^ "_props"); + i2props := make_ident (i2 ^ "_props") + end + +(* ---------------------------------------------------------------- *) + +@getregs1@ +typedef u8; +identifier tocheck.i1regs; +initializer list i1regs_vals; +position p1; +@@ + +u8 i1regs@p1[...] = { i1regs_vals, }; + +@getregs2@ +identifier tocheck.i2regs; +initializer list i2regs_vals; +position p2; +@@ + +u8 i2regs@p2[...] = { i2regs_vals, }; + +@script:ocaml@ +(_,i1regs_vals) << getregs1.i1regs_vals; +(_,i2regs_vals) << getregs2.i2regs_vals; +i1regs << tocheck.i1regs; +i2regs << tocheck.i2regs; +p1 << getregs1.p1; +p2 << getregs2.p2; +@@ + +if i1regs < i2regs && + List.sort compare i1regs_vals = List.sort compare i2regs_vals +then + let msg = + Printf.sprintf + "WARNING %s and %s (line %d) have the same registers\n" + i1regs i2regs (List.hd p2).line in + print_report p1 msg + +(* ---------------------------------------------------------------- *) + +@getdmregs1@ +identifier tocheck.i1dmregs; +initializer list i1dmregs_vals; +position p1; +@@ + +struct bq27xxx_dm_reg i1dmregs@p1[] = { i1dmregs_vals, }; + +@getdmregs2@ +identifier tocheck.i2dmregs; +initializer list i2dmregs_vals; +position p2; +@@ + +struct bq27xxx_dm_reg i2dmregs@p2[] = { i2dmregs_vals, }; + +@script:ocaml@ +(_,i1dmregs_vals) << getdmregs1.i1dmregs_vals; +(_,i2dmregs_vals) << getdmregs2.i2dmregs_vals; +i1dmregs << tocheck.i1dmregs; +i2dmregs << tocheck.i2dmregs; +p1 << getdmregs1.p1; +p2 << getdmregs2.p2; +@@ + +if i1dmregs < i2dmregs && + List.sort compare i1dmregs_vals = List.sort compare i2dmregs_vals +then + let msg = + Printf.sprintf + "WARNING %s and %s (line %d) have the same dm registers\n" + i1dmregs i2dmregs (List.hd p2).line in + print_report p1 msg + +(* ---------------------------------------------------------------- *) + +@getprops1@ +identifier tocheck.i1props; +initializer list[n1] i1props_vals; +position p1; +@@ + +enum power_supply_property i1props@p1[] = { i1props_vals, }; + +@getprops2@ +identifier tocheck.i2props; +initializer list[n2] i2props_vals; +position p2; +@@ + +enum power_supply_property i2props@p2[] = { i2props_vals, }; + +@script:ocaml@ +(_,i1props_vals) << getprops1.i1props_vals; +(_,i2props_vals) << getprops2.i2props_vals; +i1props << tocheck.i1props; +i2props << tocheck.i2props; +p1 << getprops1.p1; +p2 << getprops2.p2; +@@ + +if i1props < i2props && + List.sort compare i1props_vals = List.sort compare i2props_vals +then + let msg = + Printf.sprintf + "WARNING %s and %s (line %d) have the same properties\n" + i1props i2props (List.hd p2).line in + print_report p1 msg
This semantic patch detects duplicate arrays declared using BQ27XXX_DATA within a single structure. It is currently specific to the file drivers/power/supply/bq27xxx_battery.c. Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- scripts/coccinelle/api/battery.cocci | 161 +++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+)