lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130818100247.GB26728@pd.tnic>
Date:	Sun, 18 Aug 2013 12:02:47 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Joe Perches <joe@...ches.com>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Youquan Song <youquan.song@...ux.intel.com>,
	Yinghai Lu <yinghai@...nel.org>,
	Gleb Natapov <gleb@...hat.com>,
	Youquan Song <youquan.song@...el.com>,
	Sheng Yang <sheng@...ux.intel.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] x86, apic: Enable x2APIC physical when cpu < 256 native

On Sat, Aug 17, 2013 at 09:26:43AM -0700, Joe Perches wrote:
> > ... unless there's a way to detect new submissions and scream only
> > for those. I.e., look at lines starting with "+" which don't have
> > corresponding "-" lines.
> 
> No, that's not the right way to do that.
> That bit's relatively easy because checkpatch only looks
> at files when there's a specific command line -f flag.
> So just check the existence of the -f command line flag
> (!$file) and for an added comment that starts "/* foo" without
> a comment termination */ after it on the same line.

Right, your way is simpler.

> > This would need a bit of experimenting and is not trivial though, maybe
> > Algorithm::Diff could even help there.
> > 
> > Sounds like a mini-project for a perl dude :-)
> 
> I'm not one of those...
> 
> This might work though
> 
>  scripts/checkpatch.pl | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9ba4fc4..abe820a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2119,6 +2119,15 @@ sub process {
>  			}
>  		}
>  
> +		if (!$file &&
> +		    $realfile !~ m@^(drivers/net/|net/)@ &&
> +		    $rawline =~ /^\+/ &&	#Added line
> +		    $rawline !~ m@/\*.*\*/@ &&	#whole comments
> +		    $rawline =~ m@/\*+.+@) {	#unterminated comment
> +			WARN("COMMENT_STYLE",
> +			     "block comments use an empty /* line\n" . $herecurr);
> +		}
> +
>  		if ($realfile =~ m@^(drivers/net/|net/)@ &&
>  		    $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
>  		    $rawline =~ /^\+[ \t]*\*/) {

Almost. It needs to look only on *.[ch] files because otherwise it fires on that
	very same diff to checkpatch.pl too:

$ git diff | ./scripts/checkpatch.pl -

...

WARNING: block comments use an empty /* line
#15: FILE: scripts/checkpatch.pl:2059:
+                            "block comments use an empty /* line\n" . $herecurr);


Also, we need to catch this obvious case:

        /*
         * This is a wrong comment. */


IOW, something like a modified version of yours below. It doesn't catch
every wrong comment form out there but it should be a start.

--
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2ee9eb750560..111b6615d74d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2050,6 +2050,16 @@ sub process {
 			}
 		}
 
+		if (!$file &&
+		    $realfile !~ m@^(drivers/net/|net/)@ &&
+		    $rawline =~ /^\+/ &&		# Added line
+		    $rawline !~ m@/\*.*\*/@ &&		# whole comments
+		    ($rawline =~ m@/\*+.+@ ||		# unterminated opening
+		     $rawline =~ m@\s*\*+.+\*+/@)) {	# unterminated closing
+			WARN("COMMENT_STYLE",
+			     "Multi-line comments use an empty /* opening and */ closing line\n" . $herecurr);
+		}
+
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&
 		    $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
 		    $rawline =~ /^\+[ \t]*\*/) {

--

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ