From: Joe Perches <joe@perches.com>
To: Graeme Russ <graeme.russ@gmail.com>,
Andy Whitcroft <apw@canonical.com>,
Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>, U-Boot Users <u-boot@lists.denx.de>
Subject: Re: Expanding checkpatch for non-linux (specifically U-Boot) use
Date: Sun, 24 Apr 2011 23:02:20 -0700 [thread overview]
Message-ID: <1303711340.1745.30.camel@Joe-Laptop> (raw)
In-Reply-To: <4DB50854.8090700@gmail.com>
On Mon, 2011-04-25 at 15:36 +1000, Graeme Russ wrote:
> There has been a bit of discussion lately on the U-Boot mailing list
> regarding the use of checkpatch for U-Boot patches (see
> http://lists.denx.de/pipermail/u-boot/2011-April/090954.html)
>
> U-Boot uses the Linux coding style and checkpatch is therefore a very good
> tool for us to use to check style compliance. However, checkpatch has a few
> Linux specific checks which throw up false warnings for U-Boot patches like:
>
> WARNING: consider using kstrto* in preference to simple_strto*
> WARNING: Use #include <linux/$file> instead of <asm/$file>
>
> Also, checkpatch seems to be checking not only patched lines, but context
> lines as well. There is a policy for U-Boot patches to not intermix
> whitespace / code cleanup changes and functional changes in in the same
> patch. So to achieve zero warnings and errors, the submitter is forced to
> create an additional code-cleanup patch in addition to the functionality
> patch. The code cleanup can end up being significantly larger than the
> functionality change which discourages casual submitters.
>
> So I have a pretty simple question to ask of LKML - Will checkpatch patches
> to create a 'U-Boot' command-line option to explicitly filter out Linux
> specific warnings and errors ever be accepted into checkpatch, or will we
> be required to create and maintain a U-Boot specific version?
>
> P.S. If you could please keep the U-Boot mailing list Cc'd, that would be
> appreciated
Hi Graeme.
Perhaps some sort of .checkpatch.conf file
could be introduced which could be linked to
specific types of errors/warnings/checks
that should be reported or ignored.
checkpatch has central routines to emit messages.
sub ERROR {
if (report("ERROR: $_[0]\n")) {
our $clean = 0;
our $cnt_error++;
}
}
sub WARN {
if (report("WARNING: $_[0]\n")) {
our $clean = 0;
our $cnt_warn++;
}
}
sub CHK {
if ($check && report("CHECK: $_[0]\n")) {
our $clean = 0;
our $cnt_chk++;
}
}
For instance, warnings could be changed to include
a new unique identifier for each message.
from
WARN("Signed-off-by: is the preferred form\n" .
$herecurr);
to
WARN($WARN_SIGN_OFF,
"Signed-off-by: is the preferred form\n" .
$herecurr);
and the ERROR/WARN/CHK routines could be extended to use
entries in the .conf file to enable/disable each message.
uboot could then use an appropriate .conf file.
WARNING: multiple messages have this Message-ID (diff)
From: Joe Perches <joe@perches.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] Expanding checkpatch for non-linux (specifically U-Boot) use
Date: Sun, 24 Apr 2011 23:02:20 -0700 [thread overview]
Message-ID: <1303711340.1745.30.camel@Joe-Laptop> (raw)
In-Reply-To: <4DB50854.8090700@gmail.com>
On Mon, 2011-04-25 at 15:36 +1000, Graeme Russ wrote:
> There has been a bit of discussion lately on the U-Boot mailing list
> regarding the use of checkpatch for U-Boot patches (see
> http://lists.denx.de/pipermail/u-boot/2011-April/090954.html)
>
> U-Boot uses the Linux coding style and checkpatch is therefore a very good
> tool for us to use to check style compliance. However, checkpatch has a few
> Linux specific checks which throw up false warnings for U-Boot patches like:
>
> WARNING: consider using kstrto* in preference to simple_strto*
> WARNING: Use #include <linux/$file> instead of <asm/$file>
>
> Also, checkpatch seems to be checking not only patched lines, but context
> lines as well. There is a policy for U-Boot patches to not intermix
> whitespace / code cleanup changes and functional changes in in the same
> patch. So to achieve zero warnings and errors, the submitter is forced to
> create an additional code-cleanup patch in addition to the functionality
> patch. The code cleanup can end up being significantly larger than the
> functionality change which discourages casual submitters.
>
> So I have a pretty simple question to ask of LKML - Will checkpatch patches
> to create a 'U-Boot' command-line option to explicitly filter out Linux
> specific warnings and errors ever be accepted into checkpatch, or will we
> be required to create and maintain a U-Boot specific version?
>
> P.S. If you could please keep the U-Boot mailing list Cc'd, that would be
> appreciated
Hi Graeme.
Perhaps some sort of .checkpatch.conf file
could be introduced which could be linked to
specific types of errors/warnings/checks
that should be reported or ignored.
checkpatch has central routines to emit messages.
sub ERROR {
if (report("ERROR: $_[0]\n")) {
our $clean = 0;
our $cnt_error++;
}
}
sub WARN {
if (report("WARNING: $_[0]\n")) {
our $clean = 0;
our $cnt_warn++;
}
}
sub CHK {
if ($check && report("CHECK: $_[0]\n")) {
our $clean = 0;
our $cnt_chk++;
}
}
For instance, warnings could be changed to include
a new unique identifier for each message.
from
WARN("Signed-off-by: is the preferred form\n" .
$herecurr);
to
WARN($WARN_SIGN_OFF,
"Signed-off-by: is the preferred form\n" .
$herecurr);
and the ERROR/WARN/CHK routines could be extended to use
entries in the .conf file to enable/disable each message.
uboot could then use an appropriate .conf file.
next prev parent reply other threads:[~2011-04-25 6:02 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-25 5:36 Expanding checkpatch for non-linux (specifically U-Boot) use Graeme Russ
2011-04-25 5:36 ` [U-Boot] " Graeme Russ
2011-04-25 6:02 ` Joe Perches [this message]
2011-04-25 6:02 ` Joe Perches
2011-04-25 6:22 ` Graeme Russ
2011-04-25 6:22 ` [U-Boot] " Graeme Russ
2011-04-25 6:32 ` Joe Perches
2011-04-25 6:32 ` [U-Boot] " Joe Perches
2011-04-26 0:43 ` [RFC PATCH] checkpatch.pl: Add ability to ignore various messages Joe Perches
2011-04-26 0:43 ` [U-Boot] " Joe Perches
2011-06-09 19:26 ` Joe Perches
2011-06-09 19:26 ` [U-Boot] " Joe Perches
2011-06-09 19:40 ` Mike Frysinger
2011-06-09 19:40 ` Mike Frysinger
2011-07-27 21:17 ` Wolfgang Denk
2011-07-27 21:17 ` Wolfgang Denk
2011-07-27 22:10 ` Joe Perches
2011-07-27 22:10 ` Joe Perches
2011-08-24 21:25 ` Wolfgang Denk
2011-08-24 21:25 ` Wolfgang Denk
2011-08-24 21:33 ` Joe Perches
2011-08-24 21:33 ` 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=1303711340.1745.30.camel@Joe-Laptop \
--to=joe@perches.com \
--cc=akpm@linux-foundation.org \
--cc=apw@canonical.com \
--cc=graeme.russ@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=u-boot@lists.denx.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.