lkml.org 
[lkml]   [2019]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH net-next, 2/4] hv_netvsc: Fix error handling in netvsc_attach()
Date


> -----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
> &amp;data=02%7C01%7Chaiyangz%40microsoft.com%7C162aa016f45e4293c
> abb08d75f0c0fee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637
> 082377796295159&amp;sdata=ytjxGYTPI2D4BoNbslKPvBbsfGUEM7hXj1YAiG
> hn8Ik%3D&amp;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
\
 
 \ /
  Last update: 2019-11-04 16:09    [W:1.253 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site