Messages in this thread | | | Subject | Re: [PATCH] can: Use common error handling code in vxcan_newlink() | From | SF Markus Elfring <> | Date | Sat, 28 Oct 2017 10:23:02 +0200 |
| |
>> @@ -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
| |