* [PATCH] HID: hid-ntrig: fix the error handling code in ntrig_probe
@ 2022-03-13 15:02 Mingxuan Xiang
2022-03-30 15:25 ` Jiri Kosina
0 siblings, 1 reply; 3+ messages in thread
From: Mingxuan Xiang @ 2022-03-13 15:02 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Rafi Rubin
Cc: Mingxuan Xiang, Dongliang Mu, Jiri Kosina, linux-input, linux-kernel
If sysfs_create_group in ntrig_probe fails, sysfs_remove_group in
ntrig_remove would hit WARN().
Fix this by returning err in ntrig_probe rather than ignoring the
failure of sysfs_create_group.
CC: Dongliang Mu <dzm91@hust.edu.cn>
Fixes: eab32f5f6557 ("HID: ntrig: add sysfs access to filter parameters")
Signed-off-by: Mingxuan Xiang <mx_xiang@hust.edu.cn>
---
drivers/hid/hid-ntrig.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index b5d26f03fe6b..f49427815d98 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -951,10 +951,14 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = sysfs_create_group(&hdev->dev.kobj,
&ntrig_attribute_group);
- if (ret)
+ if (ret) {
hid_err(hdev, "cannot create sysfs group\n");
+ goto err_sysfs_create;
+ }
return 0;
+err_sysfs_create:
+ hid_hw_stop(hdev);
err_free:
kfree(nd);
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] HID: hid-ntrig: fix the error handling code in ntrig_probe
2022-03-13 15:02 [PATCH] HID: hid-ntrig: fix the error handling code in ntrig_probe Mingxuan Xiang
@ 2022-03-30 15:25 ` Jiri Kosina
2022-04-03 18:34 ` Rafi Rubin
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Kosina @ 2022-03-30 15:25 UTC (permalink / raw)
To: Mingxuan Xiang
Cc: Benjamin Tissoires, Rafi Rubin, Dongliang Mu, linux-input, linux-kernel
On Sun, 13 Mar 2022, Mingxuan Xiang wrote:
> If sysfs_create_group in ntrig_probe fails, sysfs_remove_group in
> ntrig_remove would hit WARN().
>
> Fix this by returning err in ntrig_probe rather than ignoring the
> failure of sysfs_create_group.
>
> CC: Dongliang Mu <dzm91@hust.edu.cn>
> Fixes: eab32f5f6557 ("HID: ntrig: add sysfs access to filter parameters")
> Signed-off-by: Mingxuan Xiang <mx_xiang@hust.edu.cn>
> ---
> drivers/hid/hid-ntrig.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index b5d26f03fe6b..f49427815d98 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -951,10 +951,14 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
> ret = sysfs_create_group(&hdev->dev.kobj,
> &ntrig_attribute_group);
> - if (ret)
> + if (ret) {
> hid_err(hdev, "cannot create sysfs group\n");
> + goto err_sysfs_create;
> + }
>
> return 0;
> +err_sysfs_create:
> + hid_hw_stop(hdev);
> err_free:
> kfree(nd);
Thanks for the fix.
However, I believe that the sole fact that sysfs_create_group() failed
should not be a reason for bailing the probe completely (rendering the
device completely dysfunctional), given the fact that the probe otherwise
succeeded and this just means that the parameters can't be tuned in
runtime.
So just taking a note whether we do actually have sysfs attributes created
on probe and freeing them only conditionally on remove() sounds like a bit
less drastic aproach to me.
Could you please adjust the fix and resubmit?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] HID: hid-ntrig: fix the error handling code in ntrig_probe
2022-03-30 15:25 ` Jiri Kosina
@ 2022-04-03 18:34 ` Rafi Rubin
0 siblings, 0 replies; 3+ messages in thread
From: Rafi Rubin @ 2022-04-03 18:34 UTC (permalink / raw)
To: Jiri Kosina, Mingxuan Xiang
Cc: Benjamin Tissoires, Dongliang Mu, linux-input, linux-kernel
The writable parameters are also settable as module parameters and I suspect are not really widely used. I used them for tuning the defaults and dealing with unreliable devices, not in the course of normal use. So I agree creation of the sysfs nodes
should not block probing and using the device.
Thank you for working on the error handling,
Rafi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-03 18:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-13 15:02 [PATCH] HID: hid-ntrig: fix the error handling code in ntrig_probe Mingxuan Xiang
2022-03-30 15:25 ` Jiri Kosina
2022-04-03 18:34 ` Rafi Rubin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.