On Mon, Jan 4, 2021 at 9:49 AM Jakub Kicinski <k...@kernel.org> wrote: > > On Sat, 2 Jan 2021 15:49:54 -0800 Cong Wang wrote: > > On Wed, Dec 30, 2020 at 7:46 PM Jakub Kicinski <k...@kernel.org> wrote: > > > @@ -661,9 +662,14 @@ static int bareudp_newlink(struct net *net, struct > > > net_device *dev, > > > > > > err = bareudp_link_config(dev, tb); > > > if (err) > > > - return err; > > > + goto err_unconfig; > > > > > > return 0; > > > + > > > +err_unconfig: > > > > I think we can save this goto. > > I personally prefer more idiomatic code flow to saving a single LoC. > > > > + list_del(&bareudp->next); > > > + unregister_netdevice(dev); > > > > Which is bareudp_dellink(dev, NULL). ;) > > I know, but calling full dellink when only parts of newlink fails felt > weird. And it's not lower LoC, unless called with NULL as second arg, > which again could be surprising to a person changing dellink.
I think calling a function with "bareudp_" prefix is more readable than interpreting list_del()+unregister_netdevice(). I mean if (bareudp_*()) goto err; ... err: bareudp_*(); this looks cleaner, right? Thanks.
