diff mbox series

thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

Message ID YoetjwcOEzYEFp9b@kili
State New
Headers show
Series thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table() | expand

Commit Message

Dan Carpenter May 20, 2022, 3:02 p.m. UTC
This while loop exits with "i" set to -1 and so then it sets:

	derived_table[-1] = derived_table[0] - 300;

There is no need for this assignment at all.  Just delete it.

Fixes: 72b3fc61c752 ("thermal: k3_j72xx_bandgap: Add the bandgap driver support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/thermal/k3_j72xx_bandgap.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Daniel Lezcano May 20, 2022, 3:25 p.m. UTC | #1
On 20/05/2022 17:02, Dan Carpenter wrote:
> This while loop exits with "i" set to -1 and so then it sets:

Won't it exit with 'i' set to '0' ?


> 	derived_table[-1] = derived_table[0] - 300;
> 
> There is no need for this assignment at all.  Just delete it.
> 
> Fixes: 72b3fc61c752 ("thermal: k3_j72xx_bandgap: Add the bandgap driver support")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>   drivers/thermal/k3_j72xx_bandgap.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/thermal/k3_j72xx_bandgap.c b/drivers/thermal/k3_j72xx_bandgap.c
> index 64e323158952..a9789b17513b 100644
> --- a/drivers/thermal/k3_j72xx_bandgap.c
> +++ b/drivers/thermal/k3_j72xx_bandgap.c
> @@ -151,8 +151,6 @@ static int prep_lookup_table(struct err_values *err_vals, int *ref_table)
>   		/* 300 milli celsius steps */
>   		while (i--)
>   			derived_table[i] = derived_table[i + 1] - 300;
> -		/* case 0 */
> -		derived_table[i] = derived_table[i + 1] - 300;
>   	}
>   
>   	/*
Dan Carpenter May 21, 2022, 6:56 a.m. UTC | #2
On Fri, May 20, 2022 at 05:25:56PM +0200, Daniel Lezcano wrote:
> On 20/05/2022 17:02, Dan Carpenter wrote:
> > This while loop exits with "i" set to -1 and so then it sets:
> 
> Won't it exit with 'i' set to '0' ?
> 

Wow.  You made me worried there.  I had to make a test case just to be
sure:

        int i = 10;

        while (i--)
                printf("in %d\n", i);
        printf("out %d\n", i);

Yep.  Ends on on -1.

regards,
dan carpenter
Dan Carpenter May 21, 2022, 7:04 a.m. UTC | #3
On Sat, May 21, 2022 at 09:56:16AM +0300, Dan Carpenter wrote:
> On Fri, May 20, 2022 at 05:25:56PM +0200, Daniel Lezcano wrote:
> > On 20/05/2022 17:02, Dan Carpenter wrote:
> > > This while loop exits with "i" set to -1 and so then it sets:
> > 
> > Won't it exit with 'i' set to '0' ?
> > 
> 
> Wow.  You made me worried there.  I had to make a test case just to be
> sure:
> 
>         int i = 10;
> 
>         while (i--)
>                 printf("in %d\n", i);
>         printf("out %d\n", i);
> 
> Yep.  Ends on on -1.

I wrote a blog about this a few days back.

https://staticthinking.wordpress.com/2022/05/17/i-or-i/

I really think the most readable way is to say:

	while (--i >= 0)
		derived_table[i] = derived_table[i + 1] - 300;

Some people like while (i--) because it works on unsigned variables but
that doesn't apply here and "unsigned int i;" is dumb.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/thermal/k3_j72xx_bandgap.c b/drivers/thermal/k3_j72xx_bandgap.c
index 64e323158952..a9789b17513b 100644
--- a/drivers/thermal/k3_j72xx_bandgap.c
+++ b/drivers/thermal/k3_j72xx_bandgap.c
@@ -151,8 +151,6 @@  static int prep_lookup_table(struct err_values *err_vals, int *ref_table)
 		/* 300 milli celsius steps */
 		while (i--)
 			derived_table[i] = derived_table[i + 1] - 300;
-		/* case 0 */
-		derived_table[i] = derived_table[i + 1] - 300;
 	}
 
 	/*