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 <[email protected]>
---
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
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.
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
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+)?$"
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.
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 <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
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
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) */