| Message ID | YoetjwcOEzYEFp9b@kili |
|---|---|
| State | Accepted |
| Commit | cc67ca28cf8b29245b71e01117927ed2793f35b7 |
| Headers | show |
| Series | thermal: k3_j72xx_bandgap: Fix array underflow in prep_lookup_table() | expand |
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; > } > > /*
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
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 --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; } /*
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(-)