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
next prev parent 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.