Message-ID: <4CD9887E.3040607@caviumnetworks.com>
Date: Tue, 09 Nov 2010 09:44:30 -0800
From: David Daney <ddaney@caviumnetworks.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10
MIME-Version: 1.0
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?
References: <20101109173357.GA5876@outflux.net>
In-Reply-To: <20101109173357.GA5876@outflux.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1790
Lines: 45

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/