All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: KY Srinivasan <kys@microsoft.com>
Cc: Andev <debiandev@gmail.com>, "olaf@aepfle.de" <olaf@aepfle.de>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"apw@canonical.com" <apw@canonical.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH V1 net-next 1/1] hyperv: Enable sendbuf mechanism on the send path
Date: Fri, 25 Apr 2014 01:30:59 +0300	[thread overview]
Message-ID: <20140424223059.GE26890@mwanda> (raw)
In-Reply-To: <cdb70820bcd0467586ce6893753c7065@BY2PR03MB299.namprd03.prod.outlook.com>

On Thu, Apr 24, 2014 at 10:06:24PM +0000, KY Srinivasan wrote:
> > From: Andev [mailto:debiandev@gmail.com]
> > Your use of goto exit/cleanup in some functions and returning directly on
> > errors in others could use a cleanup. Please consider doing that while you are
> > touching those files.
> 
> Will do. The most recent changes I made to netvsc.c, I think was
> consistent with the existing code; going forward we will certainly
> move towards a more consistent coding style.

It scares me when you talk about being consistent with the existing
code...  Just do it the correct way.

1) Don't do the "return ret;" if you know ret is zero.

2) Replace:
	ret = vmbus_sendpacket(...);

	return ret;
   with
  	return vmbus_sendpacket(...);

3) Don't do "goto cleanup;" when "return ret;" will suffice.  The
   do-nothing goto is misleading because you assume it will cleanup
   somthing.  Some people used to misread CodingStyle to think that all
   functions should only have one return but I have updated it so it is
   more clear.

regards,
dan carpenter


WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: KY Srinivasan <kys@microsoft.com>
Cc: "olaf@aepfle.de" <olaf@aepfle.de>, Andev <debiandev@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"apw@canonical.com" <apw@canonical.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH V1 net-next 1/1] hyperv: Enable sendbuf mechanism on the send path
Date: Fri, 25 Apr 2014 01:30:59 +0300	[thread overview]
Message-ID: <20140424223059.GE26890@mwanda> (raw)
In-Reply-To: <cdb70820bcd0467586ce6893753c7065@BY2PR03MB299.namprd03.prod.outlook.com>

On Thu, Apr 24, 2014 at 10:06:24PM +0000, KY Srinivasan wrote:
> > From: Andev [mailto:debiandev@gmail.com]
> > Your use of goto exit/cleanup in some functions and returning directly on
> > errors in others could use a cleanup. Please consider doing that while you are
> > touching those files.
> 
> Will do. The most recent changes I made to netvsc.c, I think was
> consistent with the existing code; going forward we will certainly
> move towards a more consistent coding style.

It scares me when you talk about being consistent with the existing
code...  Just do it the correct way.

1) Don't do the "return ret;" if you know ret is zero.

2) Replace:
	ret = vmbus_sendpacket(...);

	return ret;
   with
  	return vmbus_sendpacket(...);

3) Don't do "goto cleanup;" when "return ret;" will suffice.  The
   do-nothing goto is misleading because you assume it will cleanup
   somthing.  Some people used to misread CodingStyle to think that all
   functions should only have one return but I have updated it so it is
   more clear.

regards,
dan carpenter

  reply	other threads:[~2014-04-24 22:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23 21:24 [PATCH V1 net-next 1/1] hyperv: Enable sendbuf mechanism on the send path K. Y. Srinivasan
2014-04-23 21:24 ` K. Y. Srinivasan
2014-04-23 21:45 ` Dan Carpenter
2014-04-23 21:45   ` Dan Carpenter
2014-04-24 21:49 ` Andev
2014-04-24 21:49   ` Andev
2014-04-24 22:06   ` KY Srinivasan
2014-04-24 22:06     ` KY Srinivasan
2014-04-24 22:30     ` Dan Carpenter [this message]
2014-04-24 22:30       ` Dan Carpenter
2014-04-30 14:17 ` KY Srinivasan
2014-04-30 16:06   ` David Miller
2014-04-30 16:06     ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2014-04-30 17:14 K. Y. Srinivasan
2014-04-30 17:14 ` K. Y. Srinivasan
2014-04-30 17:36 ` David Miller
2014-04-30 17:36   ` David Miller
2014-04-30 18:51 ` Dan Carpenter
2014-04-30 18:51   ` Dan Carpenter

Reply instructions:

You may reply publicly 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=20140424223059.GE26890@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=apw@canonical.com \
    --cc=davem@davemloft.net \
    --cc=debiandev@gmail.com \
    --cc=devel@linuxdriverproject.org \
    --cc=jasowang@redhat.com \
    --cc=kys@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olaf@aepfle.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.