2010-08-28 15:41:19

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 0/7] Remove double test

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


2010-08-28 15:41:47

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 4/7] arch/x86/mm/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 <[email protected]>

---
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 */

2010-08-28 15:41:50

by Julia Lawall

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

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 <[email protected]>

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

2010-08-28 15:41:49

by Julia Lawall

[permalink] [raw]
Subject: [PATCH 7/7] net/wireless: Remove double test

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 <[email protected]>

---
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)

2010-08-28 15:41:44

by Julia Lawall

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

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 <[email protected]>

---
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");

2010-08-28 15:41:45

by Julia Lawall

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

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 <[email protected]>

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

2010-08-28 15:41:46

by Julia Lawall

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

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 <[email protected]>

---
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) +

2010-08-28 15:41:42

by Julia Lawall

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

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 <[email protected]>

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

2010-08-28 16:10:16

by David Brownell

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

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


2010-08-28 16:38:05

by Dan Carpenter

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

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

2010-08-28 16:49:00

by Julia Lawall

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

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 <[email protected]>

---
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");

2010-08-28 16:49:30

by Julia Lawall

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

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

2010-08-28 16:50:53

by Dan Carpenter

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

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 <[email protected]>
>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-08-28 16:54:04

by Julia Lawall

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

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

2010-08-28 17:12:43

by Stefan Richter

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

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/

2010-08-28 17:17:20

by Julia Lawall

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

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

2010-08-28 18:11:11

by David Brownell

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



--- On Sat, 8/28/10, Julia Lawall <[email protected]> wrote:

> From: Julia Lawall <[email protected]>
> Subject: Re: [PATCH 2/7] drivers/usb/gadget: Remove double test
> To: "Stefan Richter" <[email protected]>
> Cc: "David Brownell" <[email protected]>, "Thomas Dahlmann" <[email protected]>, [email protected], "David Brownell" <[email protected]>, "Greg Kroah-Hartman" <[email protected]>, [email protected], [email protected], [email protected]
> 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.



2010-08-28 18:19:41

by David Brownell

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



--- On Sat, 8/28/10, Stefan Richter <[email protected]> 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

2010-08-28 18:24:27

by David Brownell

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



--- On Sat, 8/28/10, Julia Lawall <[email protected]> wrote:

> From: Julia Lawall <[email protected]>
> Subject: Re: [PATCH 2/7] drivers/usb/gadget/amd5536udc.c: Remove double test
> To: "Dan Carpenter" <[email protected]>
> Cc: "Thomas Dahlmann" <[email protected]>, [email protected], "David Brownell" <[email protected]>, "Greg Kroah-Hartman" <[email protected]>, [email protected], [email protected], [email protected]
> 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 <[email protected]>


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");
>
>

2010-08-28 20:10:06

by Luis R. Rodriguez

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

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 <[email protected]>
> >
> > ---
> > 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

2010-08-29 10:26:34

by Pekka Enberg

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

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<[email protected]>
>
> ---
> 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 <[email protected]>

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 */
>

2010-08-29 13:06:27

by Mark Brown

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

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 <[email protected]>

2010-08-30 08:37:46

by Julia Lawall

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

Commit-ID: 9fbaf49c7f717740002d49eee1bbd03d89d8766a
Gitweb: http://git.kernel.org/tip/9fbaf49c7f717740002d49eee1bbd03d89d8766a
Author: Julia Lawall <[email protected]>
AuthorDate: Sat, 28 Aug 2010 17:41:03 +0200
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Reviewed-by: Pekka Enberg <[email protected]>
Cc: Vegard Nossum <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
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 */