[PATCH 0/7] Remove double test

[PATCH 0/7] Remove double test

[Julia Lawall]

These patches fix a case where the same expression is tested twice and the
answer is guaranteed to be the same in each case.

[PATCH 1/7] drivers/staging: Remove double test

[Julia Lawall]

The 1 element of the array is tested twice.  Change the code so that the
remaining 3 element of the array is tested instead of testing the 1 element
a second time.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
This is just as guess as to what the correct code should be.

 drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c  |    2 +-
 drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c |    2 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c  |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c
index aaf9b9d..9318695 100644
--- a/drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c
@@ -2579,7 +2579,7 @@ static inline void update_network(struct ieee80211_network *dst,
 	if(src->wmm_param[0].ac_aci_acm_aifsn|| \
 	   src->wmm_param[1].ac_aci_acm_aifsn|| \
 	   src->wmm_param[2].ac_aci_acm_aifsn|| \
-	   src->wmm_param[1].ac_aci_acm_aifsn) {
+	   src->wmm_param[3].ac_aci_acm_aifsn) {
 	  memcpy(dst->wmm_param, src->wmm_param, WME_AC_PRAM_LEN);
 	}
 	//dst->QoS_Enable = src->QoS_Enable;
diff --git a/drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c
index 09a02f7..7230cf5 100644
--- a/drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c
@@ -2298,7 +2298,7 @@ static inline void update_network(struct ieee80211_network *dst,
 	if(src->wmm_param[0].ac_aci_acm_aifsn|| \
 	   src->wmm_param[1].ac_aci_acm_aifsn|| \
 	   src->wmm_param[2].ac_aci_acm_aifsn|| \
-	   src->wmm_param[1].ac_aci_acm_aifsn) {
+	   src->wmm_param[3].ac_aci_acm_aifsn) {
 	  memcpy(dst->wmm_param, src->wmm_param, WME_AC_PRAM_LEN);
 	}
 	//dst->QoS_Enable = src->QoS_Enable;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 192123f..c8ca9d8 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -2391,7 +2391,7 @@ static inline void update_network(struct ieee80211_network *dst,
 	if(src->wmm_param[0].ac_aci_acm_aifsn|| \
 	   src->wmm_param[1].ac_aci_acm_aifsn|| \
 	   src->wmm_param[2].ac_aci_acm_aifsn|| \
-	   src->wmm_param[1].ac_aci_acm_aifsn) {
+	   src->wmm_param[3].ac_aci_acm_aifsn) {
 	  memcpy(dst->wmm_param, src->wmm_param, WME_AC_PRAM_LEN);
 	}
 	//dst->QoS_Enable = src->QoS_Enable;

[PATCH 2/7] drivers/usb/gadget: Remove double test

[Julia Lawall]

The same expression is tested twice and the result is the same each time.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
This is a guess as to what the code should be.  Perhaps something else
should have been tested instead.

 drivers/usb/gadget/amd5536udc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 731150d..6c40c99 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -203,7 +203,7 @@ static void print_regs(struct udc *dev)
 		DBG(dev, "DMA mode       = PPBNDU (packet per buffer "
 			"WITHOUT desc. update)\n");
 		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBNDU");
-	} else if (use_dma && use_dma_ppb_du && use_dma_ppb_du) {
+	} else if (use_dma && use_dma_ppb_du) {
 		DBG(dev, "DMA mode       = PPBDU (packet per buffer "
 			"WITH desc. update)\n");
 		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBDU");

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[David Brownell]

Please update $SUBJECT to say which driver is
affected; it's not everything in that directory,
and accurate GIT summaries help a lot.

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[Julia Lawall]

On Sat, 28 Aug 2010, David Brownell wrote:

> Please update $SUBJECT to say which driver is
> affected;

Done.

> it's not everything in that directory,
> and accurate GIT summaries help a lot.

Is there some sort of rule that can be followed?  For example, if the 
patch affects only one, file, should that file be named?  What if the 
patch affects only two files?

thanks,
julia

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[Stefan Richter]

Julia Lawall wrote:
> On Sat, 28 Aug 2010, David Brownell wrote:
> 
>> Please update $SUBJECT to say which driver is
>> affected;
> 
> Done.
> 
>> it's not everything in that directory,
>> and accurate GIT summaries help a lot.
> 
> Is there some sort of rule that can be followed?  For example, if the 
> patch affects only one, file, should that file be named?  What if the 
> patch affects only two files?

gitk $path or git shortlog $path, with $path being the file or directory, give
a quick impression how the resident developers call their driver or subsystem
updates.  E.g. ^USB: instead of ^drivers/usb/.

However, the committer could still adjust the title, so no drama if the patch
subject as written by a cross-tree developer looks different from those of the
usual subsystem patches.  IMO this is one of the tasks of a subsystem
maintainer.  And it is quickly enough done with a git commit --amend.

There is only a drawbackif the subject is too broad /and/ if the Cc list is
too short;  a driver developer may miss to see the submission.  The danger of
that is low though when get_maintainer.pl is used to good effect.
-- 
Stefan Richter
-=====-==-=- =--- ===--
http://arcgraph.de/sr/

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[Julia Lawall]

On Sat, 28 Aug 2010, Stefan Richter wrote:

> Julia Lawall wrote:
> > On Sat, 28 Aug 2010, David Brownell wrote:
> > 
> >> Please update $SUBJECT to say which driver is
> >> affected;
> > 
> > Done.
> > 
> >> it's not everything in that directory,
> >> and accurate GIT summaries help a lot.
> > 
> > Is there some sort of rule that can be followed?  For example, if the 
> > patch affects only one, file, should that file be named?  What if the 
> > patch affects only two files?
> 
> gitk $path or git shortlog $path, with $path being the file or directory, give
> a quick impression how the resident developers call their driver or subsystem
> updates.  E.g. ^USB: instead of ^drivers/usb/.
> 
> However, the committer could still adjust the title, so no drama if the patch
> subject as written by a cross-tree developer looks different from those of the
> usual subsystem patches.  IMO this is one of the tasks of a subsystem
> maintainer.  And it is quickly enough done with a git commit --amend.
> 
> There is only a drawbackif the subject is too broad /and/ if the Cc list is
> too short;  a driver developer may miss to see the submission.  The danger of
> that is low though when get_maintainer.pl is used to good effect.

OK, thanks.

julia

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[David Brownell]

--- On Sat, 8/28/10, Julia Lawall <julia@diku.dk> wrote:

> From: Julia Lawall <julia@diku.dk>
> Subject: Re: [PATCH 2/7] drivers/usb/gadget: Remove double test
> To: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
> Cc: "David Brownell" <david-b@pacbell.net>, "Thomas Dahlmann" <dahlmann.thomas@arcor.de>, kernel-janitors@vger.kernel.org, "David Brownell" <dbrownell@users.sourceforge.net>, "Greg Kroah-Hartman" <gregkh@suse.de>, linux-geode@lists.infradead.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Saturday, August 28, 2010, 10:17 AM
> On Sat, 28 Aug 2010, Stefan Richter
> wrote:
> 
> > Julia Lawall wrote:
> > > On Sat, 28 Aug 2010, David Brownell wrote:
> > > 
> > >> Please update $SUBJECT to say which driver
> is
> > >> affected;
> > > 
> > > Done.

Thanks.  Unless the author/maintainer of this
driver objects, the patch is OK by me.


> > > 
> > >> it's not everything in that directory,
> > >> and accurate GIT summaries help a lot.
> > > 
> > > Is there some sort of rule that can be
> followed? 

Don't be misleading ...

Generally, if it updates just one driver,
mention that driver in $SUBJECT.

Only say the subsystem itself is being
updated (which your original $SUBJECT did)
when the patch affects framework code, 
that's shared by all drivers, 

 > > >  What if the 
> > > patch affects only two files?

Which two files?
The "patch updates one thing" rule will then
cause it to affect only one driver (usually)
or some framework issue, so the above policies
kick in directly.

Rarely there will be bugs repeated (e.g. via
cut/paste) in multiple drivers, ... in which
 case  it can make sense to summarize a patch
as a multi-driver subsystem fix .  Similarly
with API changes.

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[David Brownell]

--- On Sat, 8/28/10, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> There is only a drawback if the subject
> is too broad /and/ > if the Cc list is
> too short; 

Not entirely true.  Recall that one of the roles
of the patch summary is serving as a search key
for Google or whatever ... changing it decouples patches from their discussion,  complicating
later troubleshooting (probably not in this
case, but for some patches, very much so).

- Dave

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[Dan Carpenter]

On Sat, Aug 28, 2010 at 05:41:01PM +0200, Julia Lawall wrote:
> This is a guess as to what the code should be.  Perhaps something else
> should have been tested instead.
> 
>  drivers/usb/gadget/amd5536udc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
> index 731150d..6c40c99 100644
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@ -203,7 +203,7 @@ static void print_regs(struct udc *dev)
>  		DBG(dev, "DMA mode       = PPBNDU (packet per buffer "
>  			"WITHOUT desc. update)\n");
>  		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBNDU");
> -	} else if (use_dma && use_dma_ppb_du && use_dma_ppb_du) {
> +	} else if (use_dma && use_dma_ppb_du) {


My guess is that "use_dma_ppb" was intended.

regards,
dan carpenter

Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test

[Julia Lawall]

The same expression is tested twice and the result is the same each time.
Instead test for use_dma_ppb as in the test above.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
 drivers/usb/gadget/amd5536udc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 731150d..6c40c99 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -203,7 +203,7 @@ static void print_regs(struct udc *dev)
 		DBG(dev, "DMA mode       = PPBNDU (packet per buffer "
 			"WITHOUT desc. update)\n");
 		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBNDU");
-	} else if (use_dma && use_dma_ppb_du && use_dma_ppb_du) {
+	} else if (use_dma && use_dma_ppb && use_dma_ppb_du) {
 		DBG(dev, "DMA mode       = PPBDU (packet per buffer "
 			"WITH desc. update)\n");
 		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBDU");

Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test

[David Brownell]

--- On Sat, 8/28/10, Julia Lawall <julia@diku.dk> wrote:

> From: Julia Lawall <julia@diku.dk>
> Subject: Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test
> To: "Dan Carpenter" <error27@gmail.com>
> Cc: "Thomas Dahlmann" <dahlmann.thomas@arcor.de>, kernel-janitors@vger.kernel.org, "David Brownell" <dbrownell@users.sourceforge.net>, "Greg Kroah-Hartman" <gregkh@suse.de>, linux-geode@lists.infradead.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Saturday, August 28, 2010, 9:48 AM
> The same expression is tested twice
> and the result is the same each time.
> Instead test for use_dma_ppb as in the test above.
> 
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @expression@
> expression E;
> @@
> 
> (
> * E
>   || ... || E
> |
> * E
>   && ... && E
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>


OK by me unless the maintainer/author objects.

Thank you!

- Dave



> 
> ---
>  drivers/usb/gadget/amd5536udc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/amd5536udc.c
> b/drivers/usb/gadget/amd5536udc.c
> index 731150d..6c40c99 100644
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@ -203,7 +203,7 @@ static void print_regs(struct udc
> *dev)
>          DBG(dev, "DMA
> mode       = PPBNDU (packet per
> buffer "
>             
> "WITHOUT desc. update)\n");
>         
> dev_info(&dev->pdev->dev, "DMA mode (%s)\n",
> "PPBNDU");
> -    } else if (use_dma &&
> use_dma_ppb_du && use_dma_ppb_du) {
> +    } else if (use_dma &&
> use_dma_ppb && use_dma_ppb_du) {
>          DBG(dev, "DMA
> mode       = PPBDU (packet per
> buffer "
>             
> "WITH desc. update)\n");
>         
> dev_info(&dev->pdev->dev, "DMA mode (%s)\n",
> "PPBDU");
> 
> 

Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test

[Thomas Dahlmann]

David Brownell schrieb:
> --- On Sat, 8/28/10, Julia Lawall <julia@diku.dk> wrote:
>
>   
>> From: Julia Lawall <julia@diku.dk>
>> Subject: Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test
>> To: "Dan Carpenter" <error27@gmail.com>
>> Cc: "Thomas Dahlmann" <dahlmann.thomas@arcor.de>, kernel-janitors@vger.kernel.org, "David Brownell" <dbrownell@users.sourceforge.net>, "Greg Kroah-Hartman" <gregkh@suse.de>, linux-geode@lists.infradead.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
>> Date: Saturday, August 28, 2010, 9:48 AM
>> The same expression is tested twice
>> and the result is the same each time.
>> Instead test for use_dma_ppb as in the test above.
>>
>> The sematic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @expression@
>> expression E;
>> @@
>>
>> (
>> * E
>>   || ... || E
>> |
>> * E
>>   && ... && E
>> )
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia@diku.dk>
>>     
>
>
> OK by me unless the maintainer/author objects.
>
> Thank you!
>
> - Dave
>
>   

This fix is OK. Thank you for figuring this out!

- Thomas

>
>   
>> ---
>>  drivers/usb/gadget/amd5536udc.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/amd5536udc.c
>> b/drivers/usb/gadget/amd5536udc.c
>> index 731150d..6c40c99 100644
>> --- a/drivers/usb/gadget/amd5536udc.c
>> +++ b/drivers/usb/gadget/amd5536udc.c
>> @@ -203,7 +203,7 @@ static void print_regs(struct udc
>> *dev)
>>          DBG(dev, "DMA
>> mode       = PPBNDU (packet per
>> buffer "
>>             
>> "WITHOUT desc. update)\n");
>>         
>> dev_info(&dev->pdev->dev, "DMA mode (%s)\n",
>> "PPBNDU");
>> -    } else if (use_dma &&
>> use_dma_ppb_du && use_dma_ppb_du) {
>> +    } else if (use_dma &&
>> use_dma_ppb && use_dma_ppb_du) {
>>          DBG(dev, "DMA
>> mode       = PPBDU (packet per
>> buffer "
>>             
>> "WITH desc. update)\n");
>>         
>> dev_info(&dev->pdev->dev, "DMA mode (%s)\n",
>> "PPBDU");
>>
>>
>>     
>
>   

Re: [PATCH 2/7] drivers/usb/gadget: Remove double test

[Julia Lawall]

On Sat, 28 Aug 2010, Dan Carpenter wrote:

> On Sat, Aug 28, 2010 at 05:41:01PM +0200, Julia Lawall wrote:
> > This is a guess as to what the code should be.  Perhaps something else
> > should have been tested instead.
> > 
> >  drivers/usb/gadget/amd5536udc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
> > index 731150d..6c40c99 100644
> > --- a/drivers/usb/gadget/amd5536udc.c
> > +++ b/drivers/usb/gadget/amd5536udc.c
> > @@ -203,7 +203,7 @@ static void print_regs(struct udc *dev)
> >  		DBG(dev, "DMA mode       = PPBNDU (packet per buffer "
> >  			"WITHOUT desc. update)\n");
> >  		dev_info(&dev->pdev->dev, "DMA mode (%s)\n", "PPBNDU");
> > -	} else if (use_dma && use_dma_ppb_du && use_dma_ppb_du) {
> > +	} else if (use_dma && use_dma_ppb_du) {
> 
> 
> My guess is that "use_dma_ppb" was intended.

Thanks.  I made this change.

julia

[PATCH 3/7] drivers/net/atl1c: Remove double test

[Julia Lawall]

The nic_type field is compared to athr_l2c twice.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
This is a guess as to what the code should be.  Perhaps something else
should have been tested instead.

 drivers/net/atl1c/atl1c_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/atl1c/atl1c_hw.c b/drivers/net/atl1c/atl1c_hw.c
index d8501f0..919080b 100644
--- a/drivers/net/atl1c/atl1c_hw.c
+++ b/drivers/net/atl1c/atl1c_hw.c
@@ -480,7 +480,7 @@ int atl1c_phy_reset(struct atl1c_hw *hw)
 		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0x929D);
 	}
 	if (hw->nic_type == athr_l1c || hw->nic_type == athr_l2c_b2
-		|| hw->nic_type == athr_l2c || hw->nic_type == athr_l2c) {
+		|| hw->nic_type == athr_l2c) {
 		atl1c_write_phy_reg(hw, MII_DBG_ADDR, 0x29);
 		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0xB6DD);
 	}

Re: [PATCH 3/7] drivers/net/atl1c: Remove double test

[Dan Carpenter]

I added the Atheros people to the CC list.  Jie Yang should know.

regards,
dan carpenter

On Sat, Aug 28, 2010 at 05:41:02PM +0200, Julia Lawall wrote:
> The nic_type field is compared to athr_l2c twice.
> 
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @expression@
> expression E;
> @@
> 
> (
> * E
>   || ... || E
> |
> * E
>   && ... && E
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> This is a guess as to what the code should be.  Perhaps something else
> should have been tested instead.
> 
>  drivers/net/atl1c/atl1c_hw.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/atl1c/atl1c_hw.c b/drivers/net/atl1c/atl1c_hw.c
> index d8501f0..919080b 100644
> --- a/drivers/net/atl1c/atl1c_hw.c
> +++ b/drivers/net/atl1c/atl1c_hw.c
> @@ -480,7 +480,7 @@ int atl1c_phy_reset(struct atl1c_hw *hw)
>  		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0x929D);
>  	}
>  	if (hw->nic_type == athr_l1c || hw->nic_type == athr_l2c_b2
> -		|| hw->nic_type == athr_l2c || hw->nic_type == athr_l2c) {
> +		|| hw->nic_type == athr_l2c) {
>  		atl1c_write_phy_reg(hw, MII_DBG_ADDR, 0x29);
>  		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0xB6DD);
>  	}
> 
> --
> 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

Re: [PATCH 3/7] drivers/net/atl1c: Remove double test

[Luis R. Rodriguez]

On Sat, Aug 28, 2010 at 09:50:14AM -0700, Dan Carpenter wrote:
> I added the Atheros people to the CC list.  Jie Yang should know.
> 
> regards,
> dan carpenter
> 
> On Sat, Aug 28, 2010 at 05:41:02PM +0200, Julia Lawall wrote:
> > The nic_type field is compared to athr_l2c twice.
> > 
> > The sematic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @expression@
> > expression E;
> > @@
> > 
> > (
> > * E
> >   || ... || E
> > |
> > * E
> >   && ... && E
> > )
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> > This is a guess as to what the code should be.  Perhaps something else
> > should have been tested instead.
> > 
> >  drivers/net/atl1c/atl1c_hw.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/atl1c/atl1c_hw.c b/drivers/net/atl1c/atl1c_hw.c
> > index d8501f0..919080b 100644
> > --- a/drivers/net/atl1c/atl1c_hw.c
> > +++ b/drivers/net/atl1c/atl1c_hw.c
> > @@ -480,7 +480,7 @@ int atl1c_phy_reset(struct atl1c_hw *hw)
> >  		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0x929D);
> >  	}
> >  	if (hw->nic_type == athr_l1c || hw->nic_type == athr_l2c_b2
> > -		|| hw->nic_type == athr_l2c || hw->nic_type == athr_l2c) {
> > +		|| hw->nic_type == athr_l2c) {
> >  		atl1c_write_phy_reg(hw, MII_DBG_ADDR, 0x29);
> >  		atl1c_write_phy_reg(hw, MII_DBG_DATA, 0xB6DD);
> >  	}

The patch is pretty obvious. Looks good.

  Luis

[PATCH 4/7] arch/x86/mm/kmemcheck: Remove double test

[Julia Lawall]

The opcodes 0x2e and 0x3e are tested for in the first Group 2 line as well.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
This is a guess as to what the code should be.  Perhaps something else
should have been tested instead.

 arch/x86/mm/kmemcheck/opcode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/kmemcheck/opcode.c b/arch/x86/mm/kmemcheck/opcode.c
index 63c19e2..324aa3f 100644
--- a/arch/x86/mm/kmemcheck/opcode.c
+++ b/arch/x86/mm/kmemcheck/opcode.c
@@ -9,7 +9,7 @@ static bool opcode_is_prefix(uint8_t b)
 		b == 0xf0 || b == 0xf2 || b == 0xf3
 		/* Group 2 */
 		|| b == 0x2e || b == 0x36 || b == 0x3e || b == 0x26
-		|| b == 0x64 || b == 0x65 || b == 0x2e || b == 0x3e
+		|| b == 0x64 || b == 0x65
 		/* Group 3 */
 		|| b == 0x66
 		/* Group 4 */

Re: [PATCH 4/7] arch/x86/mm/kmemcheck: Remove double test

[Pekka Enberg]

On 28.8.2010 18.41, Julia Lawall wrote:
> The opcodes 0x2e and 0x3e are tested for in the first Group 2 line as well.
>
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //<smpl>
> @expression@
> expression E;
> @@
>
> (
> * E
>    || ... || E
> |
> * E
>    &&  ...&&  E
> )
> //</smpl>
>
> Signed-off-by: Julia Lawall<julia@diku.dk>
>
> ---
> This is a guess as to what the code should be.  Perhaps something else
> should have been tested instead.

The last two items are branch hint prefixes that use same encoding as CS 
and DS segment override prefixes.

Reviewed-by: Pekka Enberg <penberg@kernel.org>

Can you pick this up in the x86.git tree, please?

>   arch/x86/mm/kmemcheck/opcode.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/kmemcheck/opcode.c b/arch/x86/mm/kmemcheck/opcode.c
> index 63c19e2..324aa3f 100644
> --- a/arch/x86/mm/kmemcheck/opcode.c
> +++ b/arch/x86/mm/kmemcheck/opcode.c
> @@ -9,7 +9,7 @@ static bool opcode_is_prefix(uint8_t b)
>   		b == 0xf0 || b == 0xf2 || b == 0xf3
>   		/* Group 2 */
>   		|| b == 0x2e || b == 0x36 || b == 0x3e || b == 0x26
> -		|| b == 0x64 || b == 0x65 || b == 0x2e || b == 0x3e
> +		|| b == 0x64 || b == 0x65
>   		/* Group 3 */
>   		|| b == 0x66
>   		/* Group 4 */
>

[tip:x86/mm] x86, kmemcheck: Remove double test

[tip-bot for Julia Lawall]

Commit-ID:  9fbaf49c7f717740002d49eee1bbd03d89d8766a
Gitweb:     http://git.kernel.org/tip/9fbaf49c7f717740002d49eee1bbd03d89d8766a
Author:     Julia Lawall <julia@diku.dk>
AuthorDate: Sat, 28 Aug 2010 17:41:03 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 30 Aug 2010 09:19:28 +0200

x86, kmemcheck: Remove double test

The opcodes 0x2e and 0x3e are tested for in the first Group 2
line as well.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Vegard Nossum <vegardno@ifi.uio.no>
LKML-Reference: <1283010066-20935-5-git-send-email-julia@diku.dk>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/mm/kmemcheck/opcode.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/kmemcheck/opcode.c b/arch/x86/mm/kmemcheck/opcode.c
index 63c19e2..324aa3f 100644
--- a/arch/x86/mm/kmemcheck/opcode.c
+++ b/arch/x86/mm/kmemcheck/opcode.c
@@ -9,7 +9,7 @@ static bool opcode_is_prefix(uint8_t b)
 		b == 0xf0 || b == 0xf2 || b == 0xf3
 		/* Group 2 */
 		|| b == 0x2e || b == 0x36 || b == 0x3e || b == 0x26
-		|| b == 0x64 || b == 0x65 || b == 0x2e || b == 0x3e
+		|| b == 0x64 || b == 0x65
 		/* Group 3 */
 		|| b == 0x66
 		/* Group 4 */

Re: [tip:x86/mm] x86, kmemcheck: Remove double test

[Vegard Nossum]

On 30 August 2010 10:36, tip-bot for Julia Lawall <julia@diku.dk> wrote:
> Commit-ID:  9fbaf49c7f717740002d49eee1bbd03d89d8766a
> Gitweb:     http://git.kernel.org/tip/9fbaf49c7f717740002d49eee1bbd03d89d8766a
> Author:     Julia Lawall <julia@diku.dk>
> AuthorDate: Sat, 28 Aug 2010 17:41:03 +0200
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 30 Aug 2010 09:19:28 +0200
>
> x86, kmemcheck: Remove double test
>
> The opcodes 0x2e and 0x3e are tested for in the first Group 2
> line as well.
>
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @expression@
> expression E;
> @@
>
> (
> * E
>  || ... || E
> |
> * E
>  && ... && E
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
> Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> Cc: Vegard Nossum <vegardno@ifi.uio.no>
> LKML-Reference: <1283010066-20935-5-git-send-email-julia@diku.dk>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/mm/kmemcheck/opcode.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>

Sorry, was a bit late.

Thanks for the patch, review, and picking up :-)


Vegard

Re: [tip:x86/mm] x86, kmemcheck: Remove double test

[Ingo Molnar]

* Vegard Nossum <vegard.nossum@gmail.com> wrote:

> On 30 August 2010 10:36, tip-bot for Julia Lawall <julia@diku.dk> wrote:
> > Commit-ID:  9fbaf49c7f717740002d49eee1bbd03d89d8766a
> > Gitweb:     http://git.kernel.org/tip/9fbaf49c7f717740002d49eee1bbd03d89d8766a
> > Author:     Julia Lawall <julia@diku.dk>
> > AuthorDate: Sat, 28 Aug 2010 17:41:03 +0200
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Mon, 30 Aug 2010 09:19:28 +0200
> >
> > x86, kmemcheck: Remove double test
> >
> > The opcodes 0x2e and 0x3e are tested for in the first Group 2
> > line as well.
> >
> > The sematic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @expression@
> > expression E;
> > @@
> >
> > (
> > * E
> >  || ... || E
> > |
> > * E
> >  && ... && E
> > )
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > Cc: Vegard Nossum <vegardno@ifi.uio.no>
> > LKML-Reference: <1283010066-20935-5-git-send-email-julia@diku.dk>
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > ---
> >  arch/x86/mm/kmemcheck/opcode.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> 
> Sorry, was a bit late.
> 
> Thanks for the patch, review, and picking up :-)

You are welcome :)

	Ingo

[PATCH 5/7] drivers/regulator: Remove double test

[Julia Lawall]

The current code tests the gpio_vid0 field twice.  Test the gpio_vid1
fields in place of the second gpio_vid0 test.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
This is a guess as to what the code should be.

 drivers/regulator/max8952.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max8952.c b/drivers/regulator/max8952.c
index f2af0b1..7bf63f4 100644
--- a/drivers/regulator/max8952.c
+++ b/drivers/regulator/max8952.c
@@ -139,7 +139,7 @@ static int max8952_set_voltage(struct regulator_dev *rdev,
 	u8 vid = -1, i;
 
 	if (!gpio_is_valid(max8952->pdata->gpio_vid0) ||
-			!gpio_is_valid(max8952->pdata->gpio_vid0)) {
+			!gpio_is_valid(max8952->pdata->gpio_vid1)) {
 		/* DVS not supported */
 		return -EPERM;
 	}

Re: [PATCH 5/7] drivers/regulator: Remove double test

[Mark Brown]

On Sat, Aug 28, 2010 at 05:41:04PM +0200, Julia Lawall wrote:
> The current code tests the gpio_vid0 field twice.  Test the gpio_vid1
> fields in place of the second gpio_vid0 test.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[PATCH 6/7] drivers/media/dvb/siano: Remove double test

[Julia Lawall]

The same expression is tested twice and the result is the same each time.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
 drivers/media/dvb/siano/smscoreapi.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/dvb/siano/smscoreapi.c b/drivers/media/dvb/siano/smscoreapi.c
index d93468c..70e3e0c 100644
--- a/drivers/media/dvb/siano/smscoreapi.c
+++ b/drivers/media/dvb/siano/smscoreapi.c
@@ -1511,8 +1511,7 @@ int smscore_gpio_set_level(struct smscore_device_t *coredev, u8 PinNum,
 		u32 msgData[3]; /* keep it 3 ! */
 	} *pMsg;
 
-	if ((NewLevel > 1) || (PinNum > MAX_GPIO_PIN_NUMBER) ||
-			(PinNum > MAX_GPIO_PIN_NUMBER))
+	if ((NewLevel > 1) || (PinNum > MAX_GPIO_PIN_NUMBER))
 		return -EINVAL;
 
 	totalLen = sizeof(struct SmsMsgHdr_ST) +

[PATCH 7/7] net/wireless: Remove double test

[Julia Lawall]

The same expression is tested twice and the result is the same each time.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@expression@
expression E;
@@

(
* E
  || ... || E
|
* E
  && ... && E
)
// </smpl>

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

---
 net/wireless/wext-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index 0ef17bc..4038593 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -611,7 +611,7 @@ struct iw_statistics *get_wireless_stats(struct net_device *dev)
 #endif
 
 #ifdef CONFIG_CFG80211_WEXT
-	if (dev->ieee80211_ptr && dev->ieee80211_ptr &&
+	if (dev->ieee80211_ptr &&
 	    dev->ieee80211_ptr->wiphy &&
 	    dev->ieee80211_ptr->wiphy->wext &&
 	    dev->ieee80211_ptr->wiphy->wext->get_wireless_stats)