Re: [PATCH -next] m68k: virt: platform: fix missing platform_device_unregister() on error in virt_platform_init()

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Yang,

On Tue, Jun 28, 2022 at 4:08 AM Yang Yingliang <yangyingliang@xxxxxxxxxx> wrote:
> Add the missing platform_device_unregister() before return
> from virt_platform_init() in the error handling case.
>
> Fixes: 05d51e42df06 ("m68k: Introduce a virtual m68k machine")
> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>

Thanks for your patch!

> --- a/arch/m68k/virt/platform.c
> +++ b/arch/m68k/virt/platform.c
> @@ -35,8 +30,10 @@ static int __init virt_platform_init(void)
>                 DEFINE_RES_MEM(virt_bi_data.rtc.mmio + 0x1000, 0x1000),
>                 DEFINE_RES_IRQ(virt_bi_data.rtc.irq + 1),
>         };
> -       struct platform_device *pdev;
> +       struct platform_device *pdev1, *pdev2;
> +       struct platform_device *pdevs[VIRTIO_BUS_NB];
>         unsigned int i;
> +       int ret = 0;
>
>         if (!MACH_IS_VIRT)
>                 return -ENODEV;
> @@ -44,29 +41,40 @@ static int __init virt_platform_init(void)
>         /* We need this to have DMA'able memory provided to goldfish-tty */
>         min_low_pfn = 0;
>
> -       pdev = platform_device_register_simple("goldfish_tty",
> -                                              PLATFORM_DEVID_NONE,
> -                                              goldfish_tty_res,
> -                                              ARRAY_SIZE(goldfish_tty_res));
> -       if (IS_ERR(pdev))
> -               return PTR_ERR(pdev);
> +       pdev1 = platform_device_register_simple("goldfish_tty",
> +                                               PLATFORM_DEVID_NONE,
> +                                               goldfish_tty_res,
> +                                               ARRAY_SIZE(goldfish_tty_res));
> +       if (IS_ERR(pdev1))
> +               return PTR_ERR(pdev1);
>
> -       pdev = platform_device_register_simple("goldfish_rtc",
> -                                              PLATFORM_DEVID_NONE,
> -                                              goldfish_rtc_res,
> -                                              ARRAY_SIZE(goldfish_rtc_res));
> -       if (IS_ERR(pdev))
> -               return PTR_ERR(pdev);
> +       pdev2 = platform_device_register_simple("goldfish_rtc",
> +                                               PLATFORM_DEVID_NONE,
> +                                               goldfish_rtc_res,
> +                                               ARRAY_SIZE(goldfish_rtc_res));
> +       if (IS_ERR(pdev2)) {
> +               ret = PTR_ERR(pdev2);
> +               goto err_unregister_tty;
> +       }
>
>         for (i = 0; i < VIRTIO_BUS_NB; i++) {
> -               int err;
> -
> -               err = virt_virtio_init(i);
> -               if (err)
> -                       return err;
> +               pdevs[i] = virt_virtio_init(i);
> +               if (IS_ERR(pdevs[i])) {
> +                       ret = PTR_ERR(pdevs[i]);
> +                       goto err_unregister_rtc_virtio;
> +               }
>         }
>
>         return 0;
> +
> +err_unregister_rtc_virtio:
> +       for (--i; (int)i >= 0; i--)
> +               platform_device_unregister(pdevs[i]);

I think you can easily avoid the cast ("casts are evil"):

    while (i > 0)
            platform_device_unregister(pdevs[--i]);

> +       platform_device_unregister(pdev2);
> +err_unregister_tty:
> +       platform_device_unregister(pdev1);
> +
> +       return ret;
>  }
>
>  arch_initcall(virt_platform_init);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds