* Re: [v4:No,Change] xHCI:Fixing xhci_readl definition and function call
[not found] <5223383f.hCMXrpqEq1AlVw9b%wangshilong1991@gmail.com>
@ 2013-09-03 15:58 ` Sarah Sharp
2013-09-03 17:25 ` [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Sarah Sharp @ 2013-09-03 15:58 UTC (permalink / raw)
To: Wang Shilong; +Cc: kumargauravgupta3, kernel-janitors, linux-kernel
On Sun, Sep 01, 2013 at 08:51:11PM +0800, Wang Shilong wrote:
> Hello, Using checkpatch.pl, i get the following warnings(errors):
> WARNING: Avoid CamelCase: <port_array[wIndex]>
> #272: FILE: drivers/usb/host/xhci-hub.c:725:
> + temp = xhci_readl(port_array[wIndex]);
>
> total: 0 errors, 1 warnings, 826 lines checked
>
> patch has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
Ignore this warning. The variable is called wIndex because it
corresponds to a field in a USB control transfer, and that's what it's
called in the spec. In this case, wIndex is the index into the hub's
port array, so it's perfectly valid to use it as an array index here.
In general, if checkpatch.pl complains about a variable a patch
introduces that's CamelCase, you should pay attention to it. If it's a
variable that already present in USB code, it's probably used to match
the spec.
Sarah Sharp
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch
2013-09-03 15:58 ` [v4:No,Change] xHCI:Fixing xhci_readl definition and function call Sarah Sharp
@ 2013-09-03 17:25 ` Joe Perches
2013-09-04 15:58 ` Sarah Sharp
0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2013-09-03 17:25 UTC (permalink / raw)
To: Andrew Morton
Cc: Sarah Sharp, Wang Shilong, kumargauravgupta3, kernel-janitors,
linux-kernel
Extend the CamelCase words found to include structure members.
In https://lkml.org/lkml/2013/9/3/318 Sarah Sharp (mostly) wrote:
"In general, if checkpatch.pl complains about a variable a patch
introduces that's CamelCase, you should pay attention to it.
Otherwise, [] ignore it."
So, if checking a patch, scan the original patched file if it's
available and add any preexisting CamelCase types so reuses do
not generate CamelCase messages.
That also means Andrew's not so cruelly spurned anymore.
https://lkml.org/lkml/2013/2/22/426
Signed-off-by: Joe Perches <joe@perches.com>
Suggested-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
---
scripts/checkpatch.pl | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9ba4fc4..b98a996 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -424,7 +424,7 @@ sub seed_camelcase_file {
if ($line =~ /^[ \t]*(?:#[ \t]*define|typedef\s+$Type)\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)/) {
$camelcase{$1} = 1;
}
- elsif ($line =~ /^\s*$Declare\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)\s*\(/) {
+ elsif ($line =~ /^\s*$Declare\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)\s*[\(\[,;]/) {
$camelcase{$1} = 1;
}
}
@@ -1593,6 +1593,8 @@ sub process {
my @setup_docs = ();
my $setup_docs = 0;
+ my $camelcase_file_seeded = 0;
+
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
@@ -3377,7 +3379,13 @@ sub process {
while ($var =~ m{($Ident)}g) {
my $word = $1;
next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/);
- seed_camelcase_includes() if ($check);
+ if ($check) {
+ seed_camelcase_includes();
+ if (!$file && !$camelcase_file_seeded) {
+ seed_camelcase_file($realfile);
+ $camelcase_file_seeded = 1;
+ }
+ }
if (!defined $camelcase{$word}) {
$camelcase{$word} = 1;
CHK("CAMELCASE",
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch
2013-09-03 17:25 ` [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch Joe Perches
@ 2013-09-04 15:58 ` Sarah Sharp
2013-09-04 23:08 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Sarah Sharp @ 2013-09-04 15:58 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Wang Shilong, kumargauravgupta3, kernel-janitors,
linux-kernel
On Tue, Sep 03, 2013 at 10:25:21AM -0700, Joe Perches wrote:
> Extend the CamelCase words found to include structure members.
>
> In https://lkml.org/lkml/2013/9/3/318 Sarah Sharp (mostly) wrote:
>
> "In general, if checkpatch.pl complains about a variable a patch
> introduces that's CamelCase, you should pay attention to it.
> Otherwise, [] ignore it."
>
> So, if checking a patch, scan the original patched file if it's
> available and add any preexisting CamelCase types so reuses do
> not generate CamelCase messages.
>
> That also means Andrew's not so cruelly spurned anymore.
> https://lkml.org/lkml/2013/2/22/426
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Suggested-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Thanks! Will this mean checkpatch.pl still complains on CamelCase names
if it's run against a file? I think that's still valuable.
Sarah Sharp
> ---
> scripts/checkpatch.pl | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9ba4fc4..b98a996 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -424,7 +424,7 @@ sub seed_camelcase_file {
> if ($line =~ /^[ \t]*(?:#[ \t]*define|typedef\s+$Type)\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)/) {
> $camelcase{$1} = 1;
> }
> - elsif ($line =~ /^\s*$Declare\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)\s*\(/) {
> + elsif ($line =~ /^\s*$Declare\s+(\w*(?:[A-Z][a-z]|[a-z][A-Z])\w*)\s*[\(\[,;]/) {
> $camelcase{$1} = 1;
> }
> }
> @@ -1593,6 +1593,8 @@ sub process {
> my @setup_docs = ();
> my $setup_docs = 0;
>
> + my $camelcase_file_seeded = 0;
> +
> sanitise_line_reset();
> my $line;
> foreach my $rawline (@rawlines) {
> @@ -3377,7 +3379,13 @@ sub process {
> while ($var =~ m{($Ident)}g) {
> my $word = $1;
> next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/);
> - seed_camelcase_includes() if ($check);
> + if ($check) {
> + seed_camelcase_includes();
> + if (!$file && !$camelcase_file_seeded) {
> + seed_camelcase_file($realfile);
> + $camelcase_file_seeded = 1;
> + }
> + }
> if (!defined $camelcase{$word}) {
> $camelcase{$word} = 1;
> CHK("CAMELCASE",
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch
2013-09-04 15:58 ` Sarah Sharp
@ 2013-09-04 23:08 ` Joe Perches
2013-09-05 17:45 ` Sarah Sharp
0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2013-09-04 23:08 UTC (permalink / raw)
To: Sarah Sharp
Cc: Andrew Morton, Wang Shilong, kumargauravgupta3, kernel-janitors,
linux-kernel
(sending for 3rd time, odd dns problems today, apologies for dupes)
On Wed, 2013-09-04 at 08:58 -0700, Sarah Sharp wrote:
> On Tue, Sep 03, 2013 at 10:25:21AM -0700, Joe Perches wrote:
> > Extend the CamelCase words found to include structure members.
> >
> > In https://lkml.org/lkml/2013/9/3/318 Sarah Sharp (mostly) wrote:
> >
> > "In general, if checkpatch.pl complains about a variable a patch
> > introduces that's CamelCase, you should pay attention to it.
> > Otherwise, [] ignore it."
> >
> > So, if checking a patch, scan the original patched file if it's
> > available and add any preexisting CamelCase types so reuses do
> > not generate CamelCase messages.
[]
> Thanks! Will this mean checkpatch.pl still complains on CamelCase names
> if it's run against a file? I think that's still valuable.
Yes.
First, checkpatch looks for all existing CamelCase #defines,
typedefs, function names and struct/union members in the
include path. (it uses regexes so it's actually not at all
close to even good at finding those).
It stores all those CamelCase uses in a hash.
If checkpatch is scanning a patch, it'll now read the file
being patched for existing uses of CamelCase #defines, etc,
and checkpatch adds those uses to the hash.
If checkpatch is scanning a file, it doesn't doesn't
prescan the file.
Then, when checkpatch scans the patch or file and finds a
CamelCase use, it looks for that use in the hash and is
silent if it's there, noisy otherwise.
This can still report CamelCase uses in a patch if say a
CamelCase type is defined in a .h file in the same directory
or some other include path and that word is not already used
by the file.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch
2013-09-04 23:08 ` Joe Perches
@ 2013-09-05 17:45 ` Sarah Sharp
0 siblings, 0 replies; 5+ messages in thread
From: Sarah Sharp @ 2013-09-05 17:45 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Wang Shilong, kumargauravgupta3, kernel-janitors,
linux-kernel
On Wed, Sep 04, 2013 at 04:08:06PM -0700, Joe Perches wrote:
> (sending for 3rd time, odd dns problems today, apologies for dupes)
>
> On Wed, 2013-09-04 at 08:58 -0700, Sarah Sharp wrote:
> > On Tue, Sep 03, 2013 at 10:25:21AM -0700, Joe Perches wrote:
> > > Extend the CamelCase words found to include structure members.
> > >
> > > In https://lkml.org/lkml/2013/9/3/318 Sarah Sharp (mostly) wrote:
> > >
> > > "In general, if checkpatch.pl complains about a variable a patch
> > > introduces that's CamelCase, you should pay attention to it.
> > > Otherwise, [] ignore it."
> > >
> > > So, if checking a patch, scan the original patched file if it's
> > > available and add any preexisting CamelCase types so reuses do
> > > not generate CamelCase messages.
> []
> > Thanks! Will this mean checkpatch.pl still complains on CamelCase names
> > if it's run against a file? I think that's still valuable.
>
> Yes.
>
> First, checkpatch looks for all existing CamelCase #defines,
> typedefs, function names and struct/union members in the
> include path. (it uses regexes so it's actually not at all
> close to even good at finding those).
>
> It stores all those CamelCase uses in a hash.
>
> If checkpatch is scanning a patch, it'll now read the file
> being patched for existing uses of CamelCase #defines, etc,
> and checkpatch adds those uses to the hash.
>
> If checkpatch is scanning a file, it doesn't doesn't
> prescan the file.
>
> Then, when checkpatch scans the patch or file and finds a
> CamelCase use, it looks for that use in the hash and is
> silent if it's there, noisy otherwise.
>
> This can still report CamelCase uses in a patch if say a
> CamelCase type is defined in a .h file in the same directory
> or some other include path and that word is not already used
> by the file.
Great! Thanks for doing this Joe.
Sarah Sharp
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-05 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <5223383f.hCMXrpqEq1AlVw9b%wangshilong1991@gmail.com>
2013-09-03 15:58 ` [v4:No,Change] xHCI:Fixing xhci_readl definition and function call Sarah Sharp
2013-09-03 17:25 ` [PATCH] checkpatch: Extend CamelCase types and ignore existing CamelCase uses in a patch Joe Perches
2013-09-04 15:58 ` Sarah Sharp
2013-09-04 23:08 ` Joe Perches
2013-09-05 17:45 ` Sarah Sharp
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).