Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
From: Haiyang Zhang <haiyangz@microsoft.com>
To: Markus Elfring <Markus.Elfring@web.de>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: "kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>,
	"linux-kernel@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()
Date: Mon, 4 Nov 2019 15:08:43 +0000
Message-ID: <BYAPR21MB13667058A6F6C641EC973327CA7F0@BYAPR21MB1366.namprd21.prod.outlook.com> (raw)
In-Reply-To: <cdf7b308-940a-ff9c-07ae-f42b94687e24@web.de>



> -----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

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 21:06 [PATCH net-next, 0/4] hv_netvsc: Add XDP support and some error handling fixes Haiyang Zhang
2019-10-28 21:07 ` [PATCH net-next, 1/4] hv_netvsc: Fix error handling in netvsc_set_features() Haiyang Zhang
2019-10-28 21:07 ` [PATCH net-next, 2/4] hv_netvsc: Fix error handling in netvsc_attach() Haiyang Zhang
2019-11-01 20:42   ` Markus Elfring
2019-11-04 15:08     ` Haiyang Zhang [this message]
2019-10-28 21:07 ` [PATCH net-next, 3/4] hv_netvsc: Add XDP support Haiyang Zhang
2019-10-28 21:33   ` Jakub Kicinski
2019-10-29 19:17     ` Haiyang Zhang
2019-10-29 19:53       ` Jakub Kicinski
2019-10-29 20:01         ` Haiyang Zhang
2019-10-29 21:59       ` Stephen Hemminger
2019-10-29 22:08         ` Haiyang Zhang
2019-10-28 21:07 ` [PATCH net-next, 4/4] hv_netvsc: Update document for " Haiyang Zhang

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR21MB13667058A6F6C641EC973327CA7F0@BYAPR21MB1366.namprd21.prod.outlook.com \
    --to=haiyangz@microsoft.com \
    --cc=Markus.Elfring@web.de \
    --cc=davem@davemloft.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olaf@aepfle.de \
    --cc=sashal@kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git