From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1NXn-0005Nk-O6 for qemu-devel@nongnu.org; Mon, 22 Jul 2013 17:21:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1NXl-0005O3-F8 for qemu-devel@nongnu.org; Mon, 22 Jul 2013 17:21:23 -0400 Received: from mail-ie0-x229.google.com ([2607:f8b0:4001:c03::229]:61161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1NXl-0005N3-9g for qemu-devel@nongnu.org; Mon, 22 Jul 2013 17:21:21 -0400 Received: by mail-ie0-f169.google.com with SMTP id at20so5705121iec.28 for ; Mon, 22 Jul 2013 14:21:20 -0700 (PDT) Sender: fluxion Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20130715162037.16676.9660.stgit@outback> References: <20130715162023.16676.87828.stgit@outback> <20130715162037.16676.9660.stgit@outback> Message-ID: <20130722212117.16294.36600@loki> Date: Mon, 22 Jul 2013 16:21:17 -0500 Subject: Re: [Qemu-devel] [PATCH v7 03/10] checkpatch.pl: Check .cpp files List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tomoki Sekiyama , qemu-devel@nongnu.org Cc: libaiqing@huawei.com, ghammer@redhat.com, stefanha@gmail.com, lcapitulino@redhat.com, vrozenfe@redhat.com, pbonzini@redhat.com, seiji.aguchi@hds.com, lersek@redhat.com, areis@redhat.com 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 ) > - :: 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 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 =3D~ /^new (file )?mode.*[7531]\d{0,2}$/) { > my $permhere =3D $here . "FILE: $realfile\n"; > - if ($realfile =3D~ /(Makefile|Kconfig|\.c|\.h|\.S= |\.tmpl)$/) { > + if ($realfile =3D~ /(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 =3D~ /^\+/ && $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 =3D~ /^\+.* /) { > @@ -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 =3D~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) { > @@ -1969,6 +1969,9 @@ sub process { > asm|__asm__)$/x) > { > = > + # Ignore 'catch (...)' in C++ > + } elsif ($name =3D~ /^catch$/ && $realfile =3D~ /= (\.cpp|\.h)$/) { > + > # cpp #define statements have non-optional spaces= , ie > # if there is a space between the name and the op= en > # parenthesis it is simply not a parameter group. > @@ -1992,7 +1995,7 @@ sub process { > \+=3D|-=3D|\*=3D|\/=3D|%=3D|\^=3D|\|=3D|&= =3D| > =3D>|->|<<|>>|<|>|=3D|!|~| > &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%| > - \?|: > + \?|::|: > }x; > my @elements =3D split(/($ops|;)/, $opline); > my $off =3D 0; > @@ -2066,7 +2069,8 @@ sub process { > # -> > # : when part of a bitfield > } elsif ($op eq '->' || $opv eq ':B') { > - if ($ctx =3D~ /Wx.|.xW/) { > + if ($ctx =3D~ /Wx.|.xW/ && > + !($opv eq ':B' && $line = =3D~ /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 =3D~ /::$/ = && $realfile =3D~ /(\.cpp|\.h)$/) { > + # '~' used as a name of D= estructor > + > + } > + elsif ($ctx !~ /[WEBC]x./ && $ca = !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { > ERROR("space required bef= ore that '$op' $at\n" . $hereptr); > } > if ($op eq '*' && $cc =3D~/\s*$Mo= difier\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 declarat= ion in C++. > } elsif ($opv eq ':C' || $opv eq ':L') { > - if ($ctx =3D~ /Wx./) { > + if ($ctx =3D~ /Wx./ && $line !~ /= class/) { > ERROR("space prohibited b= efore that '$op' $at\n" . $hereptr); > } > = > @@ -2135,6 +2144,18 @@ sub process { > } elsif ($ctx !~ /[EWC]x[CWE]/) { > my $ok =3D 0; > = > + if ($realfile =3D~ /\.cpp|\.h$/) { > + # Ignore template argumen= ts <...> in C++ > + if (($op eq '<' || $op eq= '>') && $line =3D~ /<.*>/) { > + $ok =3D 1; > + } > + > + # Ignore :: in C++ > + if ($op eq '::') { > + $ok =3D 1; > + } > + } > + > # Ignore email addresses > if (($op eq '<' && > $cc =3D~ /^\S+\@\S+>/) ||