All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] delete unnecessary null test on array
@ 2014-08-06 10:39 ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-06 10:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-janitors, linux-scsi, James E.J. Bottomley,
	Adaptec OEM Raid Solutions

Delete NULL test on array.  The complete semantic patch that finds this
problem is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r@
type T;
T [] e;
position p;
@@

(
 e ==@p NULL
|
 e !=@p NULL
|
 !@p e
)

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@

(
* (e.f) ==@p NULL
|
* (e.f) !=@p NULL
|
* !@p(e.f)
)
// </smpl>

For best results, this semantic patch requires lots of type information,
and should be used with the options --recursive-includes and
--relax-include-path.  This may take a long time to run.


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

* [PATCH 0/1] delete unnecessary null test on array
@ 2014-08-06 10:39 ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-06 10:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-janitors, linux-scsi, James E.J. Bottomley,
	Adaptec OEM Raid Solutions

Delete NULL test on array.  The complete semantic patch that finds this
problem is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r@
type T;
T [] e;
position p;
@@

(
 e =@p NULL
|
 e !=@p NULL
|
 !@p e
)

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@

(
* (e.f) =@p NULL
|
* (e.f) !=@p NULL
|
* !@p(e.f)
)
// </smpl>

For best results, this semantic patch requires lots of type information,
and should be used with the options --recursive-includes and
--relax-include-path.  This may take a long time to run.


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

* [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-06 10:39 ` Julia Lawall
@ 2014-08-06 10:39   ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-06 10:39 UTC (permalink / raw)
  To: Adaptec OEM Raid Solutions
  Cc: kernel-janitors, James E.J. Bottomley, linux-scsi, linux-kernel, joe

From: Julia Lawall <Julia.Lawall@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.Lawall@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_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) {
 		return NULL;


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

* [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-06 10:39   ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-06 10:39 UTC (permalink / raw)
  To: Adaptec OEM Raid Solutions
  Cc: kernel-janitors, James E.J. Bottomley, linux-scsi, linux-kernel, joe

From: Julia Lawall <Julia.Lawall@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.Lawall@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_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) {
 		return NULL;


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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-06 10:39   ` Julia Lawall
@ 2014-08-08 14:38     ` walter harms
  -1 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2014-08-08 14:38 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors,
	James E.J. Bottomley, linux-scsi, linux-kernel, joe



Am 06.08.2014 12:39, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@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.Lawall@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_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) {
>  		return NULL;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-08 14:38     ` walter harms
  0 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2014-08-08 14:38 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors,
	James E.J. Bottomley, linux-scsi, linux-kernel, joe



Am 06.08.2014 12:39, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@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.Lawall@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_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) {
>  		return NULL;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-06 10:39   ` Julia Lawall
@ 2014-08-08 16:59     ` James Bottomley
  -1 siblings, 0 replies; 16+ messages in thread
From: James Bottomley @ 2014-08-08 16:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

On Wed, 2014-08-06 at 12:39 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@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.Lawall@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_DEBUG"Adaptec 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


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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-08 16:59     ` James Bottomley
  0 siblings, 0 replies; 16+ messages in thread
From: James Bottomley @ 2014-08-08 16:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

On Wed, 2014-08-06 at 12:39 +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@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.Lawall@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_DEBUG"Adaptec 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


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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-08 16:59     ` James Bottomley
@ 2014-08-08 17:03       ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-08 17:03 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

> > 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_DEBUG"Adaptec 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

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-08 17:03       ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-08 17:03 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

> > 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_DEBUG"Adaptec 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

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-08 17:03       ` Julia Lawall
@ 2014-08-08 17:14         ` James Bottomley
  -1 siblings, 0 replies; 16+ messages in thread
From: James Bottomley @ 2014-08-08 17:14 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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_DEBUG"Adaptec 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





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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-08 17:14         ` James Bottomley
  0 siblings, 0 replies; 16+ messages in thread
From: James Bottomley @ 2014-08-08 17:14 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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_DEBUG"Adaptec 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





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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-08 17:14         ` James Bottomley
@ 2014-08-08 17:16           ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-08 17:16 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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_DEBUG"Adaptec 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

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-08 17:16           ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-08 17:16 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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_DEBUG"Adaptec 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

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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
  2014-08-08 17:14         ` James Bottomley
@ 2014-08-09  6:26           ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-09  6:26 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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

From: Julia Lawall <Julia.Lawall@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.Lawall@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_DEBUG"Adaptec 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){


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

* Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array
@ 2014-08-09  6:26           ` Julia Lawall
  0 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2014-08-09  6:26 UTC (permalink / raw)
  To: James Bottomley
  Cc: Adaptec OEM Raid Solutions, kernel-janitors, linux-scsi,
	linux-kernel, joe

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

From: Julia Lawall <Julia.Lawall@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.Lawall@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_DEBUG"Adaptec 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){


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

end of thread, other threads:[~2014-08-09  6:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-06 10:39 [PATCH 0/1] delete unnecessary null test on array Julia Lawall
2014-08-06 10:39 ` Julia Lawall
2014-08-06 10:39 ` [PATCH 1/1] dpt_i2o: " Julia Lawall
2014-08-06 10:39   ` Julia Lawall
2014-08-08 14:38   ` walter harms
2014-08-08 14:38     ` walter harms
2014-08-08 16:59   ` James Bottomley
2014-08-08 16:59     ` James Bottomley
2014-08-08 17:03     ` Julia Lawall
2014-08-08 17:03       ` Julia Lawall
2014-08-08 17:14       ` James Bottomley
2014-08-08 17:14         ` James Bottomley
2014-08-08 17:16         ` Julia Lawall
2014-08-08 17:16           ` Julia Lawall
2014-08-09  6:26         ` Julia Lawall
2014-08-09  6:26           ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.