[PATCH v2] coccinelle: add style check for assignment in if

[PATCH v2] coccinelle: add style check for assignment in if

[Kris Borer]

Add a semantic patch for fixing some cases of checkpatch.pl error:

ERROR: do not use assignment in if condition

Signed-off-by: Kris Borer <kborer@gmail.com>
---
 scripts/coccinelle/style/assignment_in_if.cocci | 128 ++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 scripts/coccinelle/style/assignment_in_if.cocci

diff --git a/scripts/coccinelle/style/assignment_in_if.cocci b/scripts/coccinelle/style/assignment_in_if.cocci
new file mode 100644
index 0000000..b23db8f
--- /dev/null
+++ b/scripts/coccinelle/style/assignment_in_if.cocci
@@ -0,0 +1,128 @@
+/// move assignments out of if conditions
+///
+//# This script is designed to correct code where assignments exist in if
+//# conditions. It is only capable of handling a subset of such problems.
+//# Ideally it would handle all checkpatch errors of the following type:
+//#	ERROR: do not use assignment in if condition
+//#
+//# For example:
+//#	if(result = myfun())
+//#
+//# would become:
+//#	result = myfun();
+//#	if(result)
+//
+// Confidence: Moderate
+// Copyright: (C) 2015 Kris Borer.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual patch
+
+
+// if ( (ret = call()) )
+// if ( (ret = call()) < 0 )
+@if1@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+- (i = E)
++ i
+  b ...
+|
+- (i = E),
+  E2
+)
+  ) S1 else S2
+
+
+// if ( ptr->fun && (ret = ptr->fun()) )
+@if2@
+expression i2;
+expression E1, E2;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2) {
+- if( E1 && (i2 = E2) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( ptr->fun && (ret = ptr->fun()) < 0 )
+@if3@
+expression i2;
+expression E1, E2;
+constant c;
+binary operator b;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2 b c) {
+- if( E1 && ((i2 = E2) b c) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( (ret = call()) && ret != -1 )
+// if ( (ret = call()) < 0 && ret != -1 )
+@if4@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 ) S1 else S2
+
+
+// if ( (ret = call()) && ret != -1 && ret != -2 )
+// if ( (ret = call()) < 0 && ret != -1 && ret != -2 )
+@if5@
+expression i;
+expression E, E2, E3;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 && E3 ) S1 else S2
-- 
1.9.1

Re: [PATCH v2] coccinelle: add style check for assignment in if

[SF Markus Elfring]

> +virtual patch

How do you think about to use this variable in a subsequent
SmPL rule?

Would you like to consider also the reuse of SmPL variables
like "org" and "report"?

Regards,
Markus

Re: [PATCH v2] coccinelle: add style check for assignment in if

[Julia Lawall]

On Tue, 9 Feb 2016, SF Markus Elfring wrote:

> > +virtual patch
>
> How do you think about to use this variable in a subsequent
> SmPL rule?
>
> Would you like to consider also the reuse of SmPL variables
> like "org" and "report"?

I think that there is no point for these things, because checkpatch
already gives warnings about this issue.

It could be nice for the provided rules to actually depend on patch.  But
in the context of make coccicheck, it doesn't matter, because attempts to
run the rule in another mode will just fail, due to the mode not being
declared.

julia

Re: coccinelle: add style check for assignment in if

[SF Markus Elfring]

>> Would you like to consider also the reuse of SmPL variables
>> like "org" and "report"?
> 
> I think that there is no point for these things, because checkpatch
> already gives warnings about this issue.

Can the check result display be more convenient from the semantic
patch script interface in comparison to the other tool?

Regards,
Markus

Re: coccinelle: add style check for assignment in if

[Joe Perches]

On Tue, 2016-02-09 at 13:55 +0100, SF Markus Elfring wrote:
> > > Would you like to consider also the reuse of SmPL variables
> > > like "org" and "report"?
> > 
> > I think that there is no point for these things, because checkpatch
> > already gives warnings about this issue.
> 
> Can the check result display be more convenient from the semantic
> patch script interface in comparison to the other tool?

-ENOPARSE.  More detail please.

Re: coccinelle: add style check for assignment in if

[SF Markus Elfring]

>> Can the check result display be more convenient from the semantic
>> patch script interface in comparison to the other tool?
> 
> -ENOPARSE.  More detail please.

Kris Borer suggested a SmPL script which can generate (only) patches
so far by the usual application of the Coccinelle software.
I assume that this approach was limited just because Julia Lawall
indicated that this functionality would be sufficient for a while.

The documentation for the command "make coccicheck" mentions
additional modes.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/coccinelle.txt?id=ec97946ed038f4b3faa587bc76152b198805b0c4#n170

They refer to specific output formats for the result from the
static source code analysis.

* How often would you like to look at proposed changes by the
  Org mode format (of Emacs)?
  http://orgmode.org/

* Do you find the report mode also useful occasionally?


Are you interested to get further output variants?

Examples:
* CSV
* XML

Regards,
Markus

Re: coccinelle: add style check for assignment in if

[Kris Borer]

On Wed, Feb 10, 2016 at 4:00 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> Kris Borer suggested a SmPL script which can generate (only) patches
> so far by the usual application of the Coccinelle software.
> I assume that this approach was limited just because Julia Lawall
> indicated that this functionality would be sufficient for a while.
>
> * Do you find the report mode also useful occasionally?
>
> Are you interested to get further output variants?
>
> Examples:
> * CSV
> * XML

I would happy to add additional modes or features, but I would prefer
to do those in a separate patch if possible.

[PATCH v2] coccinelle: add style check for assignment in if

[Kris Borer]

Add a semantic patch for fixing some cases of checkpatch.pl error:

ERROR: do not use assignment in if condition

Signed-off-by: Kris Borer <kborer@gmail.com>
---
 scripts/coccinelle/style/assignment_in_if.cocci | 128 ++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 scripts/coccinelle/style/assignment_in_if.cocci

diff --git a/scripts/coccinelle/style/assignment_in_if.cocci b/scripts/coccinelle/style/assignment_in_if.cocci
new file mode 100644
index 0000000..b23db8f
--- /dev/null
+++ b/scripts/coccinelle/style/assignment_in_if.cocci
@@ -0,0 +1,128 @@
+/// move assignments out of if conditions
+///
+//# This script is designed to correct code where assignments exist in if
+//# conditions. It is only capable of handling a subset of such problems.
+//# Ideally it would handle all checkpatch errors of the following type:
+//#	ERROR: do not use assignment in if condition
+//#
+//# For example:
+//#	if(result = myfun())
+//#
+//# would become:
+//#	result = myfun();
+//#	if(result)
+//
+// Confidence: Moderate
+// Copyright: (C) 2015 Kris Borer.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual patch
+
+
+// if ( (ret = call()) )
+// if ( (ret = call()) < 0 )
+@if1@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+- (i = E)
++ i
+  b ...
+|
+- (i = E),
+  E2
+)
+  ) S1 else S2
+
+
+// if ( ptr->fun && (ret = ptr->fun()) )
+@if2@
+expression i2;
+expression E1, E2;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2) {
+- if( E1 && (i2 = E2) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( ptr->fun && (ret = ptr->fun()) < 0 )
+@if3@
+expression i2;
+expression E1, E2;
+constant c;
+binary operator b;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2 b c) {
+- if( E1 && ((i2 = E2) b c) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( (ret = call()) && ret != -1 )
+// if ( (ret = call()) < 0 && ret != -1 )
+@if4@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 ) S1 else S2
+
+
+// if ( (ret = call()) && ret != -1 && ret != -2 )
+// if ( (ret = call()) < 0 && ret != -1 && ret != -2 )
+@if5@
+expression i;
+expression E, E2, E3;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 && E3 ) S1 else S2
-- 
1.9.1

[PATCH v2] coccinelle: add style check for assignment in if

[Kris Borer]

Add a semantic patch for fixing some cases of checkpatch.pl error:

ERROR: do not use assignment in if condition

Signed-off-by: Kris Borer <kborer@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
 scripts/coccinelle/style/assignment_in_if.cocci | 128 ++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 scripts/coccinelle/style/assignment_in_if.cocci

diff --git a/scripts/coccinelle/style/assignment_in_if.cocci b/scripts/coccinelle/style/assignment_in_if.cocci
new file mode 100644
index 0000000..b23db8f
--- /dev/null
+++ b/scripts/coccinelle/style/assignment_in_if.cocci
@@ -0,0 +1,128 @@
+/// move assignments out of if conditions
+///
+//# This script is designed to correct code where assignments exist in if
+//# conditions. It is only capable of handling a subset of such problems.
+//# Ideally it would handle all checkpatch errors of the following type:
+//#	ERROR: do not use assignment in if condition
+//#
+//# For example:
+//#	if(result = myfun())
+//#
+//# would become:
+//#	result = myfun();
+//#	if(result)
+//
+// Confidence: Moderate
+// Copyright: (C) 2015 Kris Borer.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual patch
+
+
+// if ( (ret = call()) )
+// if ( (ret = call()) < 0 )
+@if1@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+- (i = E)
++ i
+  b ...
+|
+- (i = E),
+  E2
+)
+  ) S1 else S2
+
+
+// if ( ptr->fun && (ret = ptr->fun()) )
+@if2@
+expression i2;
+expression E1, E2;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2) {
+- if( E1 && (i2 = E2) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( ptr->fun && (ret = ptr->fun()) < 0 )
+@if3@
+expression i2;
+expression E1, E2;
+constant c;
+binary operator b;
+@@
+
++ if( E1 ) {
++       i2 = E2;
++       if (i2 b c) {
+- if( E1 && ((i2 = E2) b c) ) {
+  ...
+- }
++       }
++ }
+
+
+// if ( (ret = call()) && ret != -1 )
+// if ( (ret = call()) < 0 && ret != -1 )
+@if4@
+expression i;
+expression E, E2;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 ) S1 else S2
+
+
+// if ( (ret = call()) && ret != -1 && ret != -2 )
+// if ( (ret = call()) < 0 && ret != -1 && ret != -2 )
+@if5@
+expression i;
+expression E, E2, E3;
+statement S1, S2;
+binary operator b;
+@@
+
++ i = E;
+  if (
+(
+- (i = E)
++ i
+|
+  (
+- (i = E)
++ i
+  b
+  ...)
+)
+  && E2 && E3 ) S1 else S2
-- 
1.9.1