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

login
register
mail settings
Submitter Julia Lawall
Date June 6, 2012, 9:41 p.m.
Message ID <1339018901-28439-2-git-send-email-Julia.Lawall@lip6.fr>
Download mbox | patch
Permalink /patch/163436/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Julia Lawall - June 6, 2012, 9:41 p.m.
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(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - June 7, 2012, 9:46 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julia Lawall - June 8, 2012, 5:12 a.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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