| Message ID | 1437069433-76366-1-git-send-email-aksgarg1989@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Anshul, On Thu, Jul 16, 2015 at 10:57:13AM -0700, Anshul Garg wrote: > Added check for poll_interval range in input_register_polled_device > function similar to input_polldev_set_poll function. > > Signed-off-by: Anshul Garg <aksgarg1989@gmail.com> > --- > drivers/input/input-polldev.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/input/input-polldev.c b/drivers/input/input-polldev.c > index 3664f81..0e88a9e 100644 > --- a/drivers/input/input-polldev.c > +++ b/drivers/input/input-polldev.c > @@ -309,6 +309,12 @@ int input_register_polled_device(struct input_polled_dev *dev) > > input_set_drvdata(input, dev); > INIT_DELAYED_WORK(&dev->work, input_polled_device_work); > + > + if (dev->poll_interval > dev->poll_interval_max || > + dev->poll_interval < dev->poll_interval_min) { > + pr_err("Poll interval Range incorrect\n"); > + return -EINVAL; > + } What driver was this change tested with? Thanks.
Hi Mr. Dmitry , On Fri, Jul 17, 2015 at 1:05 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > Hi Anshul, > > On Thu, Jul 16, 2015 at 10:57:13AM -0700, Anshul Garg wrote: >> Added check for poll_interval range in input_register_polled_device >> function similar to input_polldev_set_poll function. >> >> Signed-off-by: Anshul Garg <aksgarg1989@gmail.com> >> --- >> drivers/input/input-polldev.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/input/input-polldev.c b/drivers/input/input-polldev.c >> index 3664f81..0e88a9e 100644 >> --- a/drivers/input/input-polldev.c >> +++ b/drivers/input/input-polldev.c >> @@ -309,6 +309,12 @@ int input_register_polled_device(struct input_polled_dev *dev) >> >> input_set_drvdata(input, dev); >> INIT_DELAYED_WORK(&dev->work, input_polled_device_work); >> + >> + if (dev->poll_interval > dev->poll_interval_max || >> + dev->poll_interval < dev->poll_interval_min) { >> + pr_err("Poll interval Range incorrect\n"); >> + return -EINVAL; >> + } > > What driver was this change tested with? I have not checked this change for any driver. This check is useful if some input driver is setting outofrange poll_interval value i.e. not in range between min,max. Similar to input_polldev_set_poll which does this when user space tries to change poll interval Thanks Anshul Garg > > Thanks. > > -- > Dmitry -- 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 Thu, Jul 16, 2015 at 7:57 PM, Anshul Garg <aksgarg1989@gmail.com> wrote: > + if (dev->poll_interval > dev->poll_interval_max || > + dev->poll_interval < dev->poll_interval_min) { > + pr_err("Poll interval Range incorrect\n"); > + return -EINVAL; > + } > > if (!dev->poll_interval) > dev->poll_interval = 500; So priviously, if poll_interval was 0 you got a sane default of 500, and now you get -EINVAL. Is this really useful and correct, and nobody using it is passing 0? regards Marcus -- 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 Fri, Jul 17, 2015 at 07:11:04PM +0530, Anshul Garg wrote: > Hi Mr. Dmitry > , > > > > On Fri, Jul 17, 2015 at 1:05 AM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > Hi Anshul, > > > > On Thu, Jul 16, 2015 at 10:57:13AM -0700, Anshul Garg wrote: > >> Added check for poll_interval range in input_register_polled_device > >> function similar to input_polldev_set_poll function. > >> > >> Signed-off-by: Anshul Garg <aksgarg1989@gmail.com> > >> --- > >> drivers/input/input-polldev.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/drivers/input/input-polldev.c b/drivers/input/input-polldev.c > >> index 3664f81..0e88a9e 100644 > >> --- a/drivers/input/input-polldev.c > >> +++ b/drivers/input/input-polldev.c > >> @@ -309,6 +309,12 @@ int input_register_polled_device(struct input_polled_dev *dev) > >> > >> input_set_drvdata(input, dev); > >> INIT_DELAYED_WORK(&dev->work, input_polled_device_work); > >> + > >> + if (dev->poll_interval > dev->poll_interval_max || > >> + dev->poll_interval < dev->poll_interval_min) { > >> + pr_err("Poll interval Range incorrect\n"); > >> + return -EINVAL; > >> + } > > > > What driver was this change tested with? > I have not checked this change for any driver. > Yes, I know that you did not test it because if you would have tested it you would have noticed that it breaks existing devices, since hardly any of them set dev->poll_interval_max and instead rely on the polldev code to apply reasonable defaults. There have been a few patches that had errors that would have been easily caught if you tested the changes. Please next time you submit a patch mention how you tested it to make sure it works as intended. Thanks.
diff --git a/drivers/input/input-polldev.c b/drivers/input/input-polldev.c index 3664f81..0e88a9e 100644 --- a/drivers/input/input-polldev.c +++ b/drivers/input/input-polldev.c @@ -309,6 +309,12 @@ int input_register_polled_device(struct input_polled_dev *dev) input_set_drvdata(input, dev); INIT_DELAYED_WORK(&dev->work, input_polled_device_work); + + if (dev->poll_interval > dev->poll_interval_max || + dev->poll_interval < dev->poll_interval_min) { + pr_err("Poll interval Range incorrect\n"); + return -EINVAL; + } if (!dev->poll_interval) dev->poll_interval = 500;
Added check for poll_interval range in input_register_polled_device function similar to input_polldev_set_poll function. Signed-off-by: Anshul Garg <aksgarg1989@gmail.com> --- drivers/input/input-polldev.c | 6 ++++++ 1 file changed, 6 insertions(+)