linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Juhl <jj@chaosbits.net>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	devel@driverdev.osuosl.org,
	Lucas De Marchi <lucas.demarchi@profusion.mobi>,
	Ilia Mirkin <imirkin@alum.mit.edu>,
	linux-kernel@vger.kernel.org,
	"Justin P. Mattock" <justinmattock@gmail.com>
Subject: Re: [PATCH 1/2] Staging; bcm/CmHost.c: Style cleanup
Date: Sat, 28 Jan 2012 00:43:39 +0100 (CET)	[thread overview]
Message-ID: <alpine.LNX.2.00.1201280037080.13952@swampdragon.chaosbits.net> (raw)
In-Reply-To: <20120125062658.GR3294@mwanda>

On Wed, 25 Jan 2012, Dan Carpenter wrote:

> This is probably going to need to be redone on top of the other bcm
> cleanup patches anyway.
> 
If that turns out to be so, well, so be it. I was not aware of other 
patches, so my bad...

> On Tue, Jan 24, 2012 at 11:46:31PM +0100, Jesper Juhl wrote:
> > +		pstClassifierEntry->ucDestPortRangeLength
> > +			= psfCSType->cCPacketClassificationRule.u8ProtocolDestPortRangeLength/4;
> 
> Put the equals on the first line when you break something up like
> this. Also the real fix here is to choose shorter and better names
> than "cCPacketClassificationRule.u8ProtocolDestPortRangeLength".
> 
> If you're breaking up a condition normally the && or || go on the
> first line as well.
> 	if (foo &&
> 	    bar &&
> 	    baz) { ...
> 
> Although that's not in CodingStyle so it's not a "redo the patch"
> situation.
> 
As you say, "not in CodingStyle", so I guess it's down to your personal 
preference vs my personal preference vs what's most common in the kernel 
vs maintainers preference.

I did it the way I did for a reason.
I personally find that if one puts the '=', '==', '&&', '||', '+', 
<whatever> at the end of each line on a multi-line statement, then they 
are easy to overlook when just reading the code casually. If, however, one 
puts them at the start of each line, they stand out and are easier to see 
(harder to miss). That's why I did as I did. You may not agree, but at 
least now you know my reason for doing it the way I did.


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


  reply	other threads:[~2012-01-27 23:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-24 22:44 [PATCH 0/2] Staging: Style cleanup and mem leak fixes for drivers/staging/bcm/CmHost.c Jesper Juhl
2012-01-24 22:46 ` [PATCH 1/2] Staging; bcm/CmHost.c: Style cleanup Jesper Juhl
2012-01-24 23:13   ` Joe Perches
2012-01-24 23:17     ` Jesper Juhl
2012-01-25  6:26   ` Dan Carpenter
2012-01-27 23:43     ` Jesper Juhl [this message]
2012-01-28  7:24       ` Dan Carpenter
2012-01-24 22:47 ` [PATCH 2/2] Staging; Don't leak 'pstAddIndication' in CmHost.c::StoreCmControlResponseMessage() Jesper Juhl
2012-02-09 17:52   ` Greg KH
2012-02-09 22:33     ` Jesper Juhl
2012-02-10 18:00       ` Staging: bcm: " Greg KH
2012-02-11 23:54         ` Jesper Juhl
2012-02-14  4:05           ` Greg KH

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=alpine.LNX.2.00.1201280037080.13952@swampdragon.chaosbits.net \
    --to=jj@chaosbits.net \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@suse.de \
    --cc=imirkin@alum.mit.edu \
    --cc=justinmattock@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucas.demarchi@profusion.mobi \
    /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;
as well as URLs for NNTP newsgroup(s).