| Message ID | 20231020090237.201029-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: wacom_sys: add error code check in wacom_feature_mapping | expand |
On Fri, 20 Oct, 2023 17:02:38 +0800 "Su Hui" <suhui@nfschina.com> wrote: > hid_report_raw_event() can return error code like '-ENOMEM' if > failed, so check 'ret' to make sure all things work fine. I can agree with adding logging for error cases personally. > > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > drivers/hid/wacom_sys.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c > index 3f704b8072e8..1f898d4ee708 100644 > --- a/drivers/hid/wacom_sys.c > +++ b/drivers/hid/wacom_sys.c > @@ -320,6 +320,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, > if (ret == n && features->type == HID_GENERIC) { > ret = hid_report_raw_event(hdev, > HID_FEATURE_REPORT, data, n, 0); > + if (ret) > + hid_warn(hdev, "failed to report feature\n"); I think we should report the returned error information as well. https://docs.kernel.org/core-api/printk-formats.html#error-pointers Typically what I do is use ERR_PTR in tandem with the %pe modifier for printing errors. > } else if (ret == 2 && features->type != HID_GENERIC) { > features->touch_max = data[1]; > } else { > @@ -381,6 +383,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, > if (ret == n) { > ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, > data, n, 0); > + if (ret) > + hid_warn(hdev, "failed to report feature\n"); > } else { > hid_warn(hdev, "%s: could not retrieve sensor offsets\n", > __func__); -- Thanks for the patch, Rahul Rameshbabu
On 2023/11/6 11:57, Rahul Rameshbabu wrote: > On Fri, 20 Oct, 2023 17:02:38 +0800 "Su Hui" <suhui@nfschina.com> wrote: >> hid_report_raw_event() can return error code like '-ENOMEM' if >> failed, so check 'ret' to make sure all things work fine. > I can agree with adding logging for error cases personally. > >> Signed-off-by: Su Hui <suhui@nfschina.com> >> --- >> drivers/hid/wacom_sys.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c >> index 3f704b8072e8..1f898d4ee708 100644 >> --- a/drivers/hid/wacom_sys.c >> +++ b/drivers/hid/wacom_sys.c >> @@ -320,6 +320,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, >> if (ret == n && features->type == HID_GENERIC) { >> ret = hid_report_raw_event(hdev, >> HID_FEATURE_REPORT, data, n, 0); >> + if (ret) >> + hid_warn(hdev, "failed to report feature\n"); > I think we should report the returned error information as well. Agreed, I will send v2 soon. Thanks for your suggestions! Su Hui > > https://docs.kernel.org/core-api/printk-formats.html#error-pointers > > Typically what I do is use ERR_PTR in tandem with the %pe modifier for > printing errors. > >> } else if (ret == 2 && features->type != HID_GENERIC) { >> features->touch_max = data[1]; >> } else { >> @@ -381,6 +383,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, >> if (ret == n) { >> ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, >> data, n, 0); >> + if (ret) >> + hid_warn(hdev, "failed to report feature\n"); >> } else { >> hid_warn(hdev, "%s: could not retrieve sensor offsets\n", >> __func__); > -- > Thanks for the patch, > > Rahul Rameshbabu >
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 3f704b8072e8..1f898d4ee708 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -320,6 +320,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, if (ret == n && features->type == HID_GENERIC) { ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, data, n, 0); + if (ret) + hid_warn(hdev, "failed to report feature\n"); } else if (ret == 2 && features->type != HID_GENERIC) { features->touch_max = data[1]; } else { @@ -381,6 +383,8 @@ static void wacom_feature_mapping(struct hid_device *hdev, if (ret == n) { ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, data, n, 0); + if (ret) + hid_warn(hdev, "failed to report feature\n"); } else { hid_warn(hdev, "%s: could not retrieve sensor offsets\n", __func__);
hid_report_raw_event() can return error code like '-ENOMEM' if failed, so check 'ret' to make sure all things work fine. Signed-off-by: Su Hui <suhui@nfschina.com> --- drivers/hid/wacom_sys.c | 4 ++++ 1 file changed, 4 insertions(+)