Re: [PATCH] can: Use common error handling code in vxcan_newlink()

[SF Markus Elfring]

>> @@ -227,10 +227,8 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
>> ÂÂÂÂÂ netif_carrier_off(peer);
>> Â ÂÂÂÂÂ err = rtnl_configure_link(peer, ifmp);
>> -ÂÂÂ if (err < 0) {
>> -ÂÂÂÂÂÂÂ unregister_netdevice(peer);
>> -ÂÂÂÂÂÂÂ return err;
>> -ÂÂÂ }
>> +ÂÂÂ if (err)
>> +ÂÂÂÂÂÂÂ goto unregister_network_device;
> 
> You are changing semantic in the if-statement here.

I got an other software development opinion for this implementation detail.

http://elixir.free-electrons.com/linux/v4.14-rc6/source/net/core/rtnetlink.c#L2393
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/net/core/rtnetlink.c?id=36ef71cae353f88fd6e095e2aaa3e5953af1685d#n2513

The success predicate for the function ârtnl_configure_linkâ is that
the return value is zero. I would prefer to treat other values as
an error code then.


> I would be fine with the patch

Thanks for a bit of change acceptance.


> if you revert that if-statement as I would like to stay on the behavior
> from veth.c in veth_newlink().

Will another bit of clarification be useful around the usage of error predicates?

Regards,
Markus