Re: [Intel-wired-lan] [PATCH iwl-net] idpf: check error for register_netdev() on init

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 2/13/25 21:39, Tantilov, Emil S wrote:
> On 2/12/2025 10:21 AM, Simon Horman wrote:
>> On Mon, Feb 10, 2025 at 06:38:51PM -0800, Emil Tantilov wrote:
>>> Current init logic ignores the error code from register_netdev(),
>>> which will cause WARN_ON() on attempt to unregister it, if there was 
>>> one,
>>> and there is no info for the user that the creation of the netdev 
>>> failed.


>> Hi Emil,
>>
>> I'm wondering if we could reduce indentation and lines longer
>> than 80 characters in the above like this (completely untested!):
> I was mostly trying to focus on the fix itself, since this patch is -net 
> bound. The >80 line came about from the introduction of the local netdev 
> and it seemed cleaner to keep it in one line. I can just split the check 
> as in the original code.
> 
>>
>>
>>     for (index = 0; index < adapter->max_vports; index++) {
>>         struct idpf_vport_config *vport_config = adapter- 
>> >vport_config[index];
>>         struct net_device *netdev = adapter->netdevs[index];
>>
>>         if (!netdev ||
>>             test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags))
>>             continue;
> Again, because its mainly to add the error checking I am not sure if its 
> OK to re-shuffle the logic.
> 
>>
>>         err = register_netdev(netdev);
>>         if (err) {
>>             dev_err(&pdev->dev, "failed to register netdev for vport 
>> %d: %pe\n",
>>                 index, ERR_PTR(err));
>>             continue;
>>         }
>>         set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
>>     }
> 
> Don't mind re-spinning (and testing) v2 with the proposed change, if 
> it's not infringing on the guidelines for submission to -net.
> 
> Thanks,
> Emil

Emil, you are right that we generally don't want to do refactors on -net
submissions. In this particular case your code was just replacing the
body of the loop. Simon's snippet did the same, just with less
indentation.

The "early return" (or "continue") style is better, also in this case.