[PATCH 0/7] adjust suspicious bit operation

[PATCH 0/7] adjust suspicious bit operation

[Julia Lawall]

These patches relate to conditional tests where & is used with a constant
that is always 0 and where | is used with a constant that is always 0 or
always non-zero.

The following semantic match finds these problems
(http://coccinelle.lip6.fr/):

@and@
identifier i;
expression e;
position p;
@@

 ((e & i@p) && ...)

@iszera@
identifier and.i;
position p;
@@

#define i 0

@othera@
identifier and.i;
expression e!=0;
@@

#define i e

@script:python depends on iszera && !othera@
p << and.p;
@@

cocci.print_main("",p)

@or@
identifier i;
expression e;
position p;
@@

 ((e | i@p) && ...)


@iszero@
identifier or.i;
position p;
@@

#define i 0

@othero@
identifier or.i;
expression e!=0;
@@

#define i e

@script:python depends on (othero && !iszero) || (iszero && !othero)@
p << or.p;
@@

cocci.print_main("",p)

[PATCH 0/7] adjust suspicious bit operation

[Julia Lawall]

These patches relate to conditional tests where & is used with a constant
that is always 0 and where | is used with a constant that is always 0 or
always non-zero.

The following semantic match finds these problems
(http://coccinelle.lip6.fr/):

@and@
identifier i;
expression e;
position p;
@@

 ((e & i@p) && ...)

@iszera@
identifier and.i;
position p;
@@

#define i 0

@othera@
identifier and.i;
expression e!=0;
@@

#define i e

@script:python depends on iszera && !othera@
p << and.p;
@@

cocci.print_main("",p)

@or@
identifier i;
expression e;
position p;
@@

 ((e | i@p) && ...)


@iszero@
identifier or.i;
position p;
@@

#define i 0

@othero@
identifier or.i;
expression e!=0;
@@

#define i e

@script:python depends on (othero && !iszero) || (iszero && !othero)@
p << or.p;
@@

cocci.print_main("",p)

[PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/ide/ide-cs.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..a5e57bf 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;
 
-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

[PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/ide/ide-cs.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..a5e57bf 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;
 
-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Wed,  6 Jun 2012 23:41:35 +0200

> -	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
> +	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)

I'm really surprised someone as thorough as yourself did not
compile test this.

This is just more proof that it's absolutely pointless to make any
changes at all to the old IDE layer.  Nobody really cares, and the
risk %99.999 of the time is purely to introduce regressions rather
than make forward progress.

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Wed,  6 Jun 2012 23:41:35 +0200

> -	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
> +	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)

I'm really surprised someone as thorough as yourself did not
compile test this.

This is just more proof that it's absolutely pointless to make any
changes at all to the old IDE layer.  Nobody really cares, and the
risk %99.999 of the time is purely to introduce regressions rather
than make forward progress.

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Joe Perches]

On Thu, 2012-06-07 at 14:46 -0700, David Miller wrote:
> the
> risk %99.999 of the time is purely to introduce regressions rather
> than make forward progress.

 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b54f50c..41d4586 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3404,7 +3404,7 @@ M:	"David S. Miller" <davem@davemloft.net>
 L:	linux-ide@vger.kernel.org
 Q:	http://patchwork.ozlabs.org/project/linux-ide/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git
-S:	Maintained
+S:	Watchdogged - Patch at your own peril
 F:	Documentation/ide/
 F:	drivers/ide/
 F:	include/linux/ide.h

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Joe Perches]

On Thu, 2012-06-07 at 14:46 -0700, David Miller wrote:
> the
> risk %99.999 of the time is purely to introduce regressions rather
> than make forward progress.

 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b54f50c..41d4586 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3404,7 +3404,7 @@ M:	"David S. Miller" <davem@davemloft.net>
 L:	linux-ide@vger.kernel.org
 Q:	http://patchwork.ozlabs.org/project/linux-ide/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git
-S:	Maintained
+S:	Watchdogged - Patch at your own peril
 F:	Documentation/ide/
 F:	drivers/ide/
 F:	include/linux/ide.h

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

On Thu, 7 Jun 2012, David Miller wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> Date: Wed,  6 Jun 2012 23:41:35 +0200
>
>> -	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
>> +	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)
>
> I'm really surprised someone as thorough as yourself did not
> compile test this.

Sorry, I changed the patch at the last minute, and it was indeed not a 
good idea not to have compiled again.  I will send another version, in 
case it is useful.

julia

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

On Thu, 7 Jun 2012, David Miller wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> Date: Wed,  6 Jun 2012 23:41:35 +0200
>
>> -	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
>> +	if ((p1dev->resource[0]->flags & IO_DATA_PATH_WIDTH)
>
> I'm really surprised someone as thorough as yourself did not
> compile test this.

Sorry, I changed the patch at the last minute, and it was indeed not a 
good idea not to have compiled again.  I will send another version, in 
case it is useful.

julia

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

From nobody Wed Jun  6 21:48:37 CEST 2012
From: Julia Lawall <Julia.Lawall@lip6.fr>
To: "David S. Miller" <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org,linux-kernel@vger.kernel.org,joe@perches.com
Subject: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
  drivers/ide/ide-cs.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..f1e922e 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
  {
  	int *is_kme = priv_data;

-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
  		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
  		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
  	}

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

>From nobody Wed Jun  6 21:48:37 CEST 2012
From: Julia Lawall <Julia.Lawall@lip6.fr>
To: "David S. Miller" <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org,linux-kernel@vger.kernel.org,joe@perches.com
Subject: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
  drivers/ide/ide-cs.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..f1e922e 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
  {
  	int *is_kme = priv_data;

-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
  		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
  		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
  	}

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[walter harms]

Am 08.06.2012 07:14, schrieb Julia Lawall:
>> From nobody Wed Jun  6 21:48:37 CEST 2012
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> To: "David S. Miller" <davem@davemloft.net>
> Cc: linux-ide@vger.kernel.org,linux-kernel@vger.kernel.org,joe@perches.com
> Subject: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
> 
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
> value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
> first pick those bits and then make the test using !=.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/ide/ide-cs.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
> index 28e344e..f1e922e 100644
> --- a/drivers/ide/ide-cs.c
> +++ b/drivers/ide/ide-cs.c
> @@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct
> pcmcia_device *pdev, void *priv_data)
>  {
>      int *is_kme = priv_data;
> 
> -    if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
> +    if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
> +        != IO_DATA_PATH_WIDTH_8) {
>          pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
>          pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
>      }
> -- 

please patch until it is removed from the kernel code.
this will make checks happy and as fact of life such bug tend to show up
on surprising unpleasant times.

re,
 wh

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[walter harms]

Am 08.06.2012 07:14, schrieb Julia Lawall:
>> From nobody Wed Jun  6 21:48:37 CEST 2012
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> To: "David S. Miller" <davem@davemloft.net>
> Cc: linux-ide@vger.kernel.org,linux-kernel@vger.kernel.org,joe@perches.com
> Subject: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
> 
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
> value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
> first pick those bits and then make the test using !=.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/ide/ide-cs.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
> index 28e344e..f1e922e 100644
> --- a/drivers/ide/ide-cs.c
> +++ b/drivers/ide/ide-cs.c
> @@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct
> pcmcia_device *pdev, void *priv_data)
>  {
>      int *is_kme = priv_data;
> 
> -    if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
> +    if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
> +        != IO_DATA_PATH_WIDTH_8) {
>          pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
>          pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
>      }
> -- 

please patch until it is removed from the kernel code.
this will make checks happy and as fact of life such bug tend to show up
on surprising unpleasant times.

re,
 wh

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <julia.lawall@lip6.fr>
Date: Fri, 8 Jun 2012 07:14:03 +0200 (CEST)

> @@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct
> pcmcia_device *pdev, void *priv_data)

Patch mangled by your email client or similar.

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <julia.lawall@lip6.fr>
Date: Fri, 8 Jun 2012 07:14:03 +0200 (CEST)

> @@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct
> pcmcia_device *pdev, void *priv_data)

Patch mangled by your email client or similar.

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

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

---
I hope to have fixed the mail client problem.

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..f1e922e 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;

-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

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

---
I hope to have fixed the mail client problem.

diff --git a/drivers/ide/ide-cs.c b/drivers/ide/ide-cs.c
index 28e344e..f1e922e 100644
--- a/drivers/ide/ide-cs.c
+++ b/drivers/ide/ide-cs.c
@@ -167,7 +167,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;

-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Tue, 12 Jun 2012 10:17:25 -0400 (EDT)

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
> value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
> first pick those bits and then make the test using !=.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied, thanks.

Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Tue, 12 Jun 2012 10:17:25 -0400 (EDT)

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
> value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
> first pick those bits and then make the test using !=.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied, thanks.

[PATCH 2/7] drivers/staging/comedi/drivers/me4000.c: adjust suspicious bit operation

[Julia Lawall]

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

TRIG_ROUND_NEAREST is 0, so a bit-and with it is always false.  The
value TRIG_ROUND_MASK covers the bits of the TRIG_ROUND constants, so
first pick those bits and then make the test using =.

The same is done for TRIG_ROUND_UP for symmetry, even though bit-and would
be sufficient in this case.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
The TRIG_ROUND_UP case could be left as is, if that is preferred.  Or a
temporary variable could be introduced to avoid anding with the mask twice.

Actually, the following code suggests that the mask is not necessary, = would be sufficient:
	/* Only rounding flags are implemented */
        cmd->flags &= TRIG_ROUND_NEAREST | TRIG_ROUND_UP | TRIG_ROUND_DOWN;
But using the mask is safer, if there are future extensions.

 drivers/staging/comedi/drivers/me4000.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c
index 8ca1b54..fc60117 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -949,10 +949,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*init_ticks = (cmd->start_arg * 33) / 1000;
 		rest = (cmd->start_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*init_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_UP) {
 			if (rest)
 				(*init_ticks)++;
 		}
@@ -962,10 +962,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*scan_ticks = (cmd->scan_begin_arg * 33) / 1000;
 		rest = (cmd->scan_begin_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*scan_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_UP) {
 			if (rest)
 				(*scan_ticks)++;
 		}
@@ -975,10 +975,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*chan_ticks = (cmd->convert_arg * 33) / 1000;
 		rest = (cmd->convert_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*chan_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) = TRIG_ROUND_UP) {
 			if (rest)
 				(*chan_ticks)++;
 		}

[PATCH 2/7] drivers/staging/comedi/drivers/me4000.c: adjust suspicious bit operation

[Julia Lawall]

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

TRIG_ROUND_NEAREST is 0, so a bit-and with it is always false.  The
value TRIG_ROUND_MASK covers the bits of the TRIG_ROUND constants, so
first pick those bits and then make the test using ==.

The same is done for TRIG_ROUND_UP for symmetry, even though bit-and would
be sufficient in this case.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
The TRIG_ROUND_UP case could be left as is, if that is preferred.  Or a
temporary variable could be introduced to avoid anding with the mask twice.

Actually, the following code suggests that the mask is not necessary, == would be sufficient:
	/* Only rounding flags are implemented */
        cmd->flags &= TRIG_ROUND_NEAREST | TRIG_ROUND_UP | TRIG_ROUND_DOWN;
But using the mask is safer, if there are future extensions.

 drivers/staging/comedi/drivers/me4000.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c
index 8ca1b54..fc60117 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -949,10 +949,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*init_ticks = (cmd->start_arg * 33) / 1000;
 		rest = (cmd->start_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*init_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_UP) {
 			if (rest)
 				(*init_ticks)++;
 		}
@@ -962,10 +962,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*scan_ticks = (cmd->scan_begin_arg * 33) / 1000;
 		rest = (cmd->scan_begin_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*scan_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_UP) {
 			if (rest)
 				(*scan_ticks)++;
 		}
@@ -975,10 +975,10 @@ static int ai_round_cmd_args(struct comedi_device *dev,
 		*chan_ticks = (cmd->convert_arg * 33) / 1000;
 		rest = (cmd->convert_arg * 33) % 1000;
 
-		if (cmd->flags & TRIG_ROUND_NEAREST) {
+		if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_NEAREST) {
 			if (rest > 33)
 				(*chan_ticks)++;
-		} else if (cmd->flags & TRIG_ROUND_UP) {
+		} else if ((cmd->flags & TRIG_ROUND_MASK) == TRIG_ROUND_UP) {
 			if (rest)
 				(*chan_ticks)++;
 		}

[PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Julia Lawall]

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

PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
nonzero result.  The test is rewritten as done elsewhere in the same file.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/usb/gadget/pch_udc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
index 1cfcc9e..79f7a53 100644
--- a/drivers/usb/gadget/pch_udc.c
+++ b/drivers/usb/gadget/pch_udc.c
@@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
 			return;
 		}
 		if ((td->status & PCH_UDC_BUFF_STS) = PCH_UDC_BS_DMA_DONE)
-			if (td->status | PCH_UDC_DMA_LAST) {
+			if ((td_data->status & PCH_UDC_DMA_LAST)
+			    = PCH_UDC_DMA_LAST) {
 				count = td->status & PCH_UDC_RXTX_BYTES;
 				break;
 			}

[PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Julia Lawall]

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

PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
nonzero result.  The test is rewritten as done elsewhere in the same file.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/usb/gadget/pch_udc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
index 1cfcc9e..79f7a53 100644
--- a/drivers/usb/gadget/pch_udc.c
+++ b/drivers/usb/gadget/pch_udc.c
@@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
 			return;
 		}
 		if ((td->status & PCH_UDC_BUFF_STS) == PCH_UDC_BS_DMA_DONE)
-			if (td->status | PCH_UDC_DMA_LAST) {
+			if ((td_data->status & PCH_UDC_DMA_LAST)
+			    == PCH_UDC_DMA_LAST) {
 				count = td->status & PCH_UDC_RXTX_BYTES;
 				break;
 			}

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Sergei Shtylyov]

Hello.

On 07-06-2012 1:41, Julia Lawall wrote:

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

> PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
> nonzero result.  The test is rewritten as done elsewhere in the same file.

> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

> Signed-off-by: Julia Lawall<julia@diku.dk>

> ---
>   drivers/usb/gadget/pch_udc.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
> index 1cfcc9e..79f7a53 100644
> --- a/drivers/usb/gadget/pch_udc.c
> +++ b/drivers/usb/gadget/pch_udc.c
> @@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
>   			return;
>   		}
>   		if ((td->status&  PCH_UDC_BUFF_STS) = PCH_UDC_BS_DMA_DONE)
> -			if (td->status | PCH_UDC_DMA_LAST) {
> +			if ((td_data->status&  PCH_UDC_DMA_LAST)
> +			    = PCH_UDC_DMA_LAST) {

    But why not simply:

			if (td_data->status & PCH_UDC_DMA_LAST)

if the constant is single bit anyway? And are you sure about s/td/td_data/?

WBR, Sergei

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Sergei Shtylyov]

Hello.

On 07-06-2012 1:41, Julia Lawall wrote:

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

> PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
> nonzero result.  The test is rewritten as done elsewhere in the same file.

> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

> Signed-off-by: Julia Lawall<julia@diku.dk>

> ---
>   drivers/usb/gadget/pch_udc.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
> index 1cfcc9e..79f7a53 100644
> --- a/drivers/usb/gadget/pch_udc.c
> +++ b/drivers/usb/gadget/pch_udc.c
> @@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
>   			return;
>   		}
>   		if ((td->status&  PCH_UDC_BUFF_STS) == PCH_UDC_BS_DMA_DONE)
> -			if (td->status | PCH_UDC_DMA_LAST) {
> +			if ((td_data->status&  PCH_UDC_DMA_LAST)
> +			    == PCH_UDC_DMA_LAST) {

    But why not simply:

			if (td_data->status & PCH_UDC_DMA_LAST)

if the constant is single bit anyway? And are you sure about s/td/td_data/?

WBR, Sergei

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Julia Lawall]

On Thu, 7 Jun 2012, Sergei Shtylyov wrote:

> Hello.
>
> On 07-06-2012 1:41, Julia Lawall wrote:
>
>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
>> PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
>> nonzero result.  The test is rewritten as done elsewhere in the same file.
>
>> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
>
>> Signed-off-by: Julia Lawall<julia@diku.dk>
>
>> ---
>>   drivers/usb/gadget/pch_udc.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
>> index 1cfcc9e..79f7a53 100644
>> --- a/drivers/usb/gadget/pch_udc.c
>> +++ b/drivers/usb/gadget/pch_udc.c
>> @@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct 
>> pch_udc_ep *ep)
>>   			return;
>>   		}
>>   		if ((td->status&  PCH_UDC_BUFF_STS) = PCH_UDC_BS_DMA_DONE)
>> -			if (td->status | PCH_UDC_DMA_LAST) {
>> +			if ((td_data->status&  PCH_UDC_DMA_LAST)
>> +			    = PCH_UDC_DMA_LAST) {
>
>   But why not simply:
>
> 			if (td_data->status & PCH_UDC_DMA_LAST)
>
> if the constant is single bit anyway? And are you sure about s/td/td_data/?

Oops, thanks for noticing that.  I'll send a corrected version.

thanks,
julia

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Julia Lawall]

On Thu, 7 Jun 2012, Sergei Shtylyov wrote:

> Hello.
>
> On 07-06-2012 1:41, Julia Lawall wrote:
>
>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
>> PCH_UDC_DMA_LAST is 0x08000000 so a bit-or with this value always gives a
>> nonzero result.  The test is rewritten as done elsewhere in the same file.
>
>> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
>
>> Signed-off-by: Julia Lawall<julia@diku.dk>
>
>> ---
>>   drivers/usb/gadget/pch_udc.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
>> index 1cfcc9e..79f7a53 100644
>> --- a/drivers/usb/gadget/pch_udc.c
>> +++ b/drivers/usb/gadget/pch_udc.c
>> @@ -2208,7 +2208,8 @@ static void pch_udc_complete_receiver(struct 
>> pch_udc_ep *ep)
>>   			return;
>>   		}
>>   		if ((td->status&  PCH_UDC_BUFF_STS) == PCH_UDC_BS_DMA_DONE)
>> -			if (td->status | PCH_UDC_DMA_LAST) {
>> +			if ((td_data->status&  PCH_UDC_DMA_LAST)
>> +			    == PCH_UDC_DMA_LAST) {
>
>   But why not simply:
>
> 			if (td_data->status & PCH_UDC_DMA_LAST)
>
> if the constant is single bit anyway? And are you sure about s/td/td_data/?

Oops, thanks for noticing that.  I'll send a corrected version.

thanks,
julia

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Joe Perches]

On Thu, 2012-06-07 at 16:23 +0200, Julia Lawall wrote:
> Oops, thanks for noticing that.  I'll send a corrected version.

Hi Julia.

I should have let you know that I already submitted
a patch series for a simple treewide
"s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
https://lkml.org/lkml/2012/5/30/625

Felipe has already picked this patch up.
https://lkml.org/lkml/2012/6/1/83

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Joe Perches]

On Thu, 2012-06-07 at 16:23 +0200, Julia Lawall wrote:
> Oops, thanks for noticing that.  I'll send a corrected version.

Hi Julia.

I should have let you know that I already submitted
a patch series for a simple treewide
"s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
https://lkml.org/lkml/2012/5/30/625

Felipe has already picked this patch up.
https://lkml.org/lkml/2012/6/1/83

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Dan Carpenter]

On Thu, Jun 07, 2012 at 08:05:44AM -0700, Joe Perches wrote:
> On Thu, 2012-06-07 at 16:23 +0200, Julia Lawall wrote:
> > Oops, thanks for noticing that.  I'll send a corrected version.
> 
> Hi Julia.
> 
> I should have let you know that I already submitted
> a patch series for a simple treewide
> "s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
> https://lkml.org/lkml/2012/5/30/625
> 

You could CC them to kernel-janitors@vger.kernel.org.  Julia and I
used to tread on each other's toes until we hijacked the k-j list.

regards,
dan carpenter

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Dan Carpenter]

On Thu, Jun 07, 2012 at 08:05:44AM -0700, Joe Perches wrote:
> On Thu, 2012-06-07 at 16:23 +0200, Julia Lawall wrote:
> > Oops, thanks for noticing that.  I'll send a corrected version.
> 
> Hi Julia.
> 
> I should have let you know that I already submitted
> a patch series for a simple treewide
> "s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
> https://lkml.org/lkml/2012/5/30/625
> 

You could CC them to kernel-janitors@vger.kernel.org.  Julia and I
used to tread on each other's toes until we hijacked the k-j list.

regards,
dan carpenter

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Joe Perches]

On Fri, 2012-06-08 at 15:58 +0300, Dan Carpenter wrote:
> On Thu, Jun 07, 2012 at 08:05:44AM -0700, Joe Perches wrote:
> > I should have let you know that I already submitted
> > a patch series for a simple treewide
> > "s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
> > https://lkml.org/lkml/2012/5/30/625
> You could CC them to kernel-janitors@vger.kernel.org.

Hi Dan,

Generally I submit patches to maintainers.
All of these 4 patches were picked up by them.

cheers, Joe

Re: [PATCH 3/7] drivers/usb/gadget/pch_udc.c: adjust suspicious bit operation

[Joe Perches]

On Fri, 2012-06-08 at 15:58 +0300, Dan Carpenter wrote:
> On Thu, Jun 07, 2012 at 08:05:44AM -0700, Joe Perches wrote:
> > I should have let you know that I already submitted
> > a patch series for a simple treewide
> > "s/if (value | UC_CONSTANT)/if (value & UC_CONSTANT)/"
> > https://lkml.org/lkml/2012/5/30/625
> You could CC them to kernel-janitors@vger.kernel.org.

Hi Dan,

Generally I submit patches to maintainers.
All of these 4 patches were picked up by them.

cheers, Joe

[PATCH 4/7] fs/direct-io.c: adjust suspicious bit operation

[Julia Lawall]

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

READ is 0, so the result of the bit-and operation is 0.  Rewrite with = as
done elsewhere in the same file.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/direct-io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 0c85fae..1faf4cb 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1258,7 +1258,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
 	 */
 	BUG_ON(retval = -EIOCBQUEUED);
 	if (dio->is_async && retval = 0 && dio->result &&
-	    ((rw & READ) || (dio->result = sdio.size)))
+	    ((rw = READ) || (dio->result = sdio.size)))
 		retval = -EIOCBQUEUED;
 
 	if (retval != -EIOCBQUEUED)

[PATCH 4/7] fs/direct-io.c: adjust suspicious bit operation

[Julia Lawall]

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

READ is 0, so the result of the bit-and operation is 0.  Rewrite with == as
done elsewhere in the same file.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/direct-io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 0c85fae..1faf4cb 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1258,7 +1258,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
 	 */
 	BUG_ON(retval == -EIOCBQUEUED);
 	if (dio->is_async && retval == 0 && dio->result &&
-	    ((rw & READ) || (dio->result == sdio.size)))
+	    ((rw == READ) || (dio->result == sdio.size)))
 		retval = -EIOCBQUEUED;
 
 	if (retval != -EIOCBQUEUED)

Re: [PATCH 4/7] fs/direct-io.c: adjust suspicious bit operation

[Jeff Moyer]

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

> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> READ is 0, so the result of the bit-and operation is 0.  Rewrite with = as
> done elsewhere in the same file.
>
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Interesting catch, that one.  Yeah, we don't expect any readahead I/O to
be done via O_DIRECT.  The test for rw = READ all over that file seems
fine to me, given that.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

Re: [PATCH 4/7] fs/direct-io.c: adjust suspicious bit operation

[Jeff Moyer]

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

> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> READ is 0, so the result of the bit-and operation is 0.  Rewrite with == as
> done elsewhere in the same file.
>
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Interesting catch, that one.  Yeah, we don't expect any readahead I/O to
be done via O_DIRECT.  The test for rw == READ all over that file seems
fine to me, given that.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

[PATCH 5/7] drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c: adjust suspicious bit operation

[Julia Lawall]

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

IRQF_TRIGGER_HIGH is 0x00000004, so it seems that & was intended rather than |.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index e2480d1..bed208f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -91,7 +91,7 @@ int brcmf_sdio_intr_register(struct brcmf_sdio_dev *sdiodev)
 
 	/* redirect, configure ane enable io for interrupt signal */
 	data = SDIO_SEPINT_MASK | SDIO_SEPINT_OE;
-	if (sdiodev->irq_flags | IRQF_TRIGGER_HIGH)
+	if (sdiodev->irq_flags & IRQF_TRIGGER_HIGH)
 		data |= SDIO_SEPINT_ACT_HI;
 	brcmf_sdio_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, data, &ret);
 

[PATCH 5/7] drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c: adjust suspicious bit operation

[Julia Lawall]

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

IRQF_TRIGGER_HIGH is 0x00000004, so it seems that & was intended rather than |.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index e2480d1..bed208f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -91,7 +91,7 @@ int brcmf_sdio_intr_register(struct brcmf_sdio_dev *sdiodev)
 
 	/* redirect, configure ane enable io for interrupt signal */
 	data = SDIO_SEPINT_MASK | SDIO_SEPINT_OE;
-	if (sdiodev->irq_flags | IRQF_TRIGGER_HIGH)
+	if (sdiodev->irq_flags & IRQF_TRIGGER_HIGH)
 		data |= SDIO_SEPINT_ACT_HI;
 	brcmf_sdio_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, data, &ret);
 

Re: [PATCH 5/7] drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c: adjust suspicious bit

[Franky Lin]

On 06/06/2012 02:41 PM, Julia Lawall wrote:
> From: Julia Lawall<Julia.Lawall@lip6.fr>
>
> IRQF_TRIGGER_HIGH is 0x00000004, so it seems that&  was intended rather than |.
>
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
>
> Signed-off-by: Julia Lawall<julia@diku.dk>

Thanks, Julia. But this has already been fixed by Joe Perches [1] and 
the patch has arrived at Linux wireless tree.

Franky

[1] https://lkml.org/lkml/2012/5/30/482

Re: [PATCH 5/7] drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c: adjust suspicious bit operation

[Franky Lin]

On 06/06/2012 02:41 PM, Julia Lawall wrote:
> From: Julia Lawall<Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
>
> IRQF_TRIGGER_HIGH is 0x00000004, so it seems that&  was intended rather than |.
>
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
>
> Signed-off-by: Julia Lawall<julia-dAYI7NvHqcQ@public.gmane.org>

Thanks, Julia. But this has already been fixed by Joe Perches [1] and 
the patch has arrived at Linux wireless tree.

Franky

[1] https://lkml.org/lkml/2012/5/30/482

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c: adjust suspicious bit operation

[Franky Lin]

On 06/06/2012 02:41 PM, Julia Lawall wrote:
> From: Julia Lawall<Julia.Lawall@lip6.fr>
>
> IRQF_TRIGGER_HIGH is 0x00000004, so it seems that&  was intended rather than |.
>
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
>
> Signed-off-by: Julia Lawall<julia@diku.dk>

Thanks, Julia. But this has already been fixed by Joe Perches [1] and 
the patch has arrived at Linux wireless tree.

Franky

[1] https://lkml.org/lkml/2012/5/30/482

[PATCH 6/7] drivers/ata/pata_pcmcia.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/ata/pata_pcmcia.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index a808ba0..776e749 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -170,7 +170,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;
 
-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

[PATCH 6/7] drivers/ata/pata_pcmcia.c: adjust suspicious bit operation

[Julia Lawall]

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

IO_DATA_PATH_WIDTH_8 is 0, so a bit-and with it is always false.  The
value IO_DATA_PATH_WIDTH covers the bits of the IO_DATA_PATH constants, so
first pick those bits and then make the test using !=.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/ata/pata_pcmcia.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index a808ba0..776e749 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -170,7 +170,8 @@ static int pcmcia_check_one_config(struct pcmcia_device *pdev, void *priv_data)
 {
 	int *is_kme = priv_data;
 
-	if (!(pdev->resource[0]->flags & IO_DATA_PATH_WIDTH_8)) {
+	if ((pdev->resource[0]->flags & IO_DATA_PATH_WIDTH)
+	    != IO_DATA_PATH_WIDTH_8) {
 		pdev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
 		pdev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
 	}

[PATCH 7/7] drivers/net/can/cc770/cc770_platform.c: adjust suspicious bit operation

[Julia Lawall]

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

CPUIF_DSC is 0x40, so it would seem that & is wanted rather than |.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
index 53115ee..688371c 100644
--- a/drivers/net/can/cc770/cc770_platform.c
+++ b/drivers/net/can/cc770/cc770_platform.c
@@ -154,7 +154,7 @@ static int __devinit cc770_get_platform_data(struct platform_device *pdev,
 	struct cc770_platform_data *pdata = pdev->dev.platform_data;
 
 	priv->can.clock.freq = pdata->osc_freq;
-	if (priv->cpu_interface | CPUIF_DSC)
+	if (priv->cpu_interface & CPUIF_DSC)
 		priv->can.clock.freq /= 2;
 	priv->clkout = pdata->cor;
 	priv->bus_config = pdata->bcr;

[PATCH 7/7] drivers/net/can/cc770/cc770_platform.c: adjust suspicious bit operation

[Julia Lawall]

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

CPUIF_DSC is 0x40, so it would seem that & is wanted rather than |.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
index 53115ee..688371c 100644
--- a/drivers/net/can/cc770/cc770_platform.c
+++ b/drivers/net/can/cc770/cc770_platform.c
@@ -154,7 +154,7 @@ static int __devinit cc770_get_platform_data(struct platform_device *pdev,
 	struct cc770_platform_data *pdata = pdev->dev.platform_data;
 
 	priv->can.clock.freq = pdata->osc_freq;
-	if (priv->cpu_interface | CPUIF_DSC)
+	if (priv->cpu_interface & CPUIF_DSC)
 		priv->can.clock.freq /= 2;
 	priv->clkout = pdata->cor;
 	priv->bus_config = pdata->bcr;

Re: [PATCH 7/7] drivers/net/can/cc770/cc770_platform.c: adjust suspicious bit operation

[Marc Kleine-Budde]

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

On 06/06/2012 11:41 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> CPUIF_DSC is 0x40, so it would seem that & is wanted rather than |.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Good catch, Joe Perches has already found and fixed that problem. It has
been fixed in:

    dc605db can: cc770: Fix likely misuse of | for &

The commit is currently in David's net/master, and scheduled for the
v3.5 release.

Regards, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 7/7] drivers/net/can/cc770/cc770_platform.c: adjust suspicious bit operation

[Marc Kleine-Budde]

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

On 06/06/2012 11:41 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> CPUIF_DSC is 0x40, so it would seem that & is wanted rather than |.
> 
> This problem was found using Coccinelle (http://coccinelle.lip6.fr/).
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Good catch, Joe Perches has already found and fixed that problem. It has
been fixed in:

    dc605db can: cc770: Fix likely misuse of | for &

The commit is currently in David's net/master, and scheduled for the
v3.5 release.

Regards, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]