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] [day] [month] [year] [list]

Message-ID: <87706ab7-29d3-b56b-2912-70c6ad7d28cc@huawei.com>
Date:   Wed, 6 Jul 2022 09:33:32 +0800
From:   Yang Yingliang <yangyingliang@...wei.com>
To:     Max Filippov <jcmvbkbc@...il.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,

On 2022/7/5 23:59, Max Filippov wrote:
> 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?
I will send a v2 with these changes in one patch.

Thanks,
Yang
>
>> 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.
>