Re: question about drivers/rtc/rtc-cmos.c

From: Julia Lawall
Date: Fri Dec 27 2013 - 17:33:21 EST




On Fri, 27 Dec 2013, Andrew Morton wrote:

> On Wed, 25 Dec 2013 20:32:12 +0100 (CET) Julia Lawall <julia.lawall@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> 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@xxxxxxx>
> Cc: John Stultz <john.stultz@xxxxxxxxxx>
> Cc: Grant Likely <grant.likely@xxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> 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;
> _
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/