diff mbox series

iio: accel: bma400: Fix smatch warning based on use of unintialized value.

Message ID 20220917131401.2815486-1-jic23@kernel.org (mailing list archive)
State Accepted
Headers show
Series iio: accel: bma400: Fix smatch warning based on use of unintialized value. | expand

Commit Message

Jonathan Cameron Sept. 17, 2022, 1:14 p.m. UTC 
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Only specific bits in this value are ever used and those are initialized,
but that is complex to reason about in a checker. Hence, initialize
the value to zero and avoid the complexity.

Smatch warning:
drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
error: uninitialized symbol 'field_value'.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Jagath Jog J <jagathjog1996@gmail.com>
Cc: Alexander Potapenko <glider@google.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/accel/bma400_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jagath Jog J Sept. 18, 2022, 2:35 p.m. UTC | #1 
Hi Jonathan,

Thank you for sending this patch.
If you need a tag for this fix.

Fixes: 961db2da159d ("iio: accel: bma400: Add support for single and
double tap events")

Thank you
Jagath

On Sat, Sep 17, 2022 at 6:44 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Only specific bits in this value are ever used and those are initialized,
> but that is complex to reason about in a checker. Hence, initialize
> the value to zero and avoid the complexity.
>
> Smatch warning:
> drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
> error: uninitialized symbol 'field_value'.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Jagath Jog J <jagathjog1996@gmail.com>
> Cc: Alexander Potapenko <glider@google.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/accel/bma400_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index eceb1f8d338d..ad8fce3e08cd 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -1184,7 +1184,8 @@ static int bma400_activity_event_en(struct bma400_data *data,
>                                     enum iio_event_direction dir,
>                                     int state)
>  {
> -       int ret, reg, msk, value, field_value;
> +       int ret, reg, msk, value;
> +       int field_value = 0;
>
>         switch (dir) {
>         case IIO_EV_DIR_RISING:
> --
> 2.37.2
>
Jonathan Cameron Sept. 18, 2022, 5:45 p.m. UTC | #2 
On Sun, 18 Sep 2022 20:05:48 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Hi Jonathan,
> 
> Thank you for sending this patch.
> If you need a tag for this fix.
> 
> Fixes: 961db2da159d ("iio: accel: bma400: Add support for single and
> double tap events")
Good point. I'm careful about fixes tags to stuff still not upstream
because they tend to be a little unstable. This should be fine.
I'll see if it's valid when I rebase the tree (hopefully in a few days
time).

Also helpful if you give an Acked-by!

Thanks,

Jonathan

> 
> Thank you
> Jagath
> 
> On Sat, Sep 17, 2022 at 6:44 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Only specific bits in this value are ever used and those are initialized,
> > but that is complex to reason about in a checker. Hence, initialize
> > the value to zero and avoid the complexity.
> >
> > Smatch warning:
> > drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
> > error: uninitialized symbol 'field_value'.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: Jagath Jog J <jagathjog1996@gmail.com>
> > Cc: Alexander Potapenko <glider@google.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/accel/bma400_core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index eceb1f8d338d..ad8fce3e08cd 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
> > @@ -1184,7 +1184,8 @@ static int bma400_activity_event_en(struct bma400_data *data,
> >                                     enum iio_event_direction dir,
> >                                     int state)
> >  {
> > -       int ret, reg, msk, value, field_value;
> > +       int ret, reg, msk, value;
> > +       int field_value = 0;
> >
> >         switch (dir) {
> >         case IIO_EV_DIR_RISING:
> > --
> > 2.37.2
> >
Jagath Jog J Sept. 18, 2022, 6:20 p.m. UTC | #3 
On Sat, Sep 17, 2022 at 6:44 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Only specific bits in this value are ever used and those are initialized,
> but that is complex to reason about in a checker. Hence, initialize
> the value to zero and avoid the complexity.
>
> Smatch warning:
> drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
> error: uninitialized symbol 'field_value'.

Acked-by: Jagath Jog J <jagathjog1996@gmail.com>

>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Jagath Jog J <jagathjog1996@gmail.com>
> Cc: Alexander Potapenko <glider@google.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/accel/bma400_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index eceb1f8d338d..ad8fce3e08cd 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -1184,7 +1184,8 @@ static int bma400_activity_event_en(struct bma400_data *data,
>                                     enum iio_event_direction dir,
>                                     int state)
>  {
> -       int ret, reg, msk, value, field_value;
> +       int ret, reg, msk, value;
> +       int field_value = 0;
>
>         switch (dir) {
>         case IIO_EV_DIR_RISING:
> --
> 2.37.2
>
Dan Carpenter Sept. 19, 2022, 6:10 a.m. UTC | #4 
On Sun, Sep 18, 2022 at 06:45:03PM +0100, Jonathan Cameron wrote:
> On Sun, 18 Sep 2022 20:05:48 +0530
> Jagath Jog J <jagathjog1996@gmail.com> wrote:
> 
> > Hi Jonathan,
> > 
> > Thank you for sending this patch.
> > If you need a tag for this fix.
> > 
> > Fixes: 961db2da159d ("iio: accel: bma400: Add support for single and
> > double tap events")
> Good point. I'm careful about fixes tags to stuff still not upstream
> because they tend to be a little unstable. This should be fine.
> I'll see if it's valid when I rebase the tree (hopefully in a few days
> time).

If you forget to update the tags after a rebase then Stephen Rothwell
will let you know.  It's best to not make Stephen's job more difficult,
but it's some comfort to know that mistakes will get caught.

regards,
dan carpenter
Jonathan Cameron Sept. 19, 2022, 3:41 p.m. UTC | #5 
On Mon, 19 Sep 2022 09:10:52 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Sun, Sep 18, 2022 at 06:45:03PM +0100, Jonathan Cameron wrote:
> > On Sun, 18 Sep 2022 20:05:48 +0530
> > Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >   
> > > Hi Jonathan,
> > > 
> > > Thank you for sending this patch.
> > > If you need a tag for this fix.
> > > 
> > > Fixes: 961db2da159d ("iio: accel: bma400: Add support for single and
> > > double tap events")  
> > Good point. I'm careful about fixes tags to stuff still not upstream
> > because they tend to be a little unstable. This should be fine.
> > I'll see if it's valid when I rebase the tree (hopefully in a few days
> > time).  
> 
> If you forget to update the tags after a rebase then Stephen Rothwell
> will let you know.  It's best to not make Stephen's job more difficult,
> but it's some comfort to know that mistakes will get caught.
> 
> regards,
> dan carpenter
> 
Hi Dan,

Having messed this up and caused Stephen pointless work several times I now
have scripting (thanks to Greg KH's scripts and some local modes to make
sure the tag is in the upstream of the fix) to check this but some how
I still mess it up from time to time :(

Jonathan
Jonathan Cameron Sept. 19, 2022, 4:06 p.m. UTC | #6 
On Sun, 18 Sep 2022 23:50:44 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> On Sat, Sep 17, 2022 at 6:44 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Only specific bits in this value are ever used and those are initialized,
> > but that is complex to reason about in a checker. Hence, initialize
> > the value to zero and avoid the complexity.
> >
> > Smatch warning:
> > drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
> > error: uninitialized symbol 'field_value'.  
> 
> Acked-by: Jagath Jog J <jagathjog1996@gmail.com>

Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.

Thanks,

Jonathan
> 
> >
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: Jagath Jog J <jagathjog1996@gmail.com>
> > Cc: Alexander Potapenko <glider@google.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/accel/bma400_core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index eceb1f8d338d..ad8fce3e08cd 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
> > @@ -1184,7 +1184,8 @@ static int bma400_activity_event_en(struct bma400_data *data,
> >                                     enum iio_event_direction dir,
> >                                     int state)
> >  {
> > -       int ret, reg, msk, value, field_value;
> > +       int ret, reg, msk, value;
> > +       int field_value = 0;
> >
> >         switch (dir) {
> >         case IIO_EV_DIR_RISING:
> > --
> > 2.37.2
> >
diff mbox series

Patch

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index eceb1f8d338d..ad8fce3e08cd 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -1184,7 +1184,8 @@  static int bma400_activity_event_en(struct bma400_data *data,
 				    enum iio_event_direction dir,
 				    int state)
 {
-	int ret, reg, msk, value, field_value;
+	int ret, reg, msk, value;
+	int field_value = 0;
 
 	switch (dir) {
 	case IIO_EV_DIR_RISING: