[PATCH 0/5] [trivial] Fix dubious bitwise and/or usage.

[PATCH 0/5] [trivial] Fix dubious bitwise and/or usage.

[Alexey Zaytsev]

Hello.

Sorry to bother the lkml with such trivial stuff, but
I could not get any response from either Jiri Kosina
ot trivial@ when I sent the patches directly to them.
Maybe a linux-trivial@ mailing list would be a good idea..


These patches fix a number of cases where &/| were by
mistake used instead of &&/||. In every single case the
semantics stays the same, so none is a real bug.

---

Alexey Zaytsev (5):
      Fix dubious bitwise 'and' usage spotted by sparse.
      Fix dubious bitwise 'and' usage spotted by sparse.
      Fix dubious bitwise 'and' usage spotted by sparse.
      Fix dubious bitwise 'or' usage spotted by sparse.
      Fix dubious bitwise 'or' usage spotted by sparse.


 drivers/isdn/mISDN/l1oip_codec.c               |    2 +-
 drivers/media/video/gspca/m5602/m5602_s5k4aa.c |    2 +-
 drivers/net/wireless/ath9k/rc.c                |    2 +-
 drivers/usb/wusbcore/security.c                |    2 +-
 mm/page_alloc.c                                |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

--

[PATCH 1/5] Fix dubious bitwise 'or' usage spotted by sparse.

[Alexey Zaytsev]

It doesn't change the semantics, but it looks like
the logical 'or' was meant to be used here.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 mm/page_alloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d8ac014..6923237 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -306,7 +306,7 @@ static void destroy_compound_page(struct page *page, unsigned long order)
 	for (i = 1; i < nr_pages; i++) {
 		struct page *p = page + i;
 
-		if (unlikely(!PageTail(p) |
+		if (unlikely(!PageTail(p) ||
 				(p->first_page != page)))
 			bad_page(page);
 		__ClearPageTail(p);

[PATCH 2/5] Fix dubious bitwise 'or' usage spotted by sparse.

[Alexey Zaytsev]

It doesn't change the semantics, but it looks like
the logical 'or' was meant to be used here.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 drivers/isdn/mISDN/l1oip_codec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/mISDN/l1oip_codec.c b/drivers/isdn/mISDN/l1oip_codec.c
index a2dc457..43b3795 100644
--- a/drivers/isdn/mISDN/l1oip_codec.c
+++ b/drivers/isdn/mISDN/l1oip_codec.c
@@ -330,7 +330,7 @@ l1oip_4bit_alloc(int ulaw)
 	/* alloc conversion tables */
 	table_com = vmalloc(65536);
 	table_dec = vmalloc(512);
-	if (!table_com | !table_dec) {
+	if (!table_com || !table_dec) {
 		l1oip_4bit_free();
 		return -ENOMEM;
 	}

[PATCH 3/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Alexey Zaytsev]

It doesn't change the semantics, but it looks like
the logical 'and' was meant to be used here.
---
 drivers/media/video/gspca/m5602/m5602_s5k4aa.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/gspca/m5602/m5602_s5k4aa.c b/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
index 14b1eac..c3ebcca 100644
--- a/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
+++ b/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
@@ -114,7 +114,7 @@ int s5k4aa_read_sensor(struct sd *sd, const u8 address,
 	if (err < 0)
 		goto out;
 
-	for (i = 0; (i < len) & !err; i++) {
+	for (i = 0; (i < len) && !err; i++) {
 		err = m5602_read_bridge(sd, M5602_XB_I2C_DATA, &(i2c_data[i]));
 
 		PDEBUG(D_CONF, "Reading sensor register "

[PATCH 4/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Alexey Zaytsev]

It doesn't change the semantics, but it looks like
the logical 'and' was meant to be used here.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 drivers/net/wireless/ath9k/rc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath9k/rc.c b/drivers/net/wireless/ath9k/rc.c
index cca2fc5..e8eb20d 100644
--- a/drivers/net/wireless/ath9k/rc.c
+++ b/drivers/net/wireless/ath9k/rc.c
@@ -609,7 +609,7 @@ ath_rc_get_nextvalid_txrate(const struct ath_rate_table *rate_table,
 
 static int ath_rc_valid_phyrate(u32 phy, u32 capflag, int ignore_cw)
 {
-	if (WLAN_RC_PHY_HT(phy) & !(capflag & WLAN_RC_HT_FLAG))
+	if (WLAN_RC_PHY_HT(phy) && !(capflag & WLAN_RC_HT_FLAG))
 		return FALSE;
 	if (WLAN_RC_PHY_DS(phy) && !(capflag & WLAN_RC_DS_FLAG))
 		return FALSE;

[PATCH 5/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Alexey Zaytsev]

It doesn't change the semantics, but clearly
the logical 'and' was meant to be used here.

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 drivers/usb/wusbcore/security.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index a101cad..8f953ab 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -626,7 +626,7 @@ void wusbhc_gtk_rekey(struct wusbhc *wusbhc)
 		struct wusb_dev *wusb_dev;
 
 		wusb_dev = wusbhc->port[p].wusb_dev;
-		if (!wusb_dev || !wusb_dev->usb_dev | !wusb_dev->usb_dev->authenticated)
+		if (!wusb_dev || !wusb_dev->usb_dev || !wusb_dev->usb_dev->authenticated)
 			continue;
 
 		usb_fill_control_urb(wusb_dev->set_gtk_urb, wusb_dev->usb_dev,

Re: [PATCH 3/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Jiri Kosina]

On Sat, 10 Jan 2009, Alexey Zaytsev wrote:

> It doesn't change the semantics, but it looks like
> the logical 'and' was meant to be used here.
> ---
>  drivers/media/video/gspca/m5602/m5602_s5k4aa.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/gspca/m5602/m5602_s5k4aa.c b/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
> index 14b1eac..c3ebcca 100644
> --- a/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
> +++ b/drivers/media/video/gspca/m5602/m5602_s5k4aa.c
> @@ -114,7 +114,7 @@ int s5k4aa_read_sensor(struct sd *sd, const u8 address,
>  	if (err < 0)
>  		goto out;
>  
> -	for (i = 0; (i < len) & !err; i++) {
> +	for (i = 0; (i < len) && !err; i++) {
>  		err = m5602_read_bridge(sd, M5602_XB_I2C_DATA, &(i2c_data[i]));
>  
>  		PDEBUG(D_CONF, "Reading sensor register "

Hi Alexey,

this one doesn't apply to Linus' tree anymore, the whole 
s5k4aa_read_sensor() function is not there.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 4/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Jiri Kosina]

On Sat, 10 Jan 2009, Alexey Zaytsev wrote:

> It doesn't change the semantics, but it looks like
> the logical 'and' was meant to be used here.
> 
> Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> ---
>  drivers/net/wireless/ath9k/rc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath9k/rc.c b/drivers/net/wireless/ath9k/rc.c
> index cca2fc5..e8eb20d 100644
> --- a/drivers/net/wireless/ath9k/rc.c
> +++ b/drivers/net/wireless/ath9k/rc.c
> @@ -609,7 +609,7 @@ ath_rc_get_nextvalid_txrate(const struct ath_rate_table *rate_table,
>  
>  static int ath_rc_valid_phyrate(u32 phy, u32 capflag, int ignore_cw)
>  {
> -	if (WLAN_RC_PHY_HT(phy) & !(capflag & WLAN_RC_HT_FLAG))
> +	if (WLAN_RC_PHY_HT(phy) && !(capflag & WLAN_RC_HT_FLAG))
>  		return FALSE;
>  	if (WLAN_RC_PHY_DS(phy) && !(capflag & WLAN_RC_DS_FLAG))
>  		return FALSE;

This one needs a small fixup, as the buggy code got moved around to 
ath_rc_valid_phyrate().

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/5] [trivial] Fix dubious bitwise and/or usage.

[Jiri Kosina]

On Sat, 10 Jan 2009, Alexey Zaytsev wrote:

> Sorry to bother the lkml with such trivial stuff, but I could not get 
> any response from either Jiri Kosina ot trivial@ when I sent the patches 
> directly to them. Maybe a linux-trivial@ mailing list would be a good 
> idea..

Sorry, I have just been busy with too many other things during the merge 
window.

> ---
> 
> Alexey Zaytsev (5):
>       Fix dubious bitwise 'and' usage spotted by sparse.
>       Fix dubious bitwise 'and' usage spotted by sparse.
>       Fix dubious bitwise 'and' usage spotted by sparse.
>       Fix dubious bitwise 'or' usage spotted by sparse.
>       Fix dubious bitwise 'or' usage spotted by sparse.

I have applied all the patches with exception for the one patching 
drivers/media/video/gspca/m5602/m5602_s5k4aa.c, as it doesn't apply any 
more.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/5] [trivial] Fix dubious bitwise and/or usage.

[Alexey Zaytsev]

On Sat, Jan 10, 2009 at 03:00, Jiri Kosina <jkosina@suse.cz> wrote:
> On Sat, 10 Jan 2009, Alexey Zaytsev wrote:
>
>> Sorry to bother the lkml with such trivial stuff, but I could not get
>> any response from either Jiri Kosina ot trivial@ when I sent the patches
>> directly to them. Maybe a linux-trivial@ mailing list would be a good
>> idea..
>
> Sorry, I have just been busy with too many other things during the merge
> window.
Sorry for the disturbance then. I was not sure if you received the patches.

>
>> ---
>>
>> Alexey Zaytsev (5):
>>       Fix dubious bitwise 'and' usage spotted by sparse.
>>       Fix dubious bitwise 'and' usage spotted by sparse.
>>       Fix dubious bitwise 'and' usage spotted by sparse.
>>       Fix dubious bitwise 'or' usage spotted by sparse.
>>       Fix dubious bitwise 'or' usage spotted by sparse.
>
> I have applied all the patches with exception for the one patching
> drivers/media/video/gspca/m5602/m5602_s5k4aa.c, as it doesn't apply any
> more.

Thank you. The bugs seem to be leaving by themselves. ;)

Re: [PATCH 5/5] Fix dubious bitwise 'and' usage spotted by sparse.

[Tom Spink]

2009/1/9 Alexey Zaytsev <alexey.zaytsev@gmail.com>:
> It doesn't change the semantics, but clearly
> the logical 'and' was meant to be used here.
>
> Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> ---
>  drivers/usb/wusbcore/security.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
> index a101cad..8f953ab 100644
> --- a/drivers/usb/wusbcore/security.c
> +++ b/drivers/usb/wusbcore/security.c
> @@ -626,7 +626,7 @@ void wusbhc_gtk_rekey(struct wusbhc *wusbhc)
>                struct wusb_dev *wusb_dev;
>
>                wusb_dev = wusbhc->port[p].wusb_dev;
> -               if (!wusb_dev || !wusb_dev->usb_dev | !wusb_dev->usb_dev->authenticated)
> +               if (!wusb_dev || !wusb_dev->usb_dev || !wusb_dev->usb_dev->authenticated)
>                        continue;
>
>                usb_fill_control_urb(wusb_dev->set_gtk_urb, wusb_dev->usb_dev,
>

Hi Alexey,

This one is a logical 'or'... your changelog says 'and'!

-- 
Tom Spink
Douglas William Jerrold  - "The only athletic sport I ever mastered
was backgammon."