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

Linux ATA/IDE development
 help / color / mirror / Atom feed

* [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
       [not found] <1339018901-28439-1-git-send-email-Julia.Lawall@lip6.fr>
@ 2012-06-06 21:41 ` Julia Lawall
  2012-06-07 21:46   ` David Miller
  2012-06-06 21:41 ` [PATCH 6/7] drivers/ata/pata_pcmcia.c: " Julia Lawall
  1 sibling, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2012-06-06 21:41 UTC (permalink / raw)
  To: David S. Miller
  Cc: kernel-janitors, linux-ide, linux-kernel, joe, 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;
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-06 21:41 ` [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation Julia Lawall
@ 2012-06-07 21:46   ` David Miller
  2012-06-07 21:59     ` Joe Perches
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: David Miller @ 2012-06-07 21:46 UTC (permalink / raw)
  To: Julia.Lawall; +Cc: kernel-janitors, linux-ide, linux-kernel, joe, julia

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-07 21:46   ` David Miller
@ 2012-06-07 21:59     ` Joe Perches
  2012-06-08  5:12     ` Julia Lawall
  2012-06-08  5:14     ` Julia Lawall
  2 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2012-06-07 21:59 UTC (permalink / raw)
  To: David Miller
  Cc: Julia.Lawall, kernel-janitors, linux-ide, linux-kernel, julia

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-07 21:46   ` David Miller
  2012-06-07 21:59     ` Joe Perches
@ 2012-06-08  5:12     ` Julia Lawall
  2012-06-08  5:14     ` Julia Lawall
  2 siblings, 0 replies; 10+ messages in thread
From: Julia Lawall @ 2012-06-08  5:12 UTC (permalink / raw)
  To: David Miller
  Cc: Julia.Lawall, kernel-janitors, linux-ide, linux-kernel, joe,
	julia

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-07 21:46   ` David Miller
  2012-06-07 21:59     ` Joe Perches
  2012-06-08  5:12     ` Julia Lawall
@ 2012-06-08  5:14     ` Julia Lawall
  2012-06-08  7:30       ` walter harms
  2012-06-11 23:43       ` David Miller
  2 siblings, 2 replies; 10+ messages in thread
From: Julia Lawall @ 2012-06-08  5:14 UTC (permalink / raw)
  To: David Miller; +Cc: kernel-janitors, linux-ide, linux-kernel, joe

>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;
  	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-08  5:14     ` Julia Lawall
@ 2012-06-08  7:30       ` walter harms
  2012-06-11 23:43       ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: walter harms @ 2012-06-08  7:30 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linux-ide



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-08  5:14     ` Julia Lawall
  2012-06-08  7:30       ` walter harms
@ 2012-06-11 23:43       ` David Miller
  2012-06-12 14:17         ` Julia Lawall
  1 sibling, 1 reply; 10+ messages in thread
From: David Miller @ 2012-06-11 23:43 UTC (permalink / raw)
  To: julia.lawall; +Cc: kernel-janitors, linux-ide, linux-kernel, joe

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-11 23:43       ` David Miller
@ 2012-06-12 14:17         ` Julia Lawall
  2012-06-12 22:52           ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2012-06-12 14:17 UTC (permalink / raw)
  To: David Miller; +Cc: kernel-janitors, linux-ide, linux-kernel, joe

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;
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation
  2012-06-12 14:17         ` Julia Lawall
@ 2012-06-12 22:52           ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2012-06-12 22:52 UTC (permalink / raw)
  To: Julia.Lawall; +Cc: kernel-janitors, linux-ide, linux-kernel, joe

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 6/7] drivers/ata/pata_pcmcia.c: adjust suspicious bit operation
       [not found] <1339018901-28439-1-git-send-email-Julia.Lawall@lip6.fr>
  2012-06-06 21:41 ` [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation Julia Lawall
@ 2012-06-06 21:41 ` Julia Lawall
  1 sibling, 0 replies; 10+ messages in thread
From: Julia Lawall @ 2012-06-06 21:41 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: kernel-janitors, linux-ide, linux-kernel, joe, 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;
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-06-12 22:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1339018901-28439-1-git-send-email-Julia.Lawall@lip6.fr>
2012-06-06 21:41 ` [PATCH 1/7] drivers/ide/ide-cs.c: adjust suspicious bit operation Julia Lawall
2012-06-07 21:46   ` David Miller
2012-06-07 21:59     ` Joe Perches
2012-06-08  5:12     ` Julia Lawall
2012-06-08  5:14     ` Julia Lawall
2012-06-08  7:30       ` walter harms
2012-06-11 23:43       ` David Miller
2012-06-12 14:17         ` Julia Lawall
2012-06-12 22:52           ` David Miller
2012-06-06 21:41 ` [PATCH 6/7] drivers/ata/pata_pcmcia.c: " Julia Lawall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox