| Message ID | 20130715162037.16676.9660.stgit@outback |
|---|---|
| State | New |
| Headers | show |
Quoting Tomoki Sekiyama (2013-07-15 11:20:37) > Enable checkpatch.pl to apply the same checks as C source files for > C++ files with .cpp extensions. It also adds some exceptions for C++ > sources to suppress errors for: > - <> used in C++ template arguments (e.g. template <class T>) > - :: used to represent namespaces (e.g. SomeClass::method()) > - : used in class declaration (e.g. class T : public Super) > - ~ used in destructor method name (e.g. T::~T()) > - spacing around 'catch' (e.g. catch (...)) > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> One small nit below about documentation. Looks good otherwise. > --- > scripts/checkpatch.pl | 37 +++++++++++++++++++++++++++++-------- > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index ec0aa4c..0ef72b5 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1363,7 +1363,7 @@ sub process { > # Check for incorrect file permissions > if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { > my $permhere = $here . "FILE: $realfile\n"; > - if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) { > + if ($realfile =~ /(Makefile|Kconfig|\.c|\.cpp|\.h|\.S|\.tmpl)$/) { > ERROR("do not set execute permissions for source files\n" . $permhere); > } > } > @@ -1460,7 +1460,7 @@ sub process { > } > > # check we are in a valid source file if not then ignore this hunk > - next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); > + next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/); > > #80 column limit > if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && > @@ -1495,7 +1495,7 @@ sub process { > } > > # check we are in a valid source file C or perl if not then ignore this hunk > - next if ($realfile !~ /\.(h|c|pl)$/); > + next if ($realfile !~ /\.(h|c|cpp|pl)$/); > > # in QEMU, no tabs are allowed > if ($rawline =~ /^\+.* /) { > @@ -1505,7 +1505,7 @@ sub process { > } > > # check we are in a valid C source file if not then ignore this hunk > - next if ($realfile !~ /\.(h|c)$/); > + next if ($realfile !~ /\.(h|c|cpp)$/); > > # check for RCS/CVS revision markers > if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) { > @@ -1969,6 +1969,9 @@ sub process { > asm|__asm__)$/x) > { > > + # Ignore 'catch (...)' in C++ > + } elsif ($name =~ /^catch$/ && $realfile =~ /(\.cpp|\.h)$/) { > + > # cpp #define statements have non-optional spaces, ie > # if there is a space between the name and the open > # parenthesis it is simply not a parameter group. > @@ -1992,7 +1995,7 @@ sub process { > \+=|-=|\*=|\/=|%=|\^=|\|=|&=| > =>|->|<<|>>|<|>|=|!|~| > &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%| > - \?|: > + \?|::|: > }x; > my @elements = split(/($ops|;)/, $opline); > my $off = 0; > @@ -2066,7 +2069,8 @@ sub process { > # -> > # : when part of a bitfield > } elsif ($op eq '->' || $opv eq ':B') { > - if ($ctx =~ /Wx.|.xW/) { > + if ($ctx =~ /Wx.|.xW/ && > + !($opv eq ':B' && $line =~ /class/)) { Everything else seemed fairly obvious or was well commented, but I couldn't figure out what this change was for. Could you insert a small comment to explain? > ERROR("spaces prohibited around that '$op' $at\n" . $hereptr); > } > > @@ -2088,7 +2092,11 @@ sub process { > } elsif ($op eq '!' || $op eq '~' || > $opv eq '*U' || $opv eq '-U' || > $opv eq '&U' || $opv eq '&&U') { > - if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { > + if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cpp|\.h)$/) { > + # '~' used as a name of Destructor > + > + } > + elsif ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { > ERROR("space required before that '$op' $at\n" . $hereptr); > } > if ($op eq '*' && $cc =~/\s*$Modifier\b/) { > @@ -2126,8 +2134,9 @@ sub process { > > # A colon needs no spaces before when it is > # terminating a case value or a label. > + # Ignored if it is used in class declaration in C++. > } elsif ($opv eq ':C' || $opv eq ':L') { > - if ($ctx =~ /Wx./) { > + if ($ctx =~ /Wx./ && $line !~ /class/) { > ERROR("space prohibited before that '$op' $at\n" . $hereptr); > } > > @@ -2135,6 +2144,18 @@ sub process { > } elsif ($ctx !~ /[EWC]x[CWE]/) { > my $ok = 0; > > + if ($realfile =~ /\.cpp|\.h$/) { > + # Ignore template arguments <...> in C++ > + if (($op eq '<' || $op eq '>') && $line =~ /<.*>/) { > + $ok = 1; > + } > + > + # Ignore :: in C++ > + if ($op eq '::') { > + $ok = 1; > + } > + } > + > # Ignore email addresses <foo@bar> > if (($op eq '<' && > $cc =~ /^\S+\@\S+>/) ||
On 7/22/13 17:21 , "Michael Roth" <mdroth@linux.vnet.ibm.com> wrote: >Quoting Tomoki Sekiyama (2013-07-15 11:20:37) >> Enable checkpatch.pl to apply the same checks as C source files for >> C++ files with .cpp extensions. It also adds some exceptions for C++ >> sources to suppress errors for: >> - <> used in C++ template arguments (e.g. template <class T>) >> - :: used to represent namespaces (e.g. SomeClass::method()) >> - : used in class declaration (e.g. class T : public Super) >> - ~ used in destructor method name (e.g. T::~T()) >> - spacing around 'catch' (e.g. catch (...)) >> >> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> > >One small nit below about documentation. Looks good otherwise. <snip> >>@@ -2066,7 +2069,8 @@ sub process { >> # -> >> # : when part of a bitfield >> } elsif ($op eq '->' || $opv eq ':B') { >> - if ($ctx =~ /Wx.|.xW/) { >> + if ($ctx =~ /Wx.|.xW/ && >> + !($opv eq ':B' && $line >>=~ /class/)) { > >Everything else seemed fairly obvious or was well commented, but I >couldn't >figure out what this change was for. Could you insert a small comment to >explain? Sure. This is suppressing errors for : used in class declaration. I will do some cleanups for this. Thanks, Tomoki Sekiyama
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index ec0aa4c..0ef72b5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1363,7 +1363,7 @@ sub process { # Check for incorrect file permissions if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { my $permhere = $here . "FILE: $realfile\n"; - if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) { + if ($realfile =~ /(Makefile|Kconfig|\.c|\.cpp|\.h|\.S|\.tmpl)$/) { ERROR("do not set execute permissions for source files\n" . $permhere); } } @@ -1460,7 +1460,7 @@ sub process { } # check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); + next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/); #80 column limit if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && @@ -1495,7 +1495,7 @@ sub process { } # check we are in a valid source file C or perl if not then ignore this hunk - next if ($realfile !~ /\.(h|c|pl)$/); + next if ($realfile !~ /\.(h|c|cpp|pl)$/); # in QEMU, no tabs are allowed if ($rawline =~ /^\+.*\t/) { @@ -1505,7 +1505,7 @@ sub process { } # check we are in a valid C source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c)$/); + next if ($realfile !~ /\.(h|c|cpp)$/); # check for RCS/CVS revision markers if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) { @@ -1969,6 +1969,9 @@ sub process { asm|__asm__)$/x) { + # Ignore 'catch (...)' in C++ + } elsif ($name =~ /^catch$/ && $realfile =~ /(\.cpp|\.h)$/) { + # cpp #define statements have non-optional spaces, ie # if there is a space between the name and the open # parenthesis it is simply not a parameter group. @@ -1992,7 +1995,7 @@ sub process { \+=|-=|\*=|\/=|%=|\^=|\|=|&=| =>|->|<<|>>|<|>|=|!|~| &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%| - \?|: + \?|::|: }x; my @elements = split(/($ops|;)/, $opline); my $off = 0; @@ -2066,7 +2069,8 @@ sub process { # -> # : when part of a bitfield } elsif ($op eq '->' || $opv eq ':B') { - if ($ctx =~ /Wx.|.xW/) { + if ($ctx =~ /Wx.|.xW/ && + !($opv eq ':B' && $line =~ /class/)) { ERROR("spaces prohibited around that '$op' $at\n" . $hereptr); } @@ -2088,7 +2092,11 @@ sub process { } elsif ($op eq '!' || $op eq '~' || $opv eq '*U' || $opv eq '-U' || $opv eq '&U' || $opv eq '&&U') { - if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { + if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cpp|\.h)$/) { + # '~' used as a name of Destructor + + } + elsif ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { ERROR("space required before that '$op' $at\n" . $hereptr); } if ($op eq '*' && $cc =~/\s*$Modifier\b/) { @@ -2126,8 +2134,9 @@ sub process { # A colon needs no spaces before when it is # terminating a case value or a label. + # Ignored if it is used in class declaration in C++. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./) { + if ($ctx =~ /Wx./ && $line !~ /class/) { ERROR("space prohibited before that '$op' $at\n" . $hereptr); } @@ -2135,6 +2144,18 @@ sub process { } elsif ($ctx !~ /[EWC]x[CWE]/) { my $ok = 0; + if ($realfile =~ /\.cpp|\.h$/) { + # Ignore template arguments <...> in C++ + if (($op eq '<' || $op eq '>') && $line =~ /<.*>/) { + $ok = 1; + } + + # Ignore :: in C++ + if ($op eq '::') { + $ok = 1; + } + } + # Ignore email addresses <foo@bar> if (($op eq '<' && $cc =~ /^\S+\@\S+>/) ||
Enable checkpatch.pl to apply the same checks as C source files for C++ files with .cpp extensions. It also adds some exceptions for C++ sources to suppress errors for: - <> used in C++ template arguments (e.g. template <class T>) - :: used to represent namespaces (e.g. SomeClass::method()) - : used in class declaration (e.g. class T : public Super) - ~ used in destructor method name (e.g. T::~T()) - spacing around 'catch' (e.g. catch (...)) Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- scripts/checkpatch.pl | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-)