linux-kernel - Re: [PATCH -next 2/2] xtensa: iss: change the return type of iss_net

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMo8BfJvXaDnsXrA1ELKJz1QZmW+8NcQpkhkjKbkp1zxMTePCg@mail.gmail.com>
Date:   Tue, 5 Jul 2022 08:59:43 -0700
From:   Max Filippov <jcmvbkbc@...il.com>
To:     Yang Yingliang <yangyingliang@...wei.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "open list:TENSILICA XTENSA PORT (xtensa)" 
        <linux-xtensa@...ux-xtensa.org>, Chris Zankel <chris@...kel.net>
Subject: Re: [PATCH -next 2/2] xtensa: iss: change the return type of
 iss_net_configure() to void

Hi Yang,

On Tue, Jul 5, 2022 at 6:10 AM Yang Yingliang <yangyingliang@...wei.com> wrote:
>
> Change the return type of iss_net_configure() to void, because it's not used.
>
> Signed-off-by: Yang Yingliang <yangyingliang@...wei.com>
> ---
>  arch/xtensa/platforms/iss/network.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)

This change removes a lot of code that was added in the previous change.
Maybe fold both patches into one to avoid that pattern of adding and then
immediately removing code? Or, if you feel that changing the return type
of the iss_net_configure is an important separate step, maybe do it first?

> diff --git a/arch/xtensa/platforms/iss/network.c b/arch/xtensa/platforms/iss/network.c
> index 5e475f7472e4..e38ff51ce833 100644
> --- a/arch/xtensa/platforms/iss/network.c
> +++ b/arch/xtensa/platforms/iss/network.c
> @@ -472,16 +472,15 @@ static const struct net_device_ops iss_netdev_ops = {
>         .ndo_set_rx_mode        = iss_net_set_multicast_list,
>  };
>
> -static int iss_net_configure(int index, char *init)
> +static void iss_net_configure(int index, char *init)
>  {
>         struct net_device *dev;
>         struct iss_net_private *lp;
> -       int err;
>
>         dev = alloc_etherdev(sizeof(*lp));
>         if (dev == NULL) {
>                 pr_err("eth_configure: failed to allocate device\n");
> -               return -ENOMEM;
> +               return;
>         }
>
>         /* Initialize private element. */
> @@ -509,7 +508,6 @@ static int iss_net_configure(int index, char *init)
>         if (!tuntap_probe(lp, index, init)) {
>                 pr_err("%s: invalid arguments. Skipping device!\n",
>                        dev->name);
> -               err = -EINVAL;
>                 goto err_free_netdev;
>         }
>
> @@ -518,8 +516,7 @@ static int iss_net_configure(int index, char *init)
>         /* sysfs register */
>
>         if (!driver_registered) {
> -               err = platform_driver_register(&iss_net_driver);
> -               if (err)
> +               if (platform_driver_register(&iss_net_driver))
>                         goto err_free_netdev;
>                 driver_registered = 1;
>         }
> @@ -530,8 +527,7 @@ static int iss_net_configure(int index, char *init)
>
>         lp->pdev.id = index;
>         lp->pdev.name = DRIVER_NAME;
> -       err = platform_device_register(&lp->pdev);
> -       if (err)
> +       if (platform_device_register(&lp->pdev))
>                 goto err_free_netdev;
>         SET_NETDEV_DEV(dev, &lp->pdev.dev);
>
> @@ -541,23 +537,22 @@ static int iss_net_configure(int index, char *init)
>         dev->irq = -1;
>
>         rtnl_lock();
> -       err = register_netdevice(dev);
> -       rtnl_unlock();
> -
> -       if (err) {
> +       if (register_netdevice(dev)) {
> +               rtnl_unlock();
>                 pr_err("%s: error registering net device!\n", dev->name);
>                 goto err_unregister_device;
>         }
> +       rtnl_unlock();
>
>         timer_setup(&lp->tl, iss_net_user_timer_expire, 0);
>
> -       return 0;
> +       return;
>
>  err_unregister_device:
>         platform_device_unregister(&lp->pdev);
>  err_free_netdev:
>         free_netdev(dev);
> -       return err;
> +       return;

No need for 'return' at the end of the void function.

-- 
Thanks.
-- Max