From: Daniel Walker <dwalker@fifo99.com>
To: Jing Huang <huangj@brocade.com>
Cc: James.Bottomley@HansenPartnership.com, kgudipat@brocade.com,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
rvadivel@brocade.com, vravindr@brocade.com
Subject: Re: [PATCH 2/14] bfa: Brocade BFA FC SCSI driver (bfa1)
Date: Thu, 24 Sep 2009 08:44:02 -0700 [thread overview]
Message-ID: <1253807042.20648.188.camel@desktop> (raw)
In-Reply-To: <200909240049.n8O0nw3l013437@swe58.brocade.com>
On Wed, 2009-09-23 at 17:49 -0700, Jing Huang wrote:
> +
> + return (*(union bfi_addr_u *) &addr);
> +}
Have you run checkpatch on this code? It produces many errors due to
your "return" usage for one.. The usual style of return is not to use
parentheses since it's really not a function ..
The line I quoted above gives the following error,
ERROR: return is not a function, parentheses are not required
#266: FILE: drivers/scsi/bfa/bfa_cb_ioim_macros.h:132:
+ return (*(union bfi_addr_u *) &addr);
First of all I'd consider making your code consistent with respect to
the return statements .. I noticed that you sometimes use the
parentheses sometimes not .. Since it's more with Linux style I'd just
remove all the extra parentheses..
Checkpatch produces many other errors in your code .. If you haven't
already evaluated those errors, I'd do go through each patch and review
the errors (and the warnings) that it produces since checkpatch can give
you a fairly mechanical view into how well your code matches the Linux
coding style. The less the output from checkpatch the better ..
Daniel
next prev parent reply other threads:[~2009-09-24 15:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-24 0:49 [PATCH 2/14] bfa: Brocade BFA FC SCSI driver (bfa1) Jing Huang
2009-09-24 15:44 ` Daniel Walker [this message]
2009-09-24 15:59 ` Alan Cox
2009-09-24 16:15 ` Daniel Walker
2009-09-24 17:38 ` Jing Huang
2009-09-24 17:55 ` Daniel Walker
2009-09-24 18:08 ` Jing Huang
2009-09-24 22:28 ` James Bottomley
2009-09-24 22:33 ` Jing Huang
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=1253807042.20648.188.camel@desktop \
--to=dwalker@fifo99.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=huangj@brocade.com \
--cc=kgudipat@brocade.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=rvadivel@brocade.com \
--cc=vravindr@brocade.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox