* [PATCH] thermal: ti-soc-thermal: remove redundant assignment to variable i
@ 2020-04-15 22:40 ` Colin King
0 siblings, 0 replies; 4+ messages in thread
From: Colin King @ 2020-04-15 22:40 UTC (permalink / raw)
To: Eduardo Valentin, Keerthy, Zhang Rui, Daniel Lezcano,
Amit Kucheria, linux-pm, linux-omap
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The variable i is being assigned with a value that is never read,
the assignment is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index ab19ceff6e2a..abaf629038c3 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -1003,7 +1003,6 @@ int ti_bandgap_probe(struct platform_device *pdev)
ret = ti_bandgap_talert_init(bgp, pdev);
if (ret) {
dev_err(&pdev->dev, "failed to initialize Talert IRQ\n");
- i = bgp->conf->sensor_count;
goto disable_clk;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] thermal: ti-soc-thermal: remove redundant assignment to variable i @ 2020-04-15 22:40 ` Colin King 0 siblings, 0 replies; 4+ messages in thread From: Colin King @ 2020-04-15 22:40 UTC (permalink / raw) To: Eduardo Valentin, Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-pm, linux-omap Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> The variable i is being assigned with a value that is never read, the assignment is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King <colin.king@canonical.com> --- drivers/thermal/ti-soc-thermal/ti-bandgap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c index ab19ceff6e2a..abaf629038c3 100644 --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c @@ -1003,7 +1003,6 @@ int ti_bandgap_probe(struct platform_device *pdev) ret = ti_bandgap_talert_init(bgp, pdev); if (ret) { dev_err(&pdev->dev, "failed to initialize Talert IRQ\n"); - i = bgp->conf->sensor_count; goto disable_clk; } } -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] thermal: ti-soc-thermal: remove redundant assignment to variable i 2020-04-15 22:40 ` Colin King @ 2020-04-16 9:32 ` Dan Carpenter -1 siblings, 0 replies; 4+ messages in thread From: Dan Carpenter @ 2020-04-16 9:32 UTC (permalink / raw) To: Colin King Cc: Eduardo Valentin, Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-pm, linux-omap, kernel-janitors, linux-kernel On Wed, Apr 15, 2020 at 11:40:10PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable i is being assigned with a value that is never read, > the assignment is redundant and can be removed. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/thermal/ti-soc-thermal/ti-bandgap.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > index ab19ceff6e2a..abaf629038c3 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > @@ -1003,7 +1003,6 @@ int ti_bandgap_probe(struct platform_device *pdev) > ret = ti_bandgap_talert_init(bgp, pdev); > if (ret) { > dev_err(&pdev->dev, "failed to initialize Talert IRQ\n"); > - i = bgp->conf->sensor_count; > goto disable_clk; > } > } This isn't the right fix. The goto is wrong. The "i" variable comes from the iterator of the previous loop. When you're unwinding inside a loop then first undo the partial iteration before doing a goto. 979 /* Every thing is good? Then expose the sensors */ 980 for (i = 0; i < bgp->conf->sensor_count; i++) { 981 char *domain; 982 983 if (bgp->conf->sensors[i].register_cooling) { 984 ret = bgp->conf->sensors[i].register_cooling(bgp, i); 985 if (ret) 986 goto remove_sensors; 987 } 988 989 if (bgp->conf->expose_sensor) { 990 domain = bgp->conf->sensors[i].domain; 991 ret = bgp->conf->expose_sensor(bgp, i, domain); 992 if (ret) 993 goto remove_last_cooling; So here we should do: if (ret) { if (bgp->conf->sensors[i].unregister_cooling) bgp->conf->sensors[i].unregister_cooling(bgp, i); goto remove_sensors; } The line lengths are long so it would be cleaner to say: struct ti_temp_sensor *sensor = &bgp->conf->sensors[i]; at the start of the loop. if (ret) { if (sensor->unregister_cooling) sensor->unregister_cooling(bgp, i); goto remove_sensors; } 994 } 995 } 996 997 /* 998 * Enable the Interrupts once everything is set. Otherwise irq handler 999 * might be called as soon as it is enabled where as rest of framework 1000 * is still getting initialised. 1001 */ 1002 if (TI_BANDGAP_HAS(bgp, TALERT)) { 1003 ret = ti_bandgap_talert_init(bgp, pdev); 1004 if (ret) { 1005 dev_err(&pdev->dev, "failed to initialize Talert IRQ\n"); 1006 i = bgp->conf->sensor_count; 1007 goto disable_clk; This should be "goto remove_sensors;" as well. The i assignment can be deleted though because it already equals bgp->conf->sensor_count. 1008 } 1009 } 1010 1011 return 0; 1012 1013 remove_last_cooling: 1014 if (bgp->conf->sensors[i].unregister_cooling) 1015 bgp->conf->sensors[i].unregister_cooling(bgp, i); Delete this partial unwind because we moved it before the goto. Say we add some more error conditions at the end of the function, then now we can add more labels and it's not complicated with this partial unwind. 1016 remove_sensors: 1017 for (i--; i >= 0; i--) { It's slightly nicer to write: "while (--i >= 0) {" 1018 if (bgp->conf->sensors[i].unregister_cooling) 1019 bgp->conf->sensors[i].unregister_cooling(bgp, i); 1020 if (bgp->conf->remove_sensor) 1021 bgp->conf->remove_sensor(bgp, i); 1022 } regards, dan carpenter ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] thermal: ti-soc-thermal: remove redundant assignment to variable i @ 2020-04-16 9:32 ` Dan Carpenter 0 siblings, 0 replies; 4+ messages in thread From: Dan Carpenter @ 2020-04-16 9:32 UTC (permalink / raw) To: Colin King Cc: Eduardo Valentin, Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-pm, linux-omap, kernel-janitors, linux-kernel On Wed, Apr 15, 2020 at 11:40:10PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable i is being assigned with a value that is never read, > the assignment is redundant and can be removed. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/thermal/ti-soc-thermal/ti-bandgap.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > index ab19ceff6e2a..abaf629038c3 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > @@ -1003,7 +1003,6 @@ int ti_bandgap_probe(struct platform_device *pdev) > ret = ti_bandgap_talert_init(bgp, pdev); > if (ret) { > dev_err(&pdev->dev, "failed to initialize Talert IRQ\n"); > - i = bgp->conf->sensor_count; > goto disable_clk; > } > } This isn't the right fix. The goto is wrong. The "i" variable comes from the iterator of the previous loop. When you're unwinding inside a loop then first undo the partial iteration before doing a goto. 979 /* Every thing is good? Then expose the sensors */ 980 for (i = 0; i < bgp->conf->sensor_count; i++) { 981 char *domain; 982 983 if (bgp->conf->sensors[i].register_cooling) { 984 ret = bgp->conf->sensors[i].register_cooling(bgp, i); 985 if (ret) 986 goto remove_sensors; 987 } 988 989 if (bgp->conf->expose_sensor) { 990 domain = bgp->conf->sensors[i].domain; 991 ret = bgp->conf->expose_sensor(bgp, i, domain); 992 if (ret) 993 goto remove_last_cooling; So here we should do: if (ret) { if (bgp->conf->sensors[i].unregister_cooling) bgp->conf->sensors[i].unregister_cooling(bgp, i); goto remove_sensors; } The line lengths are long so it would be cleaner to say: struct ti_temp_sensor *sensor = &bgp->conf->sensors[i]; at the start of the loop. if (ret) { if (sensor->unregister_cooling) sensor->unregister_cooling(bgp, i); goto remove_sensors; } 994 } 995 } 996 997 /* 998 * Enable the Interrupts once everything is set. Otherwise irq handler 999 * might be called as soon as it is enabled where as rest of framework 1000 * is still getting initialised. 1001 */ 1002 if (TI_BANDGAP_HAS(bgp, TALERT)) { 1003 ret = ti_bandgap_talert_init(bgp, pdev); 1004 if (ret) { 1005 dev_err(&pdev->dev, "failed to initialize Talert IRQ\n"); 1006 i = bgp->conf->sensor_count; 1007 goto disable_clk; This should be "goto remove_sensors;" as well. The i assignment can be deleted though because it already equals bgp->conf->sensor_count. 1008 } 1009 } 1010 1011 return 0; 1012 1013 remove_last_cooling: 1014 if (bgp->conf->sensors[i].unregister_cooling) 1015 bgp->conf->sensors[i].unregister_cooling(bgp, i); Delete this partial unwind because we moved it before the goto. Say we add some more error conditions at the end of the function, then now we can add more labels and it's not complicated with this partial unwind. 1016 remove_sensors: 1017 for (i--; i >= 0; i--) { It's slightly nicer to write: "while (--i >= 0) {" 1018 if (bgp->conf->sensors[i].unregister_cooling) 1019 bgp->conf->sensors[i].unregister_cooling(bgp, i); 1020 if (bgp->conf->remove_sensor) 1021 bgp->conf->remove_sensor(bgp, i); 1022 } regards, dan carpenter ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-16 9:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-15 22:40 [PATCH] thermal: ti-soc-thermal: remove redundant assignment to variable i Colin King 2020-04-15 22:40 ` Colin King 2020-04-16 9:32 ` Dan Carpenter 2020-04-16 9:32 ` Dan Carpenter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.