| Message ID | 20170425182956.15406-1-killertofu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, April 25, 2017, Jason Gerecke <killertofu@gmail.com> wrote: > > The following Smatch complaint was generated in response to commit > 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): > > drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() > error: we previously assumed 'wacom->touch_input' could be null (see line 1577) > > The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' > used for relaying touch and pen events to userspace, respectively. If a > device does not have a touch interface or pen interface, the associated > input variable is NULL. The 'wacom_tpc_irq()' function is responsible for > forwarding input reports to a more-specific IRQ handler function. An > unknown report could theoretically be mistaken as e.g. a touch report > on a device which does not have a touch interface. This can be prevented > by only calling the pen/touch functions are called when the pen/touch > pointers are valid. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Reviewed-by: Ping Cheng <ping.cheng@wacom.com> Looks good to me. Cheers, Ping > > --- > drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- > 1 file changed, 23 insertions(+), 22 deletions(-) > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c > index 6b8f6b816195..b963499e3351 100644 > --- a/drivers/hid/wacom_wac.c > +++ b/drivers/hid/wacom_wac.c > @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) > { > unsigned char *data = wacom->data; > > - if (wacom->pen_input) > + if (wacom->pen_input) { > dev_dbg(wacom->pen_input->dev.parent, > "%s: received report #%d\n", __func__, data[0]); > - else if (wacom->touch_input) > + > + if (len == WACOM_PKGLEN_PENABLED || > + data[0] == WACOM_REPORT_PENABLED) > + return wacom_tpc_pen(wacom); > + } > + else if (wacom->touch_input) { > dev_dbg(wacom->touch_input->dev.parent, > "%s: received report #%d\n", __func__, data[0]); > > - switch (len) { > - case WACOM_PKGLEN_TPC1FG: > - return wacom_tpc_single_touch(wacom, len); > + switch (len) { > + case WACOM_PKGLEN_TPC1FG: > + return wacom_tpc_single_touch(wacom, len); > > - case WACOM_PKGLEN_TPC2FG: > - return wacom_tpc_mt_touch(wacom); > + case WACOM_PKGLEN_TPC2FG: > + return wacom_tpc_mt_touch(wacom); > > - case WACOM_PKGLEN_PENABLED: > - return wacom_tpc_pen(wacom); > + default: > + switch (data[0]) { > + case WACOM_REPORT_TPC1FG: > + case WACOM_REPORT_TPCHID: > + case WACOM_REPORT_TPCST: > + case WACOM_REPORT_TPC1FGE: > + return wacom_tpc_single_touch(wacom, len); > > - default: > - switch (data[0]) { > - case WACOM_REPORT_TPC1FG: > - case WACOM_REPORT_TPCHID: > - case WACOM_REPORT_TPCST: > - case WACOM_REPORT_TPC1FGE: > - return wacom_tpc_single_touch(wacom, len); > - > - case WACOM_REPORT_TPCMT: > - case WACOM_REPORT_TPCMT2: > - return wacom_mt_touch(wacom); > + case WACOM_REPORT_TPCMT: > + case WACOM_REPORT_TPCMT2: > + return wacom_mt_touch(wacom); > > - case WACOM_REPORT_PENABLED: > - return wacom_tpc_pen(wacom); > + } > } > } > > -- > 2.12.2 > -- 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
Just making sure this doesn't get lost in the cracks. 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.... On Tue, Apr 25, 2017 at 1:56 PM, Ping Cheng <pinglinux@gmail.com> wrote: > On Tuesday, April 25, 2017, Jason Gerecke <killertofu@gmail.com> wrote: >> >> The following Smatch complaint was generated in response to commit >> 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): >> >> drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() >> error: we previously assumed 'wacom->touch_input' could be null (see line 1577) >> >> The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' >> used for relaying touch and pen events to userspace, respectively. If a >> device does not have a touch interface or pen interface, the associated >> input variable is NULL. The 'wacom_tpc_irq()' function is responsible for >> forwarding input reports to a more-specific IRQ handler function. An >> unknown report could theoretically be mistaken as e.g. a touch report >> on a device which does not have a touch interface. This can be prevented >> by only calling the pen/touch functions are called when the pen/touch >> pointers are valid. >> >> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > Reviewed-by: Ping Cheng <ping.cheng@wacom.com> > > Looks good to me. > > Cheers, > Ping > >> >> --- >> drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- >> 1 file changed, 23 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c >> index 6b8f6b816195..b963499e3351 100644 >> --- a/drivers/hid/wacom_wac.c >> +++ b/drivers/hid/wacom_wac.c >> @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) >> { >> unsigned char *data = wacom->data; >> >> - if (wacom->pen_input) >> + if (wacom->pen_input) { >> dev_dbg(wacom->pen_input->dev.parent, >> "%s: received report #%d\n", __func__, data[0]); >> - else if (wacom->touch_input) >> + >> + if (len == WACOM_PKGLEN_PENABLED || >> + data[0] == WACOM_REPORT_PENABLED) >> + return wacom_tpc_pen(wacom); >> + } >> + else if (wacom->touch_input) { >> dev_dbg(wacom->touch_input->dev.parent, >> "%s: received report #%d\n", __func__, data[0]); >> >> - switch (len) { >> - case WACOM_PKGLEN_TPC1FG: >> - return wacom_tpc_single_touch(wacom, len); >> + switch (len) { >> + case WACOM_PKGLEN_TPC1FG: >> + return wacom_tpc_single_touch(wacom, len); >> >> - case WACOM_PKGLEN_TPC2FG: >> - return wacom_tpc_mt_touch(wacom); >> + case WACOM_PKGLEN_TPC2FG: >> + return wacom_tpc_mt_touch(wacom); >> >> - case WACOM_PKGLEN_PENABLED: >> - return wacom_tpc_pen(wacom); >> + default: >> + switch (data[0]) { >> + case WACOM_REPORT_TPC1FG: >> + case WACOM_REPORT_TPCHID: >> + case WACOM_REPORT_TPCST: >> + case WACOM_REPORT_TPC1FGE: >> + return wacom_tpc_single_touch(wacom, len); >> >> - default: >> - switch (data[0]) { >> - case WACOM_REPORT_TPC1FG: >> - case WACOM_REPORT_TPCHID: >> - case WACOM_REPORT_TPCST: >> - case WACOM_REPORT_TPC1FGE: >> - return wacom_tpc_single_touch(wacom, len); >> - >> - case WACOM_REPORT_TPCMT: >> - case WACOM_REPORT_TPCMT2: >> - return wacom_mt_touch(wacom); >> + case WACOM_REPORT_TPCMT: >> + case WACOM_REPORT_TPCMT2: >> + return wacom_mt_touch(wacom); >> >> - case WACOM_REPORT_PENABLED: >> - return wacom_tpc_pen(wacom); >> + } >> } >> } >> >> -- >> 2.12.2 >> -- 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 Tue, 2 May 2017, Jason Gerecke wrote:
> Just making sure this doesn't get lost in the cracks.
I will definitely be picking it up for 4.12-rc.
I believe you wanted to add Fixes: tag and -stable inclusion anotation as
well.
Thanks,
On Tue, 25 Apr 2017, Jason Gerecke wrote: > The following Smatch complaint was generated in response to commit > 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): > > drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() > error: we previously assumed 'wacom->touch_input' could be null (see line 1577) > > The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' > used for relaying touch and pen events to userspace, respectively. If a > device does not have a touch interface or pen interface, the associated > input variable is NULL. The 'wacom_tpc_irq()' function is responsible for > forwarding input reports to a more-specific IRQ handler function. An > unknown report could theoretically be mistaken as e.g. a touch report > on a device which does not have a touch interface. This can be prevented > by only calling the pen/touch functions are called when the pen/touch > pointers are valid. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Applied to for-4.12/upstream-fixes branch with these tags: Fixes: 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device") Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Reviewed-by: Ping Cheng <ping.cheng@wacom.com> Cc: stable@vger.kernel.org
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 6b8f6b816195..b963499e3351 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) { unsigned char *data = wacom->data; - if (wacom->pen_input) + if (wacom->pen_input) { dev_dbg(wacom->pen_input->dev.parent, "%s: received report #%d\n", __func__, data[0]); - else if (wacom->touch_input) + + if (len == WACOM_PKGLEN_PENABLED || + data[0] == WACOM_REPORT_PENABLED) + return wacom_tpc_pen(wacom); + } + else if (wacom->touch_input) { dev_dbg(wacom->touch_input->dev.parent, "%s: received report #%d\n", __func__, data[0]); - switch (len) { - case WACOM_PKGLEN_TPC1FG: - return wacom_tpc_single_touch(wacom, len); + switch (len) { + case WACOM_PKGLEN_TPC1FG: + return wacom_tpc_single_touch(wacom, len); - case WACOM_PKGLEN_TPC2FG: - return wacom_tpc_mt_touch(wacom); + case WACOM_PKGLEN_TPC2FG: + return wacom_tpc_mt_touch(wacom); - case WACOM_PKGLEN_PENABLED: - return wacom_tpc_pen(wacom); + default: + switch (data[0]) { + case WACOM_REPORT_TPC1FG: + case WACOM_REPORT_TPCHID: + case WACOM_REPORT_TPCST: + case WACOM_REPORT_TPC1FGE: + return wacom_tpc_single_touch(wacom, len); - default: - switch (data[0]) { - case WACOM_REPORT_TPC1FG: - case WACOM_REPORT_TPCHID: - case WACOM_REPORT_TPCST: - case WACOM_REPORT_TPC1FGE: - return wacom_tpc_single_touch(wacom, len); - - case WACOM_REPORT_TPCMT: - case WACOM_REPORT_TPCMT2: - return wacom_mt_touch(wacom); + case WACOM_REPORT_TPCMT: + case WACOM_REPORT_TPCMT2: + return wacom_mt_touch(wacom); - case WACOM_REPORT_PENABLED: - return wacom_tpc_pen(wacom); + } } }
The following Smatch complaint was generated in response to commit 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() error: we previously assumed 'wacom->touch_input' could be null (see line 1577) The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' used for relaying touch and pen events to userspace, respectively. If a device does not have a touch interface or pen interface, the associated input variable is NULL. The 'wacom_tpc_irq()' function is responsible for forwarding input reports to a more-specific IRQ handler function. An unknown report could theoretically be mistaken as e.g. a touch report on a device which does not have a touch interface. This can be prevented by only calling the pen/touch functions are called when the pen/touch pointers are valid. Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> --- drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-)