Messages in this thread | | | Date | Fri, 9 May 2014 16:29:14 -0500 | From | Kim Phillips <> | Subject | Re: [PATCH] checkpatch: Make --strict a default for files in drivers/net and net/ |
| |
On Fri, 9 May 2014 13:56:15 -0700 Joe Perches <joe@perches.com> wrote:
> On Fri, 2014-05-09 at 15:37 -0500, Kim Phillips wrote: > > On Wed, 7 May 2014 11:13:26 -0700 > > Joe Perches <joe@perches.com> wrote: > > > > > Networking files are generally more strictly conformant to > > > linux-kernel style > > > > checkpatch disagrees :) : > > > > {drivers/}net/ : ~10.8 CHECKs per .[hc] file > > everything else: ~10 CHECKs per .[hc] file > > no net, no drivers/staging: ~8.6 CHECKs per .[hc] file > > > > (see [1] below for details). > > > > > + if ($found_file) { > > > + if ($realfile =~ m@^(drivers/net/|net/)@) { > > > > this isn't easily extensible/scalable to other subsystems, or > > say something like "all Freescale drivers." Having it configurable > > in .checkpatch.conf might be a better solution, but I don't believe > > networking should be the only subsystem that can take advantage of > > the extra checkpatch CHECKs. > > staging probably could too.
why call staging out to be subject to stricter checks than the rest of the kernel, when it's code that's _known_ to be non-compliant?
> > Can we enable --strict universally in the Linux kernel, maybe like > > so:? > > I don't think that's appropriate (yet?).
why not? checkpatch CHECKs seem reasonably stable AFAICT.
> Anyone that wants --strict checking can either add > it on the command line
most aren't aware of --strict, and even if they are, easily forget to add it.
> or create a .checkpatch.conf > file with --strict in it.
right, but since checkpatch already has internal knowledge of the linux kernel, I thought it inappropriate to add a .checkpatch.conf, and use the $tree switch.
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -228,6 +228,7 @@ if ($tree) { > > print "Must be run from the top-level dir. of a kernel tree\n"; > > exit(2); > > } > > + $check = 1; > > } > > > > my $emitted_corrupt = 0; > > > > fwiw, --strict has been set unconditionally in u-boot's > > .checkpatch.conf for over a year now, and has significantly reduced > > patch revision churn for simple mistakes like parenthesis alignment, > > spaces after a cast and/or before a semicolon... > > Good to know.
would an alternative option be to promote CHECKs like parentheses alignment to WARN status?
> > [1] the following were run on kernel v3.15-rc4-260-g38583f0: > > > > for i in `git ls-files | grep -v '^net/' | grep -v '^drivers/net/' | grep \\\.[ch]$`; do scripts/checkpatch.pl --strict -f $i; done | grep ^CHECK: | wc -l > > 328329 > > git ls-files | grep -v '^net/' | grep -v '^drivers/net/' | grep \\\.[ch]$ | wc -l > > 32754 > > => 32754 / 328329 ~= 10 CHECKs per file.[ch] for everything outside of {drivers/}net/. > > > > for i in `git ls-files net/ drivers/net/ | grep \\\.[ch]$`; do scripts/checkpatch.pl --strict -f $i; done | grep ^CHECK: | wc -l > > 40431 > > git ls-files net/ drivers/net/ | grep \\\.[ch]$ | wc -l > > 3742 > > => {drivers/}net/: 40431 / 3742 = 10.8 CHECKs per file.[hc] > > > > for i in `git ls-files | grep -v '^net/' | grep -v '^drivers/net/' | grep -v '^drivers/staging/' | grep \\\.[ch]$`; do scripts/checkpatch.pl --strict -f $i; done | grep ^CHECK: | wc -l > > 262005 > > git ls-files | grep -v '^net/' | grep -v '^drivers/net/' | grep -v '^drivers/staging/' | grep \\\.[ch]$ | wc -l > > 30479 > > => no net, no staging: 262005 / 30479 = 8.6 CHECKs per file.[hc] > > Try that with a per-file "wc -l" for checks per LOC
good point, that reinforces the statement back for networking :) :
{drivers/}net/ : 0.013 CHECKs per .[hc] file line everything else: 0.43 CHECKs per .[hc] file line no net, no drivers/staging: 0.38 CHECKs per .[hc] file line
Thanks,
Kim
| |