mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: David Rientjes <rientjes@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, mm-commits@vger.kernel.org,
	apw@canonical.com
Subject: Re: + checkpatch-add-check-for-use-of-sizeof-without-parenthesis.patch added to -mm tree
Date: Mon, 09 Jul 2012 22:12:32 -0700	[thread overview]
Message-ID: <1341897152.6118.116.camel@joe2Laptop> (raw)
In-Reply-To: <alpine.DEB.2.00.1207091904480.25143@chino.kir.corp.google.com>

On Mon, 2012-07-09 at 19:21 -0700, David Rientjes wrote:
> On Mon, 9 Jul 2012, Joe Perches wrote:
> 
> > I don't really care what style a large block of code
> > uses.  I care that it mostly has the same form.

> Same form??  The sizeof operator has two forms depending on whether it's a 
> unary expression or a type as specified by the standard.

> The issue here is that you're mandating they all use the same form because 
> you're quoting an email from Linus four years ago that you dug up but 
> isn't required in the coding style and is already used in over 1000 places 
> in the kernel.

$ git grep -E "\bsizeof\s*\*"|wc -l
935
$ git grep -E "\bsizeof\s*\(\s*\*"|wc -l
12762

> If you want the output of checkpatch.pl to be useful, I would think you 
> would want to eliminate this kind of garbage.

You are using high emotion words for little purpose.

checkpatch is useful, but it's not all that useful for those
quite familiar with kernel style.

Except maybe to generate flame emails...

It does have some use for reviewing patches.

> > "Another example of this is "sizeof". The kernel universally (I hope) has 
> > parenthesis around the sizeof argument, even though it's clearly not 
> > required by the C language."
> 
> He's obviously addressing a single form of the sizeof operator, i.e. those 
> on unary expressions; sizeof used on a type CLEARLY DOES require the 
> parenthesis.

That's one opinion, though I doubt it's his.
<shrug>  Maybe he'll reply.  (edit: he did)

> you're talking purely about style preferences here.

_All_ of checkpatch is style preference.
None if it is a mandate.  Those that care to use it can.
You can ignore it.  I don't mind.

Otherwise, just read and write the code and do what you
think best.  I'm not a particular style zealot.  I'm not
going to nack a patch just because you or anyone else uses
a style that isn't the predominate one.

cheers, Joe


  reply	other threads:[~2012-07-10  5:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120709215256.9A4F71E0043@wpzn4.hot.corp.google.com>
2012-07-09 22:23 ` David Rientjes
2012-07-09 22:36   ` Joe Perches
2012-07-09 22:55     ` David Rientjes
2012-07-09 23:21       ` Joe Perches
2012-07-09 23:47         ` David Rientjes
2012-07-10  0:55           ` Joe Perches
2012-07-10  1:50             ` David Rientjes
2012-07-10  2:03               ` Joe Perches
2012-07-10  2:21                 ` David Rientjes
2012-07-10  5:12                   ` Joe Perches [this message]
2012-07-10  2:28               ` Linus Torvalds
2012-07-10  2:49                 ` David Rientjes
2012-07-12  5:23 ` [PATCH V2] checkpatch: Add check for use of sizeof without parenthesis Joe Perches

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=1341897152.6118.116.camel@joe2Laptop \
    --to=joe@perches.com \
    --cc=apw@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mm-commits@vger.kernel.org \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    /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

all inboxes | Powered by JetHome®