> -----Original Message----- > From: Markus Elfring <Markus.Elfring@xxxxxx> > Sent: Friday, November 1, 2019 4:43 PM > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; linux- > hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx > Cc: kernel-janitors@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; David S. > Miller <davem@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>; Olaf > Hering <olaf@xxxxxxxxx>; Sasha Levin <sashal@xxxxxxxxxx>; Stephen > Hemminger <sthemmin@xxxxxxxxxxxxx>; vkuznets <vkuznets@xxxxxxxxxx> > Subject: Re: [PATCH net-next, 2/4] hv_netvsc: Fix error handling in > netvsc_attach() > > > If rndis_filter_open() fails, we need to remove the rndis device > > created in earlier steps, before returning an error code. Otherwise, > > the retry of > > netvsc_attach() from its callers will fail and hang. > > How do you think about to choose a more “imperative mood” for your > change description? > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k > ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git% > 2Ftree%2FDocumentation%2Fprocess%2Fsubmitting- > patches.rst%3Fid%3D0dbe6cb8f7e05bc9611602ef45980a6c57b245a3%23n151 > &data=02%7C01%7Chaiyangz%40microsoft.com%7C162aa016f45e4293c > abb08d75f0c0fee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637 > 082377796295159&sdata=ytjxGYTPI2D4BoNbslKPvBbsfGUEM7hXj1YAiG > hn8Ik%3D&reserved=0 Agreed. Thanks. > > > … > > +++ b/drivers/net/hyperv/netvsc_drv.c > > @@ -982,7 +982,7 @@ static int netvsc_attach(struct net_device *ndev, > > if (netif_running(ndev)) { > > ret = rndis_filter_open(nvdev); > > if (ret) > > - return ret; > > + goto err; > > > > rdev = nvdev->extension; > > if (!rdev->link_state) > … > > I would prefer to specify the completed exception handling (addition of two > function calls) by a compound statement in the shown if branch directly. > > If you would insist to use a goto statement, I find an other label more > appropriate according to the Linux coding style. I will have more patches that make multiple entry points of error clean up steps -- so I used goto instead of putting the functions in each if-branch. I will name the labels more meaningfully. Thanks, - Haiyang
