Re: Checking for space before ending comment tag

Re: Checking for space before ending comment tag

[Joe Perches]

On Sat, 2022-03-19 at 08:35 +0100, Paul Menzel wrote:
> Dear Andy, dear Joe,
> 
> The attached patch misses a space before the closing comment tag `*/`:
> 
>      /* Check for FEC status*/
> 
>      $ git describe origin/master
>      v5.17-rc8-61-g34e047aa16c0
>      $ scripts/checkpatch.pl 
> /dev/shm/0001-drm-amd-display-FEC-check-in-timing-validation.patch
>      total: 0 errors, 0 warnings, 10 lines checked
> 
>      /dev/shm/0001-drm-amd-display-FEC-check-in-timing-validation.patch 
> has no obvious style problems and is ready for submission.
> 
> Should/could checkpatch.pl warn about that?

It _could_, but there are a large number of existing uses in the kernel.

$ git grep -P '\w\*\/\s*$' -- '*.[ch]' | wc -l
15841

Even when you exclude the uses that start with /*\w

$ git grep -P '\w\*\/\s*$' -- '*.[ch]' | grep -v -P '/\*\w' | wc -l
7239

btw: that doesn't count the thousands of lines like

/*----*/

Anyway, I prefer not to add tests for things that have that many uses.

cheers, Joe

Anyway, maybe a test could be something like:
---
 scripts/checkpatch.pl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 577e029987011..65800e6964d1e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3868,6 +3868,14 @@ sub process {
 			}
 		}
 
+# Single line comment style
+		if ($line =~ /^\+.*$;\s*$/ &&
+		    $rawline =~ /\/\* / &&
+		    $rawline =~ /\w\*\/\s*$/) {
+			WARN("COMMENT_STYLE",
+			     "Single line comments should use a space before the comment termination\n" . $herecurr);
+		}
+
 # Block comment styles
 # Networking with an initial /*
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&

Re: Checking for space before ending comment tag

[Joe Perches]

Maybe this is better as it can fix them too.

---
 scripts/checkpatch.pl | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 577e029987011..70afa4ff2addf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3868,6 +3868,22 @@ sub process {
 			}
 		}
 
+# Single line comment style - warn when '/*comment */' or '/* comment*/'
+		if ($line =~ /^\+.*$;/ &&
+		    $rawline =~ m@/\*.+\*/@ &&
+		    (($rawline =~ m@/\* @  && $rawline =~ m@\w\*/@) ||
+		     ($rawline =~ m@/\*\w@ && $rawline =~ m@ \*/@))) {
+			if (WARN("COMMENT_STYLE",
+				 "Single line comments should use a space after /* and before */\n" . $herecurr) &&
+			    $fix) {
+				$rawline =~ m@/\*.+\*/@;
+				my $comment = substr($rawline, $-[0], $+[0] - $-[0]);
+				my $newcomment = trim(substr($comment, 2, -2));
+				$fixed[$fixlinenr] =~
+				    s@\Q$comment\E@/\* $newcomment \*/@;
+			}
+		}
+
 # Block comment styles
 # Networking with an initial /*
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&