Tue, Nov 01, 2022 at 05:25:42PM CET, kuba@xxxxxxxxxx wrote:
>On Mon, 31 Oct 2022 13:42:40 +0100 Jiri Pirko wrote:
>> +/*
>> + * Driver should use this to assign devlink port instance to a netdevice
>> + * before it registers the netdevice. Therefore devlink_port is static
>> + * during the netdev lifetime after it is registered.
>> + */
>> +#define SET_NETDEV_DEVLINK_PORT(dev, _devlink_port) \
>> +({ \
>> + WARN_ON(dev->reg_state != NETREG_UNINITIALIZED); \
>> + ((dev)->devlink_port = (_devlink_port)); \
>> +})
>
>The argument wrapping is inconsistent here - dev is in brackets
>on the second line but not on the first. _devlink_port is prefixed
>with underscore and dev is not. Let's make this properly secure
>and define a local var for dev.
"_" is there because the struct field is named the same. I will change
this to "port" if you don't like "_", no problem.
I will put the first dev to (). Not sure what you mean by "local var",
but I believe that "(dev)" is secure.
>
>> @@ -10107,6 +10107,7 @@ int register_netdevice(struct net_device *dev)
>> return ret;
>>
>> err_uninit:
>> + call_netdevice_notifiers(NETDEV_PRE_UNINIT, dev);
>
>I think we should make this symmetric with POST_INIT.
>IOW PRE_UNINIT should only be called if POST_INIT was called.
