treewide: Fix likely misuses of | for &

PATCH In review Joe Perches 5 participants 11 messages May 30, 2012 – Jun 1, 2012

Subsystems (6)

Networking Network drivers USB Networking drivers (wireless) +2 more subsystems

lore

Share & tools

lore

Joe Perches

author

2012-05-30 20:25 UTC #0 lore raw

cover treewide: Fix likely misuses of | for &

| with a non-zero constant is always true.
Likely these should have been &.

Joe Perches (4):
mac80211: Fix likely misuse of | for &
can: cc770: Fix likely misuse of | for &
brcmfmac: Fix likely misuse of | for &
usb: gadget: pch_udc: Fix likely misuse of | for &

 drivers/net/can/cc770/cc770_platform.c           |    2 +-
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    4 ++--
 drivers/usb/gadget/pch_udc.c                     |    2 +-
 net/mac80211/cfg.c                               |    6 +++---
 4 files changed, 7 insertions(+), 7 deletions(-)

Signature · 1 line

1.7.8.111.gad25c.dirty

Joe Perches

author

2012-05-30 20:25 UTC #1 lore raw

PATCH 1/4 mac80211: Fix likely misuse of | for &

Using | with a constant is always true.
Likely this should have be &.

cc Ben Greear <[email protected]>

Signed-off-by Joe Perches <[email protected]>

---
 net/mac80211/cfg.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

net/mac80211/cfg.c +3 -3

index 495831e..e9cecca 100644
@@ -533,16 +533,16 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
 		sinfo.filled = 0;
 		sta_set_sinfo(sta, &sinfo);
 
-		if (sinfo.filled | STATION_INFO_TX_BITRATE)
+		if (sinfo.filled & STATION_INFO_TX_BITRATE)
 			data[i] = 100000 *
 				cfg80211_calculate_bitrate(&sinfo.txrate);
 		i++;
-		if (sinfo.filled | STATION_INFO_RX_BITRATE)
+		if (sinfo.filled & STATION_INFO_RX_BITRATE)
 			data[i] = 100000 *
 				cfg80211_calculate_bitrate(&sinfo.rxrate);
 		i++;
 
-		if (sinfo.filled | STATION_INFO_SIGNAL_AVG)
+		if (sinfo.filled & STATION_INFO_SIGNAL_AVG)
 			data[i] = (u8)sinfo.signal_avg;
 		i++;
 	} else {

Signature · 1 line

1.7.8.111.gad25c.dirty

Ben Greear

2012-05-30 20:29 UTC #2 lore raw

Re: [PATCH 1/4] mac80211: Fix likely misuse of | for &

Quoted 2 lines · Joe Perches

Using | with a constant is always true.
Likely this should have be&.

Yes, I think your change is correct. Looks like something a compiler or static analysis tool could catch and warn about...

Sorry for the bug!

Ben

Quoted 31 lines

cc Ben Greear<[email protected]>

Signed-off-by Joe Perches<[email protected]>

---
  net/mac80211/cfg.c |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

net/mac80211/cfg.c +3 -3

index 495831e..e9cecca 100644
@@ -533,16 +533,16 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
  		sinfo.filled = 0;
  		sta_set_sinfo(sta,&sinfo);
 
-		if (sinfo.filled | STATION_INFO_TX_BITRATE)
+		if (sinfo.filled&  STATION_INFO_TX_BITRATE)
  			data[i] = 100000 *
  				cfg80211_calculate_bitrate(&sinfo.txrate);
  		i++;
-		if (sinfo.filled | STATION_INFO_RX_BITRATE)
+		if (sinfo.filled&  STATION_INFO_RX_BITRATE)
  			data[i] = 100000 *
  				cfg80211_calculate_bitrate(&sinfo.rxrate);
  		i++;
 
-		if (sinfo.filled | STATION_INFO_SIGNAL_AVG)
+		if (sinfo.filled&  STATION_INFO_SIGNAL_AVG)
  			data[i] = (u8)sinfo.signal_avg;
  		i++;
  	} else {

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Joe Perches

author

2012-05-30 20:25 UTC #3 lore raw

PATCH 2/4 can: cc770: Fix likely misuse of | for &

In reply to Joe Perches

Using | with a constant is always true.
Likely this should have be &.

Signed-off-by Joe Perches <[email protected]>

---
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/net/can/cc770/cc770_platform.c +1 -1

index 53115ee..688371c 100644
@@ -154,7 +154,7 @@ static int __devinit cc770_get_platform_data(struct platform_device *pdev,
 	struct cc770_platform_data *pdata = pdev->dev.platform_data;
 
 	priv->can.clock.freq = pdata->osc_freq;
-	if (priv->cpu_interface | CPUIF_DSC)
+	if (priv->cpu_interface & CPUIF_DSC)
 		priv->can.clock.freq /= 2;
 	priv->clkout = pdata->cor;
 	priv->bus_config = pdata->bcr;

Signature · 1 line

1.7.8.111.gad25c.dirty

Marc Kleine-Budde

maintainer

2012-05-31 20:54 UTC #4 lore raw

Re: [PATCH 2/4] can: cc770: Fix likely misuse of | for &

Quoted 4 lines · Joe Perches

Using | with a constant is always true.
Likely this should have be &.
Signed-off-by Joe Perches <[email protected]>

Sounds reasonable. And there are no in tree users of the platform driver that this fix could break.

commited to linux-can,
Marc

Quoted 17 lines

---
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/net/can/cc770/cc770_platform.c +1 -1

index 53115ee..688371c 100644
@@ -154,7 +154,7 @@ static int __devinit cc770_get_platform_data(struct platform_device *pdev,
 	struct cc770_platform_data *pdata = pdev->dev.platform_data;
 
 	priv->can.clock.freq = pdata->osc_freq;
-	if (priv->cpu_interface | CPUIF_DSC)
+	if (priv->cpu_interface & CPUIF_DSC)
 		priv->can.clock.freq /= 2;
 	priv->clkout = pdata->cor;
 	priv->bus_config = pdata->bcr;

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Joe Perches

author

2012-05-30 20:25 UTC #5 lore raw

PATCH 3/4 brcmfmac: Fix likely misuse of | for &

In reply to Joe Perches

Using | with a constant is always true.
Likely this should have be &.

Signed-off-by Joe Perches <[email protected]>

---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +2 -2

index e2480d1..8e7e692 100644
@@ -89,9 +89,9 @@ int brcmf_sdio_intr_register(struct brcmf_sdio_dev *sdiodev)
 	data |= 1 << SDIO_FUNC_1 | 1 << SDIO_FUNC_2 | 1;
 	brcmf_sdio_regwb(sdiodev, SDIO_CCCR_IENx, data, &ret);
 
-	/* redirect, configure ane enable io for interrupt signal */
+	/* redirect, configure and enable io for interrupt signal */
 	data = SDIO_SEPINT_MASK | SDIO_SEPINT_OE;
-	if (sdiodev->irq_flags | IRQF_TRIGGER_HIGH)
+	if (sdiodev->irq_flags & IRQF_TRIGGER_HIGH)
 		data |= SDIO_SEPINT_ACT_HI;
 	brcmf_sdio_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, data, &ret);

Signature · 1 line

1.7.8.111.gad25c.dirty

Arend van Spriel

Acked-by on v1 3/4

2012-05-30 20:50 UTC #6 lore raw

Re: [PATCH 3/4] brcmfmac: Fix likely misuse of | for &

Quoted 2 lines · Joe Perches

Using | with a constant is always true.
Likely this should have be &.

Acked-by Arend van Spriel <[email protected]>

Quoted 4 lines

Signed-off-by Joe Perches <[email protected]>

---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Joe Perches

author

2012-05-30 20:26 UTC #7 lore raw

PATCH 4/4 usb: gadget: pch_udc: Fix likely misuse of | for &

In reply to Joe Perches

Using | with a constant is always true.
Likely this should have be &.

Signed-off-by Joe Perches <[email protected]>

---
 drivers/usb/gadget/pch_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/gadget/pch_udc.c +1 -1

index 1cfcc9e..f4fb71c 100644
@@ -2208,7 +2208,7 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
 			return;
 		}
 		if ((td->status & PCH_UDC_BUFF_STS) == PCH_UDC_BS_DMA_DONE)
-			if (td->status | PCH_UDC_DMA_LAST) {
+			if (td->status & PCH_UDC_DMA_LAST) {
 				count = td->status & PCH_UDC_RXTX_BYTES;
 				break;
 			}

Signature · 1 line

1.7.8.111.gad25c.dirty

Felipe Balbi

Acked-by on v1 4/4

2012-05-31 07:29 UTC #8 lore raw

Re: [PATCH 4/4] usb: gadget: pch_udc: Fix likely misuse of | for &

Quoted 4 lines · Joe Perches

Using | with a constant is always true.
Likely this should have be &.
Signed-off-by Joe Perches <[email protected]>

this looks good to me. Do you want me to carry it on my tree ?

If you want to carry it out as a fix:

Acked-by Felipe Balbi <[email protected]>

Quoted 20 lines

---
 drivers/usb/gadget/pch_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/gadget/pch_udc.c +1 -1

index 1cfcc9e..f4fb71c 100644
@@ -2208,7 +2208,7 @@ static void pch_udc_complete_receiver(struct pch_udc_ep *ep)
 			return;
 		}
 		if ((td->status & PCH_UDC_BUFF_STS) == PCH_UDC_BS_DMA_DONE)
-			if (td->status | PCH_UDC_DMA_LAST) {
+			if (td->status & PCH_UDC_DMA_LAST) {
 				count = td->status & PCH_UDC_RXTX_BYTES;
 				break;
 			}

Signature · 1 line

1.7.8.111.gad25c.dirty

Signature · 1 line

balbi

Joe Perches

author

2012-05-31 19:44 UTC #9 lore raw

Re: [PATCH 4/4] usb: gadget: pch_udc: Fix likely misuse of | for &

Quoted 7 lines · Felipe Balbi

Quoted 3 lines · Joe Perches
Using | with a constant is always true.
Likely this should have be &.
Signed-off-by Joe Perches <[email protected]>
this looks good to me. Do you want me to carry it on my tree ? If you want to carry it out as a fix:
Acked-by Felipe Balbi <[email protected]>

I presume that question is for Greg.

Felipe Balbi

2012-06-01 09:52 UTC #10 lore raw

Re: [PATCH 4/4] usb: gadget: pch_udc: Fix likely misuse of | for &

Quoted 10 lines · Joe Perches

Quoted 7 lines · Felipe Balbi
Quoted 3 lines · Joe Perches
Using | with a constant is always true.
Likely this should have be &.
Signed-off-by Joe Perches <[email protected]>
this looks good to me. Do you want me to carry it on my tree ? If you want to carry it out as a fix:
Acked-by Felipe Balbi <[email protected]>
I presume that question is for Greg.

Then I'll take it... driver/usb/gadget/ goes through me.

Signature · 1 line

balbi