Messages in this thread | | | From | Haiyang Zhang <> | Subject | RE: [PATCH net-next, 2/4] hv_netvsc: Fix error handling in netvsc_attach() | Date | Mon, 4 Nov 2019 15:08:43 +0000 |
| |
> -----Original Message----- > From: Markus Elfring <Markus.Elfring@web.de> > Sent: Friday, November 1, 2019 4:43 PM > To: Haiyang Zhang <haiyangz@microsoft.com>; linux- > hyperv@vger.kernel.org; netdev@vger.kernel.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org; David S. > Miller <davem@davemloft.net>; KY Srinivasan <kys@microsoft.com>; Olaf > Hering <olaf@aepfle.de>; Sasha Levin <sashal@kernel.org>; Stephen > Hemminger <sthemmin@microsoft.com>; vkuznets <vkuznets@redhat.com> > 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
| |