[PATCH] checkpatch: forgive use of mixed case variables measuring units

[PATCH] checkpatch: forgive use of mixed case variables measuring units

[Phil Carmody]

I don't think anyone really has an issue with things like max_mV.
And whilst nS et al. may not be SI standard, at least it's clear
what they represent.

Signed-off-by: Phil Carmody <phil.carmody@partner.samsung.com>
---
 scripts/checkpatch.pl |    1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b954de5..c29fd2f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2940,6 +2940,7 @@ sub process {
 			if ($var !~ /$Constant/ &&
 			    $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ &&
 			    $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
+			    $var !~ /^[a-z_]*_[numk][VAS]$/ &&
 			    !defined $camelcase{$var}) {
 				$camelcase{$var} = 1;
 				WARN("CAMELCASE",
-- 
1.7.9.5

Re: [PATCH] checkpatch: forgive use of mixed case variables measuring units

[Joe Perches]

On Wed, 2013-05-29 at 14:02 +0300, Phil Carmody wrote:
> I don't think anyone really has an issue with things like max_mV.
> And whilst nS et al. may not be SI standard, at least it's clear
> what they represent.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2940,6 +2940,7 @@ sub process {
>  			if ($var !~ /$Constant/ &&
>  			    $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ &&
>  			    $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
> +			    $var !~ /^[a-z_]*_[numk][VAS]$/ &&
>  			    !defined $camelcase{$var}) {
>  				$camelcase{$var} = 1;
>  				WARN("CAMELCASE",

Hi Phil.

CamelCase was downgraded to a --strict only (CHK)
test in -next recently.  commit f0e8102413
("checkpatch: change CamelCase test and make it --strict")

I'm hesitant to add a longish whitelist inside
checkpatch itself, but if it's added, it should
probably be an array.

RE: [PATCH] checkpatch: forgive use of mixed case variables measuring units

[Phil Carmody]

Joe Perches wrote:
> On Wed, 2013-05-29 at 14:02 +0300, Phil Carmody wrote:
> > I don't think anyone really has an issue with things like max_mV.
> > And whilst nS et al. may not be SI standard, at least it's clear
> > what they represent.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -2940,6 +2940,7 @@ sub process {
> >  			if ($var !~ /$Constant/ &&
> >  			    $var =~ /[A-Z]\w*[a-z]|[a-z]\w*[A-Z]/ &&
> >  			    $var !~
/"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
> > +			    $var !~ /^[a-z_]*_[numk][VAS]$/ &&
> >  			    !defined $camelcase{$var}) {
> >  				$camelcase{$var} = 1;
> >  				WARN("CAMELCASE",
> 
> Hi Phil.
> 
> CamelCase was downgraded to a --strict only (CHK)
> test in -next recently.  commit f0e8102413
> ("checkpatch: change CamelCase test and make it --strict")
> 
> I'm hesitant to add a longish whitelist inside
> checkpatch itself, but if it's added, it should
> probably be an array.

Thanks for the response, Joe, sorry I didn't reply earlier.

I agree that a creeping list of exceptions where CamelCase
is to be overlooked would be bad, but I would argue that 
perhaps my exceptions aren't actual CamelCase - they're
(pretending to be) SI units, and just happen to match the
CamelCase regexp. I did a grep for my regexp, and everything
I noticed in a quick scan did look like a justifiable 
variable name.

Of course, it's not a biggie either way, and of course it's
your call, but I do feel my patch has enough merit to be
worth defending.

Cheers,
Phil

Re: [PATCH] checkpatch: forgive use of mixed case variables measuring units

[Joe Perches]

On Wed, 2013-06-12 at 15:59 +0300, Phil Carmody wrote:
> I agree that a creeping list of exceptions where CamelCase
> is to be overlooked would be bad, but I would argue that 
> perhaps my exceptions aren't actual CamelCase - they're
> (pretending to be) SI units, and just happen to match the
> CamelCase regexp. I did a grep for my regexp, and everything
> I noticed in a quick scan did look like a justifiable 
> variable name.

Maybe, but this regex misses variants like:

regulator_min_uA_show

Maybe "^[a-z_]*_[numk][VAS](?:_\w+)?$"

But this regex also does not match on other common
sound variants "_dB", "_mB" and temperature like "_mC".

There's also the arm bL variant (bigLittle)

So I guess it'd be better to use
"^[a-z_]*_[a-z][A-Z](?:_\w+)?$"

Re: [PATCH] checkpatch: forgive use of mixed case variables measuring units

[Jason Cooper]

On Wed, Jun 12, 2013 at 09:02:15AM -0700, Joe Perches wrote:
> On Wed, 2013-06-12 at 15:59 +0300, Phil Carmody wrote:
> > I agree that a creeping list of exceptions where CamelCase
> > is to be overlooked would be bad, but I would argue that 
> > perhaps my exceptions aren't actual CamelCase - they're
> > (pretending to be) SI units, and just happen to match the
> > CamelCase regexp. I did a grep for my regexp, and everything
> > I noticed in a quick scan did look like a justifiable 
> > variable name.
> 
> Maybe, but this regex misses variants like:
> 
> regulator_min_uA_show
> 
> Maybe "^[a-z_]*_[numk][VAS](?:_\w+)?$"
> 
> But this regex also does not match on other common
> sound variants "_dB", "_mB" and temperature like "_mC".

And _dBm ...

thx,

Jason.

Re: [PATCH] checkpatch: forgive use of mixed case variables measuring units

[Joe Perches]

On Wed, 2013-06-12 at 12:17 -0400, Jason Cooper wrote:
> On Wed, Jun 12, 2013 at 09:02:15AM -0700, Joe Perches wrote:
> > On Wed, 2013-06-12 at 15:59 +0300, Phil Carmody wrote:
> > > I agree that a creeping list of exceptions where CamelCase
> > > is to be overlooked would be bad, but I would argue that 
> > > perhaps my exceptions aren't actual CamelCase - they're
> > > (pretending to be) SI units, and just happen to match the
> > > CamelCase regexp. I did a grep for my regexp, and everything
> > > I noticed in a quick scan did look like a justifiable 
> > > variable name.
> > 
> > Maybe, but this regex misses variants like:
> > 
> > regulator_min_uA_show
> > 
> > Maybe "^[a-z_]*_[numk][VAS](?:_\w+)?$"
> > 
> > But this regex also does not match on other common
> > sound variants "_dB", "_mB" and temperature like "_mC".
> 
> And _dBm ...

As far as I can tell, all those uses are either in staging
or are in obsolete/superceded code.

There is one I think ugly variant that could be fixed:

drivers/media/dvb-frontends/stv0367_priv.h:     s32 Power_dBmx10;       /* Power of the RF signal (dBm x 10) */

[PATCH] checkpatch: CamelCase - ignore SI unit variants like "_uV"

[Joe Perches]

Many existing variable names use SI like variants that
should be otherwise obvious and acceptable.

Whitelist them from the CamelCase message.

Suggested-by: Phil Carmody <phil.carmody@partner.samsung.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5989415..5ada911 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3195,7 +3195,10 @@ sub process {
 #CamelCase
 			if ($var !~ /^$Constant$/ &&
 			    $var =~ /[A-Z][a-z]|[a-z][A-Z]/ &&
+#Ignore Page<foo> variants
 			    $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
+#Ignore SI style variants like nS, mV and dB (ie: max_uV, regulator_min_uA_show)
+			    $var !~ /"_[a-z][A-Z](?:_\w+)?$/ &&
 			    !defined $camelcase{$var}) {
 				$camelcase{$var} = 1;
 				CHK("CAMELCASE",
-- 
1.8.1.2.459.gbcd45b4.dirty