| Message ID | 20151209102205.GB3173@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
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,
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.... -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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);
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> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html