* checkpatch: kill the bogus camelcase check @ 2013-04-09 21:50 James Bottomley 2013-04-10 13:26 ` Andy Whitcroft 0 siblings, 1 reply; 9+ messages in thread From: James Bottomley @ 2013-04-09 21:50 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel, linux-scsi It's making checkpatch unusable on most drivers because it's spewing tons of bogus warnings. The problem is the assumption that studly caps is always wrong: it isn't if the variables are named after the various conventions in the hardware programming guides (which are usually written by Microsoft people). In order to encourage people to use checkpatch, it has to be *useful* it can't stray too far into dogmatic things like this that are essentially unfixable by most people who submit patches. Signed-off-by: James Bottomley <JBottomley@Parallels.com> --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b28cc38..5588dd3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1398,8 +1398,6 @@ sub process { my %suppress_export; my $suppress_statement = 0; - my %camelcase = (); - # Pre-scan the patch sanitizing the lines. # Pre-scan the patch looking for any __setup documentation. # @@ -2925,19 +2923,6 @@ sub process { } } -#CamelCase - while ($line =~ m{($Constant|$Lval)}g) { - my $var = $1; - if ($var !~ /$Constant/ && - $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ && - $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && - !defined $camelcase{$var}) { - $camelcase{$var} = 1; - WARN("CAMELCASE", - "Avoid CamelCase: <$var>\n" . $herecurr); - } - } - #no spaces allowed after \ in define if ($line=~/\#\s*define.*\\\s$/) { WARN("WHITESPACE_AFTER_LINE_CONTINUATION", ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: checkpatch: kill the bogus camelcase check 2013-04-09 21:50 checkpatch: kill the bogus camelcase check James Bottomley @ 2013-04-10 13:26 ` Andy Whitcroft 2013-04-10 14:35 ` Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Andy Whitcroft @ 2013-04-10 13:26 UTC (permalink / raw) To: James Bottomley; +Cc: Joe Perches, linux-kernel, linux-scsi On Tue, Apr 09, 2013 at 02:50:54PM -0700, James Bottomley wrote: > It's making checkpatch unusable on most drivers because it's spewing > tons of bogus warnings. The problem is the assumption that studly caps > is always wrong: it isn't if the variables are named after the various > conventions in the hardware programming guides (which are usually > written by Microsoft people). > > In order to encourage people to use checkpatch, it has to be *useful* it > can't stray too far into dogmatic things like this that are essentially > unfixable by most people who submit patches. > > Signed-off-by: James Bottomley <JBottomley@Parallels.com> Joe, perhaps this could become a strict check? -apw ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: checkpatch: kill the bogus camelcase check 2013-04-10 13:26 ` Andy Whitcroft @ 2013-04-10 14:35 ` Joe Perches 2013-04-10 14:52 ` Borislav Petkov 0 siblings, 1 reply; 9+ messages in thread From: Joe Perches @ 2013-04-10 14:35 UTC (permalink / raw) To: Andy Whitcroft; +Cc: James Bottomley, linux-kernel, linux-scsi On Wed, 2013-04-10 at 14:26 +0100, Andy Whitcroft wrote: > On Tue, Apr 09, 2013 at 02:50:54PM -0700, James Bottomley wrote: > > It's making checkpatch unusable on most drivers because it's spewing > > tons of bogus warnings. The problem is the assumption that studly caps > > is always wrong: it isn't if the variables are named after the various > > conventions in the hardware programming guides (which are usually > > written by Microsoft people). > > > > In order to encourage people to use checkpatch, it has to be *useful* it > > can't stray too far into dogmatic things like this that are essentially > > unfixable by most people who submit patches. > > > > Signed-off-by: James Bottomley <JBottomley@Parallels.com> > > Joe, perhaps this could become a strict check? or maybe exclude drivers/scsi and include/scsi/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: checkpatch: kill the bogus camelcase check 2013-04-10 14:35 ` Joe Perches @ 2013-04-10 14:52 ` Borislav Petkov 2013-04-10 15:07 ` James Bottomley 0 siblings, 1 reply; 9+ messages in thread From: Borislav Petkov @ 2013-04-10 14:52 UTC (permalink / raw) To: Joe Perches; +Cc: Andy Whitcroft, James Bottomley, linux-kernel, linux-scsi On Wed, Apr 10, 2013 at 07:35:58AM -0700, Joe Perches wrote: > or maybe exclude drivers/scsi and include/scsi/ and arch/x86/kvm/emulate.c -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: checkpatch: kill the bogus camelcase check 2013-04-10 14:52 ` Borislav Petkov @ 2013-04-10 15:07 ` James Bottomley 2013-04-10 15:19 ` Borislav Petkov 0 siblings, 1 reply; 9+ messages in thread From: James Bottomley @ 2013-04-10 15:07 UTC (permalink / raw) To: Borislav Petkov; +Cc: Joe Perches, Andy Whitcroft, linux-kernel, linux-scsi On Wed, 2013-04-10 at 16:52 +0200, Borislav Petkov wrote: > On Wed, Apr 10, 2013 at 07:35:58AM -0700, Joe Perches wrote: > > or maybe exclude drivers/scsi and include/scsi/ > > and arch/x86/kvm/emulate.c Actually, we get this all over drivers. Some of the problems are to do with the fact that the check is wrong, so it thinks things like this drm_core_has_MTRR are studly caps when they're not, but we have a lot of device programming manual driven studly caps in PCI, ata, ide etc .. We also have sanctioned use in mm, things like: SetPageReserved ClearPageReserved But the point still stands. When checkpatch warns about this, there's nothing that the person submitting the patch can do because the usage was already embedded into the file they're patching. James ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: checkpatch: kill the bogus camelcase check 2013-04-10 15:07 ` James Bottomley @ 2013-04-10 15:19 ` Borislav Petkov 2013-04-11 14:45 ` [PATCH] checkpatch: Make camelcase test --strict and less noisy Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Borislav Petkov @ 2013-04-10 15:19 UTC (permalink / raw) To: James Bottomley; +Cc: Joe Perches, Andy Whitcroft, linux-kernel, linux-scsi On Wed, Apr 10, 2013 at 08:07:41AM -0700, James Bottomley wrote: > On Wed, 2013-04-10 at 16:52 +0200, Borislav Petkov wrote: > > On Wed, Apr 10, 2013 at 07:35:58AM -0700, Joe Perches wrote: > > > or maybe exclude drivers/scsi and include/scsi/ > > > > and arch/x86/kvm/emulate.c > > Actually, we get this all over drivers. Some of the problems are to do > with the fact that the check is wrong, so it thinks things like this > > drm_core_has_MTRR > > are studly caps when they're not, but we have a lot of device > programming manual driven studly caps in PCI, ata, ide etc .. > > We also have sanctioned use in mm, things like: > > SetPageReserved > ClearPageReserved > > But the point still stands. When checkpatch warns about this, there's > nothing that the person submitting the patch can do because the usage > was already embedded into the file they're patching. Ha, I hit the nail right on the head! :-) This is exactly the reaction I was aiming at, with mentioning yet another file in the kernel where this check fires. Yes, excepting certain files is not a good idea. Yes, the check needs to go away because it is plain wrong. Even if its there, I keep ignoring it which makes checkpatch less useful and trustworthy. So full ACK to the intent to either remove it or make it a suggestion only. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] checkpatch: Make camelcase test --strict and less noisy 2013-04-10 15:19 ` Borislav Petkov @ 2013-04-11 14:45 ` Joe Perches 2013-05-01 12:34 ` Borislav Petkov 0 siblings, 1 reply; 9+ messages in thread From: Joe Perches @ 2013-04-11 14:45 UTC (permalink / raw) To: Borislav Petkov; +Cc: James Bottomley, Andy Whitcroft, linux-kernel, linux-scsi CamelCase tests are a bit noisy against certain types of code acceptable to some kernel developers. Make the test applicable only with --strict. Do not bleat a message on nominally acceptable CamelCase uses that are separated by an _ like drm_core_has_MTRR. Signed-off-by: Joe Perches <joe@perches.com> --- scripts/checkpatch.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3fb6d86..97226fb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2937,12 +2937,12 @@ sub process { while ($line =~ m{($Constant|$Lval)}g) { my $var = $1; if ($var !~ /$Constant/ && - $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ && + $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && !defined $camelcase{$var}) { $camelcase{$var} = 1; - WARN("CAMELCASE", - "Avoid CamelCase: <$var>\n" . $herecurr); + CHK("CAMELCASE", + "Avoid CamelCase: <$var>\n" . $herecurr); } } ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch: Make camelcase test --strict and less noisy 2013-04-11 14:45 ` [PATCH] checkpatch: Make camelcase test --strict and less noisy Joe Perches @ 2013-05-01 12:34 ` Borislav Petkov 2013-05-01 13:50 ` James Bottomley 0 siblings, 1 reply; 9+ messages in thread From: Borislav Petkov @ 2013-05-01 12:34 UTC (permalink / raw) To: Joe Perches; +Cc: James Bottomley, Andy Whitcroft, linux-kernel, linux-scsi On Thu, Apr 11, 2013 at 07:45:03AM -0700, Joe Perches wrote: > CamelCase tests are a bit noisy against certain > types of code acceptable to some kernel developers. > > Make the test applicable only with --strict. > > Do not bleat a message on nominally acceptable > CamelCase uses that are separated by an _ like > drm_core_has_MTRR. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > scripts/checkpatch.pl | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 3fb6d86..97226fb 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2937,12 +2937,12 @@ sub process { > while ($line =~ m{($Constant|$Lval)}g) { > my $var = $1; > if ($var !~ /$Constant/ && > - $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ && > + $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && > $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && > !defined $camelcase{$var}) { > $camelcase{$var} = 1; > - WARN("CAMELCASE", > - "Avoid CamelCase: <$var>\n" . $herecurr); > + CHK("CAMELCASE", > + "Avoid CamelCase: <$var>\n" . $herecurr); Yep, this is better. James? -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] checkpatch: Make camelcase test --strict and less noisy 2013-05-01 12:34 ` Borislav Petkov @ 2013-05-01 13:50 ` James Bottomley 0 siblings, 0 replies; 9+ messages in thread From: James Bottomley @ 2013-05-01 13:50 UTC (permalink / raw) To: Borislav Petkov; +Cc: Joe Perches, Andy Whitcroft, linux-kernel, linux-scsi On Wed, 2013-05-01 at 14:34 +0200, Borislav Petkov wrote: > On Thu, Apr 11, 2013 at 07:45:03AM -0700, Joe Perches wrote: > > CamelCase tests are a bit noisy against certain > > types of code acceptable to some kernel developers. > > > > Make the test applicable only with --strict. > > > > Do not bleat a message on nominally acceptable > > CamelCase uses that are separated by an _ like > > drm_core_has_MTRR. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > scripts/checkpatch.pl | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 3fb6d86..97226fb 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -2937,12 +2937,12 @@ sub process { > > while ($line =~ m{($Constant|$Lval)}g) { > > my $var = $1; > > if ($var !~ /$Constant/ && > > - $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ && > > + $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && > > $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && > > !defined $camelcase{$var}) { > > $camelcase{$var} = 1; > > - WARN("CAMELCASE", > > - "Avoid CamelCase: <$var>\n" . $herecurr); > > + CHK("CAMELCASE", > > + "Avoid CamelCase: <$var>\n" . $herecurr); > > Yep, this is better. Fine with me ... I don't use --strict so it will make the message deluge go away. Thanks, James ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-05-01 13:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-09 21:50 checkpatch: kill the bogus camelcase check James Bottomley 2013-04-10 13:26 ` Andy Whitcroft 2013-04-10 14:35 ` Joe Perches 2013-04-10 14:52 ` Borislav Petkov 2013-04-10 15:07 ` James Bottomley 2013-04-10 15:19 ` Borislav Petkov 2013-04-11 14:45 ` [PATCH] checkpatch: Make camelcase test --strict and less noisy Joe Perches 2013-05-01 12:34 ` Borislav Petkov 2013-05-01 13:50 ` James Bottomley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).