[PATCH] thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[PATCH] thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[Dan Carpenter]

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(-)

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;
 	}
 
 	/*
-- 
2.35.1

Re: [PATCH] thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[Daniel Lezcano]

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;
>   	}
>   
>   	/*


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[Dan Carpenter]

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

Re: [PATCH] thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[Dan Carpenter]

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

[thermal: thermal/next] thermal/drivers/k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

[thermal-bot for Dan Carpenter]

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     cc67ca28cf8b29245b71e01117927ed2793f35b7
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//cc67ca28cf8b29245b71e01117927ed2793f35b7
Author:        Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate:    Fri, 20 May 2022 18:02:39 +03:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Thu, 28 Jul 2022 17:29:48 +02:00

thermal/drivers/k3_j72xx_bandgap: Fix array underflow in prep_lookup_table()

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>
Link: https://lore.kernel.org/r/YoetjwcOEzYEFp9b@kili
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 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 3a35aa3..27d4cae 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;
 	}
 
 	/*