* [PATCH] iio: scd4x: check return of scd4x_write_and_fetch
@ 2022-02-27 15:43 trix
2022-02-27 17:48 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: trix @ 2022-02-27 15:43 UTC (permalink / raw)
To: roan, jic23, lars, nathan, ndesaulniers
Cc: linux-iio, linux-kernel, llvm, Tom Rix
From: Tom Rix <trix@redhat.com>
Clang static analysis reports this problem
scd4x.c:474:10: warning: The left operand of '==' is a
garbage value
if (val == 0xff) {
~~~ ^
val is only set from a successful call to scd4x_write_and_fetch()
So check it's return.
Fixes: 49d22b695cbb ("drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor")
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/iio/chemical/scd4x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
index 20d4e7584e923..b978330fb761c 100644
--- a/drivers/iio/chemical/scd4x.c
+++ b/drivers/iio/chemical/scd4x.c
@@ -471,7 +471,7 @@ static ssize_t calibration_forced_value_store(struct device *dev,
ret = scd4x_write_and_fetch(state, CMD_FRC, arg, &val, sizeof(val));
mutex_unlock(&state->lock);
- if (val == 0xff) {
+ if (!ret && val == 0xff) {
dev_err(dev, "forced calibration has failed");
return -EINVAL;
}
--
2.26.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: scd4x: check return of scd4x_write_and_fetch
2022-02-27 15:43 [PATCH] iio: scd4x: check return of scd4x_write_and_fetch trix
@ 2022-02-27 17:48 ` Jonathan Cameron
2022-02-28 21:06 ` Tom Rix
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2022-02-27 17:48 UTC (permalink / raw)
To: trix; +Cc: roan, lars, nathan, ndesaulniers, linux-iio, linux-kernel, llvm
On Sun, 27 Feb 2022 07:43:31 -0800
trix@redhat.com wrote:
> From: Tom Rix <trix@redhat.com>
>
> Clang static analysis reports this problem
> scd4x.c:474:10: warning: The left operand of '==' is a
> garbage value
> if (val == 0xff) {
> ~~~ ^
> val is only set from a successful call to scd4x_write_and_fetch()
> So check it's return.
>
> Fixes: 49d22b695cbb ("drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor")
> Signed-off-by: Tom Rix <trix@redhat.com>
Good find, but I'd prefer a separate check on ret inline with what the
other error checking paths in that function are doing.
> ---
> drivers/iio/chemical/scd4x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index 20d4e7584e923..b978330fb761c 100644
> --- a/drivers/iio/chemical/scd4x.c
> +++ b/drivers/iio/chemical/scd4x.c
> @@ -471,7 +471,7 @@ static ssize_t calibration_forced_value_store(struct device *dev,
> ret = scd4x_write_and_fetch(state, CMD_FRC, arg, &val, sizeof(val));
> mutex_unlock(&state->lock);
>
> - if (val == 0xff) {
> + if (!ret && val == 0xff) {
> dev_err(dev, "forced calibration has failed");
> return -EINVAL;
> }
Prefer
if (ret)
return ret;
if (val == 0xff) {
dev_err(dev, "...
return -EINVAL;
}
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: scd4x: check return of scd4x_write_and_fetch
2022-02-27 17:48 ` Jonathan Cameron
@ 2022-02-28 21:06 ` Tom Rix
0 siblings, 0 replies; 3+ messages in thread
From: Tom Rix @ 2022-02-28 21:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: roan, lars, nathan, ndesaulniers, linux-iio, linux-kernel, llvm
On 2/27/22 9:48 AM, Jonathan Cameron wrote:
> On Sun, 27 Feb 2022 07:43:31 -0800
> trix@redhat.com wrote:
>
>> From: Tom Rix <trix@redhat.com>
>>
>> Clang static analysis reports this problem
>> scd4x.c:474:10: warning: The left operand of '==' is a
>> garbage value
>> if (val == 0xff) {
>> ~~~ ^
>> val is only set from a successful call to scd4x_write_and_fetch()
>> So check it's return.
>>
>> Fixes: 49d22b695cbb ("drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor")
>> Signed-off-by: Tom Rix <trix@redhat.com>
> Good find, but I'd prefer a separate check on ret inline with what the
> other error checking paths in that function are doing.
>
>> ---
>> drivers/iio/chemical/scd4x.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
>> index 20d4e7584e923..b978330fb761c 100644
>> --- a/drivers/iio/chemical/scd4x.c
>> +++ b/drivers/iio/chemical/scd4x.c
>> @@ -471,7 +471,7 @@ static ssize_t calibration_forced_value_store(struct device *dev,
>> ret = scd4x_write_and_fetch(state, CMD_FRC, arg, &val, sizeof(val));
>> mutex_unlock(&state->lock);
>>
>> - if (val == 0xff) {
>> + if (!ret && val == 0xff) {
>> dev_err(dev, "forced calibration has failed");
>> return -EINVAL;
>> }
> Prefer
>
> if (ret)
> return ret;
>
> if (val == 0xff) {
> dev_err(dev, "...
> return -EINVAL;
> }
ok and the next line can be simplified to
return len;
Tom
> Thanks,
>
> Jonathan
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-28 21:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-27 15:43 [PATCH] iio: scd4x: check return of scd4x_write_and_fetch trix
2022-02-27 17:48 ` Jonathan Cameron
2022-02-28 21:06 ` Tom Rix
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.