HID: wacom: bitwise vs logical ORs

PATCH Applied Dan Carpenter 3 participants 4 messages Dec 9, 2015 – Dec 17, 2015

Subsystems (3)

Kernel janitors HID HID wacom driver

lore

Share & tools

lore

Dan Carpenter

author

2015-12-09 10:22 UTC #0 lore raw

PATCH 1/1 HID: wacom: bitwise vs logical ORs

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

drivers/hid/wacom_wac.c +4 -4

index 7cf0417..af330aa 100644
@@ -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);

Jiri Kosina

maintainer

2015-12-16 14:57 UTC #1 lore raw

Quoted 6 lines · 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 <[email protected]>

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,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Jason Gerecke

Reviewed-by

2015-12-16 17:49 UTC #2 lore raw

Quoted 15 lines · Jiri Kosina

Quoted 6 lines · 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 <[email protected]>
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 ...

Quoted 6 lines

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

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....

Jiri Kosina

maintainer Applied

2015-12-17 23:02 UTC #3 lore raw

Quoted 6 lines · Jason Gerecke

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

Queued in for-4.5/wacom, thanks.

Signature · 2 lines

Jiri Kosina
SUSE Labs