public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Daney <ddaney@caviumnetworks.com>
To: Kees Cook <kees.cook@canonical.com>
Cc: linux-kernel@vger.kernel.org, Andy Whitcroft <apw@canonical.com>
Subject: Re: [linux-next] automatic use of checkpatch.pl for security?
Date: Tue, 09 Nov 2010 09:44:30 -0800	[thread overview]
Message-ID: <4CD9887E.3040607@caviumnetworks.com> (raw)
In-Reply-To: <20101109173357.GA5876@outflux.net>

On 11/09/2010 09:33 AM, Kees Cook wrote:
> Hi,
>
> In an effort to continue the constification work, it'd be nice to
> not accidentally introduce regressions or add additional work. Since
> checkpatch.pl already knows to warn about a lot of things including const
> structures, it would be great to have all commits going through linux-next
> (or something) have to pass at least a subset of checkpatch.pl's checks.
>
> For example, Lionel Debroux pointed out to me that looking at the last
> 1000 commits, there are a lot of warnings, including things like:
>
> WARNING: struct dma_map_ops should normally be const
> #499: FILE: arch/mips/mm/dma-default.c:301:
> +static struct dma_map_ops mips_default_dma_map_ops = {
>
> Can we add some kind of automatic checking to actually give checkpatch.pl
> some real teeth for at least some of its checks?
>

Ok, did you actually try to make it const as suggested?  If you had, you 
would have found that there are declarations throughout the code base 
that conflict with checkpatch.pl's suggestion.

There are several things we could do:

1) Force people to clean up the entire kernel tree before making trivial 
changes that checkpatch.pl might complain about.

2) Change checkpatch.pl so that it doesn't complain about this.

3) Make reasonable changes and ignore the checkpatch.pl warning.


In that specific case you cite, #3 was chosen.

If you gate admission to linux-next with some sort of automated check 
like this, I fear the wrath of the disgruntled masses may fall upon you.

David Daney

  reply	other threads:[~2010-11-09 17:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-09 17:33 [linux-next] automatic use of checkpatch.pl for security? Kees Cook
2010-11-09 17:44 ` David Daney [this message]
2010-11-09 17:59   ` Kees Cook
2010-11-09 20:49     ` Lionel Debroux
2010-11-10 18:28       ` Randy Dunlap

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=4CD9887E.3040607@caviumnetworks.com \
    --to=ddaney@caviumnetworks.com \
    --cc=apw@canonical.com \
    --cc=kees.cook@canonical.com \
    --cc=linux-kernel@vger.kernel.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