Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-09 Thread Julia Lawall 
From nobody Sat Aug  9 08:17:15 CEST 2014
From: Julia Lawall julia.law...@lip6.fr
To: Adaptec OEM Raid Solutions aacr...@adaptec.com
Cc: James E.J. Bottomley 
jbottom...@parallels.com,linux-scsi@vger.kernel.org,linux-ker...@vger.kernel.org
Subject: [PATCH] dpt_i2o: delete unnecessary null test on array

From: Julia Lawall julia.law...@lip6.fr

Convert a NULL test on an array to a NULL test on its element.  Delete a
redundant test and clean up the code a little.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
T [] e;
position p;
@@
e ==@p NULL

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@
* e.f ==@p NULL
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
v2: Test instead the array element, and clean up the code a bit.

 drivers/scsi/dpt_i2o.c |9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..4647c93 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,15 +1169,14 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;

-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
before they are allocated\n);
+   d = pHba-channel[chan].device[id];
+   if (!d) {
+   printk(KERN_DEBUG Adaptec I2O RAID: Trying to find device 
before they are allocated\n);
return NULL;
}
 
-   d = pHba-channel[chan].device[id];
-   if(!d || d-tid == 0) {
+   if (d-tid == 0)
return NULL;
-   }
 
/* If it is the only lun at that address then this should match*/
if(d-scsi_lun == lun){

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

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-08 Thread walter harms 


Am 06.08.2014 12:39, schrieb Julia Lawall:
 From: Julia Lawall julia.law...@lip6.fr
 
 Delete NULL test on array (always false).
 
 A simplified version of the semantic match that finds this problem is as
 follows: (http://coccinelle.lip6.fr/)
 
 // smpl
 @r@
 type T;
 T [] e;
 position p;
 @@
 e ==@p NULL
 
 @ disable fld_to_ptr@
 expression e;
 identifier f;
 position r.p;
 @@
 * e.f ==@p NULL
 // /smpl
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
 I don't know if this is the correct change, or if some other test was
 intended.  But the code has been this way since at least 2.4.20, so it
 would seem that no one has been bothered by the lack of whatever this was
 supposed to test for.
 
  drivers/scsi/dpt_i2o.c |5 -
  1 file changed, 5 deletions(-)
 
 diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
 index 67283ef..62e276b 100644
 --- a/drivers/scsi/dpt_i2o.c
 +++ b/drivers/scsi/dpt_i2o.c
 @@ -1169,11 +1169,6 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
 pHba, u32 chan, u32 id, u6
   if(chan  0 || chan = MAX_CHANNEL)
   return NULL;
   

chan is u32 and u32  0 ?
for the next round.

re,
 wh

 - if( pHba-channel[chan].device == NULL){
 - printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
 before they are allocated\n);
 - return NULL;
 - }
 -
   d = pHba-channel[chan].device[id];
   if(!d || d-tid == 0) {
   return NULL;
 
 --
 To unsubscribe from this list: send the line unsubscribe kernel-janitors in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-08 Thread James Bottomley 
On Wed, 2014-08-06 at 12:39 +0200, Julia Lawall wrote:
 From: Julia Lawall julia.law...@lip6.fr
 
 Delete NULL test on array (always false).
 
 A simplified version of the semantic match that finds this problem is as
 follows: (http://coccinelle.lip6.fr/)
 
 // smpl
 @r@
 type T;
 T [] e;
 position p;
 @@
 e ==@p NULL
 
 @ disable fld_to_ptr@
 expression e;
 identifier f;
 position r.p;
 @@
 * e.f ==@p NULL
 // /smpl
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
 I don't know if this is the correct change, or if some other test was
 intended.  But the code has been this way since at least 2.4.20, so it
 would seem that no one has been bothered by the lack of whatever this was
 supposed to test for.
 
  drivers/scsi/dpt_i2o.c |5 -
  1 file changed, 5 deletions(-)
 
 diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
 index 67283ef..62e276b 100644
 --- a/drivers/scsi/dpt_i2o.c
 +++ b/drivers/scsi/dpt_i2o.c
 @@ -1169,11 +1169,6 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
 pHba, u32 chan, u32 id, u6
   if(chan  0 || chan = MAX_CHANNEL)
   return NULL;
   
 - if( pHba-channel[chan].device == NULL){
 - printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
 before they are allocated\n);
 - return NULL;
 - }

dpt_i2o is always weirdly fun, but I think, based on the message, this
check is supposed to be 

if( pHba-channel[chan].device[id] == NULL){

Since device is an array of device pointers which are allocated by
parsing data.

James

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

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-08 Thread Julia Lawall 
  diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
  index 67283ef..62e276b 100644
  --- a/drivers/scsi/dpt_i2o.c
  +++ b/drivers/scsi/dpt_i2o.c
  @@ -1169,11 +1169,6 @@ static struct adpt_device* 
  adpt_find_device(adpt_hba* pHba, u32 chan, u32 id, u6
  if(chan  0 || chan = MAX_CHANNEL)
  return NULL;
 
  -   if( pHba-channel[chan].device == NULL){
  -   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
  before they are allocated\n);
  -   return NULL;
  -   }

 dpt_i2o is always weirdly fun, but I think, based on the message, this
 check is supposed to be

 if( pHba-channel[chan].device[id] == NULL){

 Since device is an array of device pointers which are allocated by
 parsing data.

That seems to be already checked immediately below:

d = pHba-channel[chan].device[id];
if(!d || d-tid == 0) {
return NULL;
}

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-08 Thread James Bottomley 
On Fri, 2014-08-08 at 19:03 +0200, Julia Lawall wrote:
   diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
   index 67283ef..62e276b 100644
   --- a/drivers/scsi/dpt_i2o.c
   +++ b/drivers/scsi/dpt_i2o.c
   @@ -1169,11 +1169,6 @@ static struct adpt_device* 
   adpt_find_device(adpt_hba* pHba, u32 chan, u32 id, u6
 if(chan  0 || chan = MAX_CHANNEL)
 return NULL;
  
   - if( pHba-channel[chan].device == NULL){
   - printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
   before they are allocated\n);
   - return NULL;
   - }
 
  dpt_i2o is always weirdly fun, but I think, based on the message, this
  check is supposed to be
 
  if( pHba-channel[chan].device[id] == NULL){
 
  Since device is an array of device pointers which are allocated by
  parsing data.
 
 That seems to be already checked immediately below:
 
 d = pHba-channel[chan].device[id];
   if(!d || d-tid == 0) {
 return NULL;

Yes, I know, but no message is emitted.  The message seems to be for a
violation of the state machine which device[id] = NULL implies.

James




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

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-08 Thread Julia Lawall 
On Fri, 8 Aug 2014, James Bottomley wrote:

 On Fri, 2014-08-08 at 19:03 +0200, Julia Lawall wrote:
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..62e276b 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,11 +1169,6 @@ static struct adpt_device* 
adpt_find_device(adpt_hba* pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;
   
-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find 
device before they are allocated\n);
-   return NULL;
-   }
  
   dpt_i2o is always weirdly fun, but I think, based on the message, this
   check is supposed to be
  
   if( pHba-channel[chan].device[id] == NULL){
  
   Since device is an array of device pointers which are allocated by
   parsing data.
 
  That seems to be already checked immediately below:
 
  d = pHba-channel[chan].device[id];
  if(!d || d-tid == 0) {
  return NULL;

 Yes, I know, but no message is emitted.  The message seems to be for a
 violation of the state machine which device[id] = NULL implies.

OK, if the message seems helpful, then I can split up the tests.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] dpt_i2o: delete unnecessary null test on array

2014-08-06 Thread Julia Lawall 
From: Julia Lawall julia.law...@lip6.fr

Delete NULL test on array (always false).

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
T [] e;
position p;
@@
e ==@p NULL

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@
* e.f ==@p NULL
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
I don't know if this is the correct change, or if some other test was
intended.  But the code has been this way since at least 2.4.20, so it
would seem that no one has been bothered by the lack of whatever this was
supposed to test for.

 drivers/scsi/dpt_i2o.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..62e276b 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,11 +1169,6 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;

-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
before they are allocated\n);
-   return NULL;
-   }
-
d = pHba-channel[chan].device[id];
if(!d || d-tid == 0) {
return NULL;

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