* question about drivers/rtc/rtc-cmos.c
@ 2013-12-25 19:32 Julia Lawall
2013-12-27 17:29 ` Alessandro Zummo
2013-12-27 22:22 ` Andrew Morton
0 siblings, 2 replies; 5+ messages in thread
From: Julia Lawall @ 2013-12-25 19:32 UTC (permalink / raw)
To: a.zummo, grant.likely, robh+dt, rtc-linux, linux-kernel
The function cmos_do_probe contains the code:
if (is_hpet_enabled()) {
int err;
rtc_cmos_int_handler = hpet_rtc_interrupt;
err = hpet_register_irq_handler(cmos_interrupt);
if (err != 0) {
dev_warn(dev, "hpet_register_irq_handler "
" failed in rtc_init().");
goto cleanup1;
}
}
Is it intentional that the error code returned by
hpet_register_irq_handler is put ina local variable that will not be seen
at label cleanup1? The return value is retval, which is 0 at this point.
thanks,
julia
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: question about drivers/rtc/rtc-cmos.c
2013-12-25 19:32 question about drivers/rtc/rtc-cmos.c Julia Lawall
@ 2013-12-27 17:29 ` Alessandro Zummo
2013-12-27 20:26 ` Julia Lawall
2013-12-27 22:22 ` Andrew Morton
1 sibling, 1 reply; 5+ messages in thread
From: Alessandro Zummo @ 2013-12-27 17:29 UTC (permalink / raw)
To: Julia Lawall; +Cc: grant.likely, robh+dt, rtc-linux, linux-kernel
On Wed, 25 Dec 2013 20:32:12 +0100 (CET)
Julia Lawall <julia.lawall@lip6.fr> wrote:
> Is it intentional that the error code returned by
> hpet_register_irq_handler is put ina local variable that will not be seen
> at label cleanup1? The return value is retval, which is 0 at this point.
Well, I'd say it's an error. Did you got it by hand
or with one of your tools? ;)
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: question about drivers/rtc/rtc-cmos.c
2013-12-27 17:29 ` Alessandro Zummo
@ 2013-12-27 20:26 ` Julia Lawall
0 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2013-12-27 20:26 UTC (permalink / raw)
To: Alessandro Zummo
Cc: Julia Lawall, grant.likely, robh+dt, rtc-linux, linux-kernel
On Fri, 27 Dec 2013, Alessandro Zummo wrote:
> On Wed, 25 Dec 2013 20:32:12 +0100 (CET)
> Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> > Is it intentional that the error code returned by
> > hpet_register_irq_handler is put ina local variable that will not be seen
> > at label cleanup1? The return value is retval, which is 0 at this point.
>
> Well, I'd say it's an error. Did you got it by hand
> or with one of your tools? ;)
Coccinelle. No idea if it an happen in real life.
I will drop err and replace it by retval.
julia
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: question about drivers/rtc/rtc-cmos.c
2013-12-25 19:32 question about drivers/rtc/rtc-cmos.c Julia Lawall
2013-12-27 17:29 ` Alessandro Zummo
@ 2013-12-27 22:22 ` Andrew Morton
2013-12-27 22:32 ` Julia Lawall
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2013-12-27 22:22 UTC (permalink / raw)
To: Julia Lawall; +Cc: a.zummo, grant.likely, robh+dt, rtc-linux, linux-kernel
On Wed, 25 Dec 2013 20:32:12 +0100 (CET) Julia Lawall <julia.lawall@lip6.fr> wrote:
> The function cmos_do_probe contains the code:
>
> if (is_hpet_enabled()) {
> int err;
>
> rtc_cmos_int_handler = hpet_rtc_interrupt;
> err = hpet_register_irq_handler(cmos_interrupt);
> if (err != 0) {
> dev_warn(dev, "hpet_register_irq_handler "
> " failed in rtc_init().");
> goto cleanup1;
> }
> }
>
> Is it intentional that the error code returned by
> hpet_register_irq_handler is put ina local variable that will not be seen
> at label cleanup1? The return value is retval, which is 0 at this point.
No, I'd say that's a bug. How does this look?
From: Andrew Morton <akpm@linux-foundation.org>
Subject: drivers/rtc/rtc-cmos.c: propagate hpet_register_irq_handler() failure
If hpet_register_irq_handler() fails, cmos_do_probe() will incorrectly
return 0.
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/rtc/rtc-cmos.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff -puN drivers/rtc/rtc-cmos.c~drivers-rtc-rtc-cmosc-propagate-hpet_register_irq_handler-failure drivers/rtc/rtc-cmos.c
--- a/drivers/rtc/rtc-cmos.c~drivers-rtc-rtc-cmosc-propagate-hpet_register_irq_handler-failure
+++ a/drivers/rtc/rtc-cmos.c
@@ -708,11 +708,9 @@ cmos_do_probe(struct device *dev, struct
irq_handler_t rtc_cmos_int_handler;
if (is_hpet_enabled()) {
- int err;
-
rtc_cmos_int_handler = hpet_rtc_interrupt;
- err = hpet_register_irq_handler(cmos_interrupt);
- if (err != 0) {
+ retval = hpet_register_irq_handler(cmos_interrupt);
+ if (retval) {
dev_warn(dev, "hpet_register_irq_handler "
" failed in rtc_init().");
goto cleanup1;
_
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: question about drivers/rtc/rtc-cmos.c
2013-12-27 22:22 ` Andrew Morton
@ 2013-12-27 22:32 ` Julia Lawall
0 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2013-12-27 22:32 UTC (permalink / raw)
To: Andrew Morton
Cc: Julia Lawall, a.zummo, grant.likely, robh+dt, rtc-linux,
linux-kernel
On Fri, 27 Dec 2013, Andrew Morton wrote:
> On Wed, 25 Dec 2013 20:32:12 +0100 (CET) Julia Lawall <julia.lawall@lip6.fr> wrote:
>
> > The function cmos_do_probe contains the code:
> >
> > if (is_hpet_enabled()) {
> > int err;
> >
> > rtc_cmos_int_handler = hpet_rtc_interrupt;
> > err = hpet_register_irq_handler(cmos_interrupt);
> > if (err != 0) {
> > dev_warn(dev, "hpet_register_irq_handler "
> > " failed in rtc_init().");
> > goto cleanup1;
> > }
> > }
> >
> > Is it intentional that the error code returned by
> > hpet_register_irq_handler is put ina local variable that will not be seen
> > at label cleanup1? The return value is retval, which is 0 at this point.
>
> No, I'd say that's a bug. How does this look?
That is what I was going to propose.
julia
>
>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: drivers/rtc/rtc-cmos.c: propagate hpet_register_irq_handler() failure
>
> If hpet_register_irq_handler() fails, cmos_do_probe() will incorrectly
> return 0.
>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> drivers/rtc/rtc-cmos.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff -puN drivers/rtc/rtc-cmos.c~drivers-rtc-rtc-cmosc-propagate-hpet_register_irq_handler-failure drivers/rtc/rtc-cmos.c
> --- a/drivers/rtc/rtc-cmos.c~drivers-rtc-rtc-cmosc-propagate-hpet_register_irq_handler-failure
> +++ a/drivers/rtc/rtc-cmos.c
> @@ -708,11 +708,9 @@ cmos_do_probe(struct device *dev, struct
> irq_handler_t rtc_cmos_int_handler;
>
> if (is_hpet_enabled()) {
> - int err;
> -
> rtc_cmos_int_handler = hpet_rtc_interrupt;
> - err = hpet_register_irq_handler(cmos_interrupt);
> - if (err != 0) {
> + retval = hpet_register_irq_handler(cmos_interrupt);
> + if (retval) {
> dev_warn(dev, "hpet_register_irq_handler "
> " failed in rtc_init().");
> goto cleanup1;
> _
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-27 22:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-25 19:32 question about drivers/rtc/rtc-cmos.c Julia Lawall
2013-12-27 17:29 ` Alessandro Zummo
2013-12-27 20:26 ` Julia Lawall
2013-12-27 22:22 ` Andrew Morton
2013-12-27 22:32 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox