2009-01-09 23:39:14

by Alexey Zaytsev

[permalink] [raw]
Subject: [PATCH 0/5] [trivial] Fix dubious bitwise and/or usage.

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

--


2009-01-09 23:39:34

by Alexey Zaytsev

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

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

Signed-off-by: Alexey Zaytsev <[email protected]>
---
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);

2009-01-09 23:39:49

by Alexey Zaytsev

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

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

Signed-off-by: Alexey Zaytsev <[email protected]>
---
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;
}

2009-01-09 23:40:18

by Alexey Zaytsev

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

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 "

2009-01-09 23:40:55

by Alexey Zaytsev

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

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

Signed-off-by: Alexey Zaytsev <[email protected]>
---
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;

2009-01-09 23:41:42

by Alexey Zaytsev

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

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

Signed-off-by: Alexey Zaytsev <[email protected]>
---
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,

2009-01-09 23:56:24

by Jiri Kosina

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

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

2009-01-09 23:59:13

by Jiri Kosina

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

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

2009-01-10 00:00:40

by Jiri Kosina

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

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

2009-01-10 01:29:16

by Alexey Zaytsev

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

On Sat, Jan 10, 2009 at 03:00, Jiri Kosina <[email protected]> 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. ;)

2009-01-10 09:47:26

by Tom Spink

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

2009/1/9 Alexey Zaytsev <[email protected]>:
> It doesn't change the semantics, but clearly
> the logical 'and' was meant to be used here.
>
> Signed-off-by: Alexey Zaytsev <[email protected]>
> ---
> 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."