These patches fix a case where the same expression is tested twice and the answer is guaranteed to be the same in each case. --
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) + --
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 */ --
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> --
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 */ --
Sorry, was a bit late. Thanks for the patch, review, and picking up :-) Vegard --
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) --
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); } --
I added the Atheros people to the CC list. Jie Yang should know. regards, dan carpenter --
The patch is pretty obvious. Looks good. Luis --
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); ...
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"); --
My guess is that "use_dma_ppb" was intended. regards, dan carpenter --
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"); --
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; } --
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --
