* [PATCH] HID: hid-sensor-custom: remove useless variable
@ 2021-04-09 7:15 Jiapeng Chong
2021-04-09 18:19 ` Srinivas Pandruvada
0 siblings, 1 reply; 6+ messages in thread
From: Jiapeng Chong @ 2021-04-09 7:15 UTC (permalink / raw)
To: jikos
Cc: jic23, srinivas.pandruvada, benjamin.tissoires, linux-input,
linux-iio, linux-kernel, Jiapeng Chong
Fix the following gcc warning:
drivers/hid/hid-sensor-custom.c:400:7: warning: variable ‘ret’ set but
not used [-Wunused-but-set-variable].
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
drivers/hid/hid-sensor-custom.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index 2628bc5..e430673 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -397,15 +397,14 @@ static ssize_t store_value(struct device *dev, struct device_attribute *attr,
if (!strncmp(name, "value", strlen("value"))) {
u32 report_id;
- int ret;
if (kstrtoint(buf, 0, &value) != 0)
return -EINVAL;
report_id = sensor_inst->fields[field_index].attribute.
report_id;
- ret = sensor_hub_set_feature(sensor_inst->hsdev, report_id,
- index, sizeof(value), &value);
+ sensor_hub_set_feature(sensor_inst->hsdev, report_id, index,
+ sizeof(value), &value);
} else
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] HID: hid-sensor-custom: remove useless variable
2021-04-09 7:15 [PATCH] HID: hid-sensor-custom: remove useless variable Jiapeng Chong
@ 2021-04-09 18:19 ` Srinivas Pandruvada
2021-04-11 13:56 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas Pandruvada @ 2021-04-09 18:19 UTC (permalink / raw)
To: Jiapeng Chong, jikos
Cc: jic23, benjamin.tissoires, linux-input, linux-iio, linux-kernel
On Fri, 2021-04-09 at 15:15 +0800, Jiapeng Chong wrote:
> Fix the following gcc warning:
>
> drivers/hid/hid-sensor-custom.c:400:7: warning: variable ‘ret’ set
> but
> not used [-Wunused-but-set-variable].
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com
> ---
> drivers/hid/hid-sensor-custom.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> sensor-custom.c
> index 2628bc5..e430673 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -397,15 +397,14 @@ static ssize_t store_value(struct device *dev,
> struct device_attribute *attr,
>
> if (!strncmp(name, "value", strlen("value"))) {
> u32 report_id;
> - int ret;
>
> if (kstrtoint(buf, 0, &value) != 0)
> return -EINVAL;
>
> report_id = sensor_inst->fields[field_index].attribute.
> report_
> id;
> - ret = sensor_hub_set_feature(sensor_inst->hsdev,
> report_id,
> - index, sizeof(value),
> &value);
> + sensor_hub_set_feature(sensor_inst->hsdev, report_id,
> index,
> + sizeof(value), &value);
> } else
> return -EINVAL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] HID: hid-sensor-custom: remove useless variable
2021-04-09 18:19 ` Srinivas Pandruvada
@ 2021-04-11 13:56 ` Jonathan Cameron
2021-04-11 16:06 ` Srinivas Pandruvada
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2021-04-11 13:56 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Jiapeng Chong, jikos, benjamin.tissoires, linux-input, linux-iio,
linux-kernel
On Fri, 09 Apr 2021 11:19:12 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> On Fri, 2021-04-09 at 15:15 +0800, Jiapeng Chong wrote:
> > Fix the following gcc warning:
> >
> > drivers/hid/hid-sensor-custom.c:400:7: warning: variable ‘ret’ set
> > but
> > not used [-Wunused-but-set-variable].
> >
> > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com
Perhaps better to return ret if it is non zero?
I can't immediately figure out if there is a reason we know that can't
happen.
Jonathan
>
> > ---
> > drivers/hid/hid-sensor-custom.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> > sensor-custom.c
> > index 2628bc5..e430673 100644
> > --- a/drivers/hid/hid-sensor-custom.c
> > +++ b/drivers/hid/hid-sensor-custom.c
> > @@ -397,15 +397,14 @@ static ssize_t store_value(struct device *dev,
> > struct device_attribute *attr,
> >
> > if (!strncmp(name, "value", strlen("value"))) {
> > u32 report_id;
> > - int ret;
> >
> > if (kstrtoint(buf, 0, &value) != 0)
> > return -EINVAL;
> >
> > report_id = sensor_inst->fields[field_index].attribute.
> > report_
> > id;
> > - ret = sensor_hub_set_feature(sensor_inst->hsdev,
> > report_id,
> > - index, sizeof(value),
> > &value);
> > + sensor_hub_set_feature(sensor_inst->hsdev, report_id,
> > index,
> > + sizeof(value), &value);
> > } else
> > return -EINVAL;
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] HID: hid-sensor-custom: remove useless variable
2021-04-11 13:56 ` Jonathan Cameron
@ 2021-04-11 16:06 ` Srinivas Pandruvada
2021-04-12 10:20 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas Pandruvada @ 2021-04-11 16:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiapeng Chong, jikos, benjamin.tissoires, linux-input, linux-iio,
linux-kernel
On Sun, 2021-04-11 at 14:56 +0100, Jonathan Cameron wrote:
> On Fri, 09 Apr 2021 11:19:12 -0700
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
> > On Fri, 2021-04-09 at 15:15 +0800, Jiapeng Chong wrote:
> > > Fix the following gcc warning:
> > >
> > > drivers/hid/hid-sensor-custom.c:400:7: warning: variable ‘ret’
> > > set
> > > but
> > > not used [-Wunused-but-set-variable].
> > >
> > > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com
>
> Perhaps better to return ret if it is non zero?
> I can't immediately figure out if there is a reason we know that
> can't
> happen.
Only time it can fail when there is no report descriptor or the field
index is >= report->maxfield.
But since the attribute is registered from the report descriptor and
index, this can't happen.
But we can enhance sensor_hub_set_feature() to fail when
hid_set_field() fails. There is one case where field->logical_minimum
< 0 and value is out of range.
Thanks,
Srinivas
>
> Jonathan
>
> > > ---
> > > drivers/hid/hid-sensor-custom.c | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> > > sensor-custom.c
> > > index 2628bc5..e430673 100644
> > > --- a/drivers/hid/hid-sensor-custom.c
> > > +++ b/drivers/hid/hid-sensor-custom.c
> > > @@ -397,15 +397,14 @@ static ssize_t store_value(struct device
> > > *dev,
> > > struct device_attribute *attr,
> > >
> > > if (!strncmp(name, "value", strlen("value"))) {
> > > u32 report_id;
> > > - int ret;
> > >
> > > if (kstrtoint(buf, 0, &value) != 0)
> > > return -EINVAL;
> > >
> > > report_id = sensor_inst->fields[field_index].attribute.
> > > report_
> > > id;
> > > - ret = sensor_hub_set_feature(sensor_inst->hsdev,
> > > report_id,
> > > - index, sizeof(value),
> > > &value);
> > > + sensor_hub_set_feature(sensor_inst->hsdev, report_id,
> > > index,
> > > + sizeof(value), &value);
> > > } else
> > > return -EINVAL;
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] HID: hid-sensor-custom: remove useless variable
2021-04-11 16:06 ` Srinivas Pandruvada
@ 2021-04-12 10:20 ` Jonathan Cameron
2021-04-12 22:27 ` srinivas pandruvada
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2021-04-12 10:20 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Jonathan Cameron, Jiapeng Chong, jikos, benjamin.tissoires,
linux-input, linux-iio, linux-kernel
On Sun, 11 Apr 2021 09:06:35 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> On Sun, 2021-04-11 at 14:56 +0100, Jonathan Cameron wrote:
> > On Fri, 09 Apr 2021 11:19:12 -0700
> > Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> >
> > > On Fri, 2021-04-09 at 15:15 +0800, Jiapeng Chong wrote:
> > > > Fix the following gcc warning:
> > > >
> > > > drivers/hid/hid-sensor-custom.c:400:7: warning: variable ‘ret’
> > > > set
> > > > but
> > > > not used [-Wunused-but-set-variable].
> > > >
> > > > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > > > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com
> >
> > Perhaps better to return ret if it is non zero?
> > I can't immediately figure out if there is a reason we know that
> > can't
> > happen.
> Only time it can fail when there is no report descriptor or the field
> index is >= report->maxfield.
> But since the attribute is registered from the report descriptor and
> index, this can't happen.
> But we can enhance sensor_hub_set_feature() to fail when
> hid_set_field() fails. There is one case where field->logical_minimum
> < 0 and value is out of range.
I'll go with what you think. Apply as is, or handle the
return value because we might at some later date return an error that
can actually happen from here?
Jonathan
>
> Thanks,
> Srinivas
>
>
> >
> > Jonathan
> >
> > > > ---
> > > > drivers/hid/hid-sensor-custom.c | 5 ++---
> > > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> > > > sensor-custom.c
> > > > index 2628bc5..e430673 100644
> > > > --- a/drivers/hid/hid-sensor-custom.c
> > > > +++ b/drivers/hid/hid-sensor-custom.c
> > > > @@ -397,15 +397,14 @@ static ssize_t store_value(struct device
> > > > *dev,
> > > > struct device_attribute *attr,
> > > >
> > > > if (!strncmp(name, "value", strlen("value"))) {
> > > > u32 report_id;
> > > > - int ret;
> > > >
> > > > if (kstrtoint(buf, 0, &value) != 0)
> > > > return -EINVAL;
> > > >
> > > > report_id = sensor_inst->fields[field_index].attribute.
> > > > report_
> > > > id;
> > > > - ret = sensor_hub_set_feature(sensor_inst->hsdev,
> > > > report_id,
> > > > - index, sizeof(value),
> > > > &value);
> > > > + sensor_hub_set_feature(sensor_inst->hsdev, report_id,
> > > > index,
> > > > + sizeof(value), &value);
> > > > } else
> > > > return -EINVAL;
> > > >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] HID: hid-sensor-custom: remove useless variable
2021-04-12 10:20 ` Jonathan Cameron
@ 2021-04-12 22:27 ` srinivas pandruvada
0 siblings, 0 replies; 6+ messages in thread
From: srinivas pandruvada @ 2021-04-12 22:27 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, Jiapeng Chong, jikos, benjamin.tissoires,
linux-input, linux-iio, linux-kernel
On Mon, 2021-04-12 at 11:20 +0100, Jonathan Cameron wrote:
> On Sun, 11 Apr 2021 09:06:35 -0700
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
> > On Sun, 2021-04-11 at 14:56 +0100, Jonathan Cameron wrote:
> > > On Fri, 09 Apr 2021 11:19:12 -0700
> > > Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> > >
> > > > On Fri, 2021-04-09 at 15:15 +0800, Jiapeng Chong wrote:
> > > > > Fix the following gcc warning:
> > > > >
> > > > > drivers/hid/hid-sensor-custom.c:400:7: warning: variable
> > > > > ‘ret’
> > > > > set
> > > > > but
> > > > > not used [-Wunused-but-set-variable].
> > > > >
> > > > > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > > > > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com
> > > > > >
> > > > Acked-by: Srinivas Pandruvada <
> > > > srinivas.pandruvada@linux.intel.com
> > >
> > > Perhaps better to return ret if it is non zero?
> > > I can't immediately figure out if there is a reason we know that
> > > can't
> > > happen.
> > Only time it can fail when there is no report descriptor or the
> > field
> > index is >= report->maxfield.
> > But since the attribute is registered from the report descriptor
> > and
> > index, this can't happen.
> > But we can enhance sensor_hub_set_feature() to fail when
> > hid_set_field() fails. There is one case where field-
> > >logical_minimum
> > < 0 and value is out of range.
>
> I'll go with what you think. Apply as is, or handle the
> return value because we might at some later date return an error that
> can actually happen from here?
>
I will send you a patch this week to handle error condition.
Thanks,
Srinivas
> Jonathan
>
> > Thanks,
> > Srinivas
> >
> >
> > > Jonathan
> > >
> > > > > ---
> > > > > drivers/hid/hid-sensor-custom.c | 5 ++---
> > > > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hid/hid-sensor-custom.c
> > > > > b/drivers/hid/hid-
> > > > > sensor-custom.c
> > > > > index 2628bc5..e430673 100644
> > > > > --- a/drivers/hid/hid-sensor-custom.c
> > > > > +++ b/drivers/hid/hid-sensor-custom.c
> > > > > @@ -397,15 +397,14 @@ static ssize_t store_value(struct
> > > > > device
> > > > > *dev,
> > > > > struct device_attribute *attr,
> > > > >
> > > > > if (!strncmp(name, "value", strlen("value"))) {
> > > > > u32 report_id;
> > > > > - int ret;
> > > > >
> > > > > if (kstrtoint(buf, 0, &value) != 0)
> > > > > return -EINVAL;
> > > > >
> > > > > report_id = sensor_inst-
> > > > > >fields[field_index].attribute.
> > > > >
> > > > > report_
> > > > > id;
> > > > > - ret = sensor_hub_set_feature(sensor_inst-
> > > > > >hsdev,
> > > > > report_id,
> > > > > - index,
> > > > > sizeof(value),
> > > > > &value);
> > > > > + sensor_hub_set_feature(sensor_inst->hsdev,
> > > > > report_id,
> > > > > index,
> > > > > + sizeof(value), &value);
> > > > > } else
> > > > > return -EINVAL;
> > > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-12 22:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 7:15 [PATCH] HID: hid-sensor-custom: remove useless variable Jiapeng Chong
2021-04-09 18:19 ` Srinivas Pandruvada
2021-04-11 13:56 ` Jonathan Cameron
2021-04-11 16:06 ` Srinivas Pandruvada
2021-04-12 10:20 ` Jonathan Cameron
2021-04-12 22:27 ` srinivas pandruvada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).