* [PATCH] iio: imu: st_lsm6dsx: check st_lsm6dsx_shub_read_output return
@ 2020-08-09 17:55 trix
2020-08-10 8:08 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: trix @ 2020-08-09 17:55 UTC (permalink / raw)
To: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
clang static analysis reports this represenative problem
st_lsm6dsx_shub.c:540:8: warning: Assigned value is garbage or undefined
*val = (s16)le16_to_cpu(*((__le16 *)data));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
data is set with
err = st_lsm6dsx_shub_read(sensor, ch->address, data, len);
if (err < 0)
return err;
The problem with st_lsm6dsx_shub_read() is this statement
err = st_lsm6dsx_shub_read_output(hw, data,
len & ST_LS6DSX_READ_OP_MASK);
The err value is never checked.
So check err.
Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index ed83471dc7dd..8c8d8870ca07 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -313,6 +313,8 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr,
err = st_lsm6dsx_shub_read_output(hw, data,
len & ST_LS6DSX_READ_OP_MASK);
+ if (err < 0)
+ return err;
st_lsm6dsx_shub_master_enable(sensor, false);
--
2.18.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: check st_lsm6dsx_shub_read_output return
2020-08-09 17:55 [PATCH] iio: imu: st_lsm6dsx: check st_lsm6dsx_shub_read_output return trix
@ 2020-08-10 8:08 ` Andy Shevchenko
2020-09-19 15:31 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2020-08-10 8:08 UTC (permalink / raw)
To: trix
Cc: Lorenzo Bianconi, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, linux-iio,
Linux Kernel Mailing List
On Sun, Aug 9, 2020 at 8:56 PM <trix@redhat.com> wrote:
>
> From: Tom Rix <trix@redhat.com>
>
> clang static analysis reports this represenative problem
>
> st_lsm6dsx_shub.c:540:8: warning: Assigned value is garbage or undefined
> *val = (s16)le16_to_cpu(*((__le16 *)data));
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> data is set with
>
> err = st_lsm6dsx_shub_read(sensor, ch->address, data, len);
> if (err < 0)
> return err;
>
> The problem with st_lsm6dsx_shub_read() is this statement
>
> err = st_lsm6dsx_shub_read_output(hw, data,
> len & ST_LS6DSX_READ_OP_MASK);
>
> The err value is never checked.
> So check err.
>
> Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
>
> Signed-off-by: Tom Rix <trix@redhat.com>
You see, the commit message can be divided to three sections
1. Title / very short description
2. Detailed description
3. Tag block
Each of them has some specific rules:
1. One quite short line prefixed by subsystem / driver in the
specified format (usually gathered by reading git log against the
module in question)
2. Should explain why this change is done
3. Should be one tag -- one line, no blank lines in between.
Hope, you will use this in the future.
After addressing that (perhaps Jonathan will do it for you)
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> index ed83471dc7dd..8c8d8870ca07 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> @@ -313,6 +313,8 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr,
>
> err = st_lsm6dsx_shub_read_output(hw, data,
> len & ST_LS6DSX_READ_OP_MASK);
> + if (err < 0)
> + return err;
>
> st_lsm6dsx_shub_master_enable(sensor, false);
>
> --
> 2.18.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: check st_lsm6dsx_shub_read_output return
2020-08-10 8:08 ` Andy Shevchenko
@ 2020-09-19 15:31 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-09-19 15:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: trix, Lorenzo Bianconi, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, linux-iio, Linux Kernel Mailing List
On Mon, 10 Aug 2020 11:08:39 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sun, Aug 9, 2020 at 8:56 PM <trix@redhat.com> wrote:
> >
> > From: Tom Rix <trix@redhat.com>
> >
> > clang static analysis reports this represenative problem
> >
> > st_lsm6dsx_shub.c:540:8: warning: Assigned value is garbage or undefined
> > *val = (s16)le16_to_cpu(*((__le16 *)data));
> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > data is set with
> >
> > err = st_lsm6dsx_shub_read(sensor, ch->address, data, len);
> > if (err < 0)
> > return err;
> >
> > The problem with st_lsm6dsx_shub_read() is this statement
> >
> > err = st_lsm6dsx_shub_read_output(hw, data,
> > len & ST_LS6DSX_READ_OP_MASK);
> >
> > The err value is never checked.
> > So check err.
> >
>
>
> > Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
> >
> > Signed-off-by: Tom Rix <trix@redhat.com>
>
> You see, the commit message can be divided to three sections
>
> 1. Title / very short description
> 2. Detailed description
> 3. Tag block
>
> Each of them has some specific rules:
> 1. One quite short line prefixed by subsystem / driver in the
> specified format (usually gathered by reading git log against the
> module in question)
> 2. Should explain why this change is done
> 3. Should be one tag -- one line, no blank lines in between.
>
> Hope, you will use this in the future.
>
> After addressing that (perhaps Jonathan will do it for you)
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Description adjusted and patch applied. Given timing in cycle
I've queued this for the next merge window rather than trying to
get it in during this cycle.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to see if we missed anything.
thanks,
Jonathan
>
> > ---
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> > index ed83471dc7dd..8c8d8870ca07 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> > @@ -313,6 +313,8 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr,
> >
> > err = st_lsm6dsx_shub_read_output(hw, data,
> > len & ST_LS6DSX_READ_OP_MASK);
> > + if (err < 0)
> > + return err;
> >
> > st_lsm6dsx_shub_master_enable(sensor, false);
> >
> > --
> > 2.18.1
> >
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-19 15:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-09 17:55 [PATCH] iio: imu: st_lsm6dsx: check st_lsm6dsx_shub_read_output return trix
2020-08-10 8:08 ` Andy Shevchenko
2020-09-19 15:31 ` Jonathan Cameron
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).