[patch] HID: wacom: bitwise vs logical ORs

[patch] HID: wacom: bitwise vs logical ORs

[Dan Carpenter]

Smatch complains that these should probably be bitwise ORs instead of
logical.  It doesn't matter for "prox" but it makes a difference for
"strip1" and "strip2".

Fixes: c7f0522a1ad1 ('HID: wacom: Slim down wacom_intuos_pad processing')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 7cf0417..af330aa 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -545,12 +545,12 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
 			          ((data[6] & 0x0F) << 4)  |
 			          (data[5] & 0x0F);
 		}
-		strip1 = (data[1] << 8) || data[2];
-		strip2 = (data[3] << 8) || data[4];
+		strip1 = (data[1] << 8) | data[2];
+		strip2 = (data[3] << 8) | data[4];
 	}
 
-	prox = (buttons & ~(~0 << nbuttons)) || (keys & ~(~0 << nkeys)) ||
-	       (ring1 & 0x80) || (ring2 & 0x80) || strip1 || strip2;
+	prox = (buttons & ~(~0 << nbuttons)) | (keys & ~(~0 << nkeys)) |
+	       (ring1 & 0x80) | (ring2 & 0x80) | strip1 | strip2;
 
 	wacom_report_numbered_buttons(input, nbuttons, buttons);
 

Re: [patch] HID: wacom: bitwise vs logical ORs

[Jiri Kosina]

On Wed, 9 Dec 2015, Dan Carpenter wrote:

> Smatch complains that these should probably be bitwise ORs instead of
> logical.  It doesn't matter for "prox" but it makes a difference for
> "strip1" and "strip2".
> 
> Fixes: c7f0522a1ad1 ('HID: wacom: Slim down wacom_intuos_pad processing')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Jason, could you please provide your Ack for this one?

It's obviously a Correct Thing To Do(TM), but I assume you've tested on 
your devices with this patch, so some other changes might potentially be 
needed to "compensate" for the fix ... 

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [patch] HID: wacom: bitwise vs logical ORs

[Jason Gerecke]

On Wed, Dec 16, 2015 at 6:57 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Wed, 9 Dec 2015, Dan Carpenter wrote:
>
>> Smatch complains that these should probably be bitwise ORs instead of
>> logical.  It doesn't matter for "prox" but it makes a difference for
>> "strip1" and "strip2".
>>
>> Fixes: c7f0522a1ad1 ('HID: wacom: Slim down wacom_intuos_pad processing')
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Jason, could you please provide your Ack for this one?
>
> It's obviously a Correct Thing To Do(TM), but I assume you've tested on
> your devices with this patch, so some other changes might potentially be
> needed to "compensate" for the fix ...
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

This patch looks fine to me (the 'prox' calculation /should/ be
logical, but I suppose bitwise works too :D). Found two other bugs
from c7f0522 while reviewing though -- I'll have patches for you
shortly.

Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

Re: [patch] HID: wacom: bitwise vs logical ORs

[Jiri Kosina]

On Wed, 16 Dec 2015, Jason Gerecke wrote:

> This patch looks fine to me (the 'prox' calculation /should/ be
> logical, but I suppose bitwise works too :D). Found two other bugs
> from c7f0522 while reviewing though -- I'll have patches for you
> shortly.
> 
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

Queued in for-4.5/wacom, thanks.

-- 
Jiri Kosina
SUSE Labs