diff mbox

checkpatch: Check *.cc files and allow C99 comments for C++ code

Message ID 1402155757-24977-1-git-send-email-sw@weilnetz.de
State Under Review
Headers show

Commit Message

Stefan Weil June 7, 2014, 3:42 p.m. UTC 
QEMU uses both *.cc and *.cpp for some files with C++ code:

disas/libvixl/utils.cc
disas/libvixl/a64/disasm-a64.cc
disas/libvixl/a64/instructions-a64.cc
disas/libvixl/a64/decoder-a64.cc
disas/arm-a64.cc
qga/vss-win32/provider.cpp
qga/vss-win32/install.cpp
qga/vss-win32/requester.cpp

*.cpp files were already checked. Add the cc pattern, so *.cc are checked
now, too.

Comments with // are quite common in C++ code, so don't forbid them.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

The current *.cc files now trigger several warnings and errors,
but that's not a good reason why they should be suppressed.

Regards,
Stefan Weil

 scripts/checkpatch.pl |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Peter Maydell June 7, 2014, 3:58 p.m. UTC | #1 
On 7 June 2014 16:42, Stefan Weil <sw@weilnetz.de> wrote:
> QEMU uses both *.cc and *.cpp for some files with C++ code:
>
> disas/libvixl/utils.cc
> disas/libvixl/a64/disasm-a64.cc
> disas/libvixl/a64/instructions-a64.cc
> disas/libvixl/a64/decoder-a64.cc
> disas/arm-a64.cc
> qga/vss-win32/provider.cpp
> qga/vss-win32/install.cpp
> qga/vss-win32/requester.cpp
>
> *.cpp files were already checked. Add the cc pattern, so *.cc are checked
> now, too.

Note that there's not much point in checking libvixl's .cc files,
because we're only going to change these for (a) updates
to new upstream versions and (b) fixes for serious issues
that can't wait for a new upstream version; they're never
going to match our coding style.

thanks
-- PMM
Stefan Weil June 7, 2014, 4:10 p.m. UTC | #2 
Am 07.06.2014 17:58, schrieb Peter Maydell:
> On 7 June 2014 16:42, Stefan Weil <sw@weilnetz.de> wrote:
>> QEMU uses both *.cc and *.cpp for some files with C++ code:
>>
>> disas/libvixl/utils.cc
>> disas/libvixl/a64/disasm-a64.cc
>> disas/libvixl/a64/instructions-a64.cc
>> disas/libvixl/a64/decoder-a64.cc
>> disas/arm-a64.cc
>> qga/vss-win32/provider.cpp
>> qga/vss-win32/install.cpp
>> qga/vss-win32/requester.cpp
>>
>> *.cpp files were already checked. Add the cc pattern, so *.cc are checked
>> now, too.
> 
> Note that there's not much point in checking libvixl's .cc files,
> because we're only going to change these for (a) updates
> to new upstream versions and (b) fixes for serious issues
> that can't wait for a new upstream version; they're never
> going to match our coding style.
> 
> thanks
> -- PMM
> 

disas/arm-a64.cc is purely QEMU code and should use the QEMU coding
style. For Linux header files, libvixl and other imported code, people
usually know how they have to handle them. My own patch also fails when
it is checked with checkpatch.pl: it uses tabs, and some lines are too
long. It's also imported code...

Stefan
diff mbox

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9d46e5a..f29bdbe 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|\.cpp|\.h|\.S|\.tmpl)$/) {
+			if ($realfile =~ /(Makefile|Kconfig|\.c|\.cc|\.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|cpp|s|S|pl|sh)$/);
+		next if ($realfile !~ /\.(h|c|cc|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|cpp|pl)$/);
+		next if ($realfile !~ /\.(h|c|cc|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|cpp)$/);
+		next if ($realfile !~ /\.(h|c|cc|cpp)$/);
 
 # check for RCS/CVS revision markers
 		if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
@@ -1792,7 +1792,7 @@  sub process {
 		}
 
 # no C99 // comments
-		if ($line =~ m{//}) {
+		if (($line =~ m{//}) && !($realfile =~ /(\.cc|\.cpp)$/)) {
 			ERROR("do not use C99 // comments\n" . $herecurr);
 		}
 		# Remove C99 comments.
@@ -1970,7 +1970,7 @@  sub process {
 			{
 
 			# Ignore 'catch (...)' in C++
-			} elsif ($name =~ /^catch$/ && $realfile =~ /(\.cpp|\.h)$/) {
+			} elsif ($name =~ /^catch$/ && $realfile =~ /(\.cc|\.cpp|\.h)$/) {
 
 			# cpp #define statements have non-optional spaces, ie
 			# if there is a space between the name and the open
@@ -2067,7 +2067,7 @@  sub process {
 
 				# Ignore : used in class declaration in C++
 				} elsif ($opv eq ':B' && $ctx =~ /Wx[WE]/ &&
-						 $line =~ /class/ && $realfile =~ /(\.cpp|\.h)$/) {
+						 $line =~ /class/ && $realfile =~ /(\.cc|\.cpp|\.h)$/) {
 
 				# No spaces for:
 				#   ->
@@ -2095,7 +2095,7 @@  sub process {
 				} elsif ($op eq '!' || $op eq '~' ||
 					 $opv eq '*U' || $opv eq '-U' ||
 					 $opv eq '&U' || $opv eq '&&U') {
-					if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cpp|\.h)$/) {
+					if ($op eq '~' && $ca =~ /::$/ && $realfile =~ /(\.cc|\.cpp|\.h)$/) {
 						# '~' used as a name of Destructor
 
 					} elsif ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
@@ -2145,7 +2145,7 @@  sub process {
 				} elsif ($ctx !~ /[EWC]x[CWE]/) {
 					my $ok = 0;
 
-					if ($realfile =~ /\.cpp|\.h$/) {
+					if ($realfile =~ /\.cc|\.cpp|\.h$/) {
 						# Ignore template arguments <...> in C++
 						if (($op eq '<' || $op eq '>') && $line =~ /<.*>/) {
 							$ok = 1;