[PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 scripts/coccinelle/misc/boolinit.cocci |  178 +++++++++++++++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/scripts/coccinelle/misc/boolinit.cocci b/scripts/coccinelle/misc/boolinit.cocci
new file mode 100644
index 0000000..b7edfdb
--- /dev/null
+++ b/scripts/coccinelle/misc/boolinit.cocci
@@ -0,0 +1,178 @@
+/// Bool initializations should use true and false.  Bool tests don't need
+/// comparisons.  Based on contributions from Joe Perches, Rusty Russell
+/// and Bruce W Allan.
+///
+// Confidence: High
+// Copyright: (C) 2012 Julia Lawall, INRIA/LIP6.  GPLv2.
+// Copyright: (C) 2012 Gilles Muller, INRIA/LiP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: -include_headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@depends on patch@
+bool t;
+symbol true;
+symbol false;
+@@
+
+(
+- t = true
++ t
+|
+- true = t
++ t
+|
+- t != true
++ !t
+|
+- true != t
++ !t
+|
+- t = false
++ !t
+|
+- false = t
++ !t
+|
+- t != false
++ t
+|
+- false != t
++ t
+)
+
+@depends on patch disable is_zero, isnt_zero@
+bool t;
+@@
+
+(
+- t = 1
++ t
+|
+- t != 1
++ !t
+|
+- t = 0
++ !t
+|
+- t != 0
++ t
+)
+
+@depends on patch@
+bool b;
+@@
+(
+ b +- 0
++ false
+|
+ b +- 1
++ true
+)
+
+// ---------------------------------------------------------------------
+
+@r1 depends on !patch@
+bool t;
+position p;
+@@
+
+(
+* t@p = true
+|
+* true = t@p
+|
+* t@p != true
+|
+* true != t@p
+|
+* t@p = false
+|
+* false = t@p
+|
+* t@p != false
+|
+* false != t@p
+)
+
+@r2 depends on !patch disable is_zero, isnt_zero@
+bool t;
+position p;
+@@
+
+(
+* t@p = 1
+|
+* t@p != 1
+|
+* t@p = 0
+|
+* t@p != 0
+)
+
+@r3 depends on !patch@
+bool b;
+position p1,p2;
+constant c;
+@@
+(
+*b@p1 = 0
+|
+*b@p1 = 1
+|
+*b@p2 = c
+)
+
+@script:python depends on org@
+p << r1.p;
+@@
+
+cocci.print_main("WARNING: Comparison to bool",p)
+
+@script:python depends on org@
+p << r2.p;
+@@
+
+cocci.print_main("ERROR: Comparison of bool to 0/1",p)
+
+@script:python depends on org@
+p1 << r3.p1;
+@@
+
+cocci.print_main("ERROR: Assignment of bool to 0/1",p1)
+
+@script:python depends on org@
+p2 << r3.p2;
+@@
+
+cocci.print_main("ERROR: Assignment of bool to non-0/1 constant",p2)
+
+@script:python depends on report@
+p << r1.p;
+@@
+
+coccilib.report.print_report(p[0],"WARNING: Comparison to bool")
+
+@script:python depends on report@
+p << r2.p;
+@@
+
+coccilib.report.print_report(p[0],"ERROR: Comparison of bool to 0/1")
+
+@script:python depends on report@
+p1 << r3.p1;
+@@
+
+coccilib.report.print_report(p1[0],"ERROR: Assignment of bool to 0/1")
+
+@script:python depends on report@
+p2 << r3.p2;
+@@
+
+coccilib.report.print_report(p2[0],"ERROR: Assignment of bool to non-0/1 constant")

Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Joe Perches]

On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>

Thanks Julia.  It seems to work well.

Someone could run this with:

$ make coccicheck COCCI=scripts/coccinelle/misc/boolinit.cocci

Re: [Cocci] Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On Fri, 2012-02-10 at 09:45 -0800, Joe Perches wrote:
> On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Thanks Julia.  It seems to work well.
> 
> Someone could run this with:
> 
> $ make coccicheck COCCI=scripts/coccinelle/misc/boolinit.cocci

I've executed it for ARCH=i386 and i386_defconfig, I can send the patch
tomorrow, but it going to be big AFAICS.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

On Fri, 2012-02-10 at 09:45 -0800, Joe Perches wrote:
> On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Thanks Julia.  It seems to work well.

I wonder though, why would we want to change if (a == true) with if (a)
etc? Julia did not provide the explanation in the commit message but
referred to you and Rusty in the semantic patch.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Joe Perches]

On Fri, 2012-02-10 at 20:44 +0200, Artem Bityutskiy wrote:
> On Fri, 2012-02-10 at 09:45 -0800, Joe Perches wrote:
> > On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> > > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > Thanks Julia.  It seems to work well.
> I wonder though, why would we want to change if (a = true) with if (a)
> etc? Julia did not provide the explanation in the commit message but
> referred to you and Rusty in the semantic patch.

Testing booleans against specific values is poor form.
booleans should be tested or !tested.

If you are going to submit these patches,
I suggest you break them out by 2nd level
directory.

patch 1: drivers/foo/...
patch 2: drivers/bar/...
...
patch n: drivers/baz/...

and cc: trivial@kernel.org

Re: [Cocci] Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On Fri, 2012-02-10 at 10:51 -0800, Joe Perches wrote:
> On Fri, 2012-02-10 at 20:44 +0200, Artem Bityutskiy wrote:
> > On Fri, 2012-02-10 at 09:45 -0800, Joe Perches wrote:
> > > On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> > > > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > > Thanks Julia.  It seems to work well.
> > I wonder though, why would we want to change if (a == true) with if (a)
> > etc? Julia did not provide the explanation in the commit message but
> > referred to you and Rusty in the semantic patch.
> 
> Testing booleans against specific values is poor form.
> booleans should be tested or !tested.

OK, so this is about taste, I thought there is a more serious reason.

> If you are going to submit these patches,
> I suggest you break them out by 2nd level
> directory.

No, I was just trying to help Julia a bit and let her spend more time
writing good spatches. I am not going to sumbit patches, sorry.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] Re: [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

On Fri, 2012-02-10 at 20:55 +0200, Artem Bityutskiy wrote:
> On Fri, 2012-02-10 at 10:51 -0800, Joe Perches wrote:
> > On Fri, 2012-02-10 at 20:44 +0200, Artem Bityutskiy wrote:
> > > On Fri, 2012-02-10 at 09:45 -0800, Joe Perches wrote:
> > > > On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> > > > > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > > > Thanks Julia.  It seems to work well.
> > > I wonder though, why would we want to change if (a == true) with if (a)
> > > etc? Julia did not provide the explanation in the commit message but
> > > referred to you and Rusty in the semantic patch.
> > 
> > Testing booleans against specific values is poor form.
> > booleans should be tested or !tested.
> 
> OK, so this is about taste, I thought there is a more serious reason.

But let me be clear - I am all for having this spatch in the kernel tree
- it is good for code hygiene.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
> +coccilib.report.print_report(p[0],"ERROR: Comparison of bool to 0/1")

All of these look more like style/taste/hygiene suggestions, and I think
they should be tagged as "WARNING", not "ERROR".

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] [PATCH] scripts/coccinelle/misc/boolinit.cocci: semantic patch for bool issues

[Julia Lawall]

On Fri, 10 Feb 2012, Artem Bityutskiy wrote:

> On Fri, 2012-02-10 at 18:11 +0100, Julia Lawall wrote:
>> +coccilib.report.print_report(p[0],"ERROR: Comparison of bool to 0/1")
>
> All of these look more like style/taste/hygiene suggestions, and I think
> they should be tagged as "WARNING", not "ERROR".

OK, I'll fix that.

julia