Re: [B.A.T.M.A.N.] [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark comments

Re: [B.A.T.M.A.N.] [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark comments

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

On Wednesday, 16 January 2019 11.26.53 CET Thomas Gleixner wrote:
> The SuperH boot code files use a magic format for the SPDX identifier
> comment:
> 
>   LIST "SPDX-License-Identifier: .... "
> 
> The trailing quotation mark is not stripped before the token parser is
> invoked and causes the scan to fail. Handle it gracefully.
[...]

This patch introduces a false positive when checking files with an ANSI-C 
style /* SPDX-License-Identifier: .... */ comment line.

    $ ./scripts/checkpatch.pl -q -f include/linux/bug.h
    WARNING: 'SPDX-License-Identifier: GPL-2.0 */' is not supported in LICENSES/...
    #1: FILE: include/linux/bug.h:1:
    +/* SPDX-License-Identifier: GPL-2.0 */
    
    total: 0 errors, 1 warnings, 79 lines checked

checkpatch.pl is already stripping the "/* " prefix and only sends the 
remaining "SPDX-License-Identifier: GPL-2.0 */" via stdin to
scripts/spdxcheck.py. Thus the newly introduced check

> +                # Remove trailing comment closure
> +                if line.startswith('/*'):
> +                    expr = expr.rstrip('*/').strip()

doesn't match and thus the code doesn't remove the " */" at the end of the 
line.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [B.A.T.M.A.N.] [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark comments

[Joe Perches]

On Sun, 2019-01-20 at 16:32 +0100, Sven Eckelmann wrote:
> On Wednesday, 16 January 2019 11.26.53 CET Thomas Gleixner wrote:
> > The SuperH boot code files use a magic format for the SPDX identifier
> > comment:
> > 
> >   LIST "SPDX-License-Identifier: .... "
> > 
> > The trailing quotation mark is not stripped before the token parser is
> > invoked and causes the scan to fail. Handle it gracefully.
> [...]
> 
> This patch introduces a false positive when checking files with an ANSI-C 
> style /* SPDX-License-Identifier: .... */ comment line.
> 
>     $ ./scripts/checkpatch.pl -q -f include/linux/bug.h
>     WARNING: 'SPDX-License-Identifier: GPL-2.0 */' is not supported in LICENSES/...
>     #1: FILE: include/linux/bug.h:1:
>     +/* SPDX-License-Identifier: GPL-2.0 */
>     
>     total: 0 errors, 1 warnings, 79 lines checked
> 
> checkpatch.pl is already stripping the "/* " prefix and only sends the 
> remaining "SPDX-License-Identifier: GPL-2.0 */" via stdin to
> scripts/spdxcheck.py. Thus the newly introduced check
> 
> > +                # Remove trailing comment closure
> > +                if line.startswith('/*'):
> > +                    expr = expr.rstrip('*/').strip()
> 
> doesn't match and thus the code doesn't remove the " */" at the end of the 
> line.

Well, maybe checkpatch should remove the comment trailer.
---
Miscellanea:

o Indent a block properly too

 scripts/checkpatch.pl | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 155fa9305166..e0b542008256 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3029,8 +3029,10 @@ sub process {
 				$checklicenseline = 2;
 			} elsif ($rawline =~ /^\+/) {
 				my $comment = "";
+				my $trailer = "";
 				if ($realfile =~ /\.(h|s|S)$/) {
 					$comment = '/*';
+					$trailer = '*/';
 				} elsif ($realfile =~ /\.(c|dts|dtsi)$/) {
 					$comment = '//';
 				} elsif (($checklicenseline == 2) || $realfile =~ /\.(sh|pl|py|awk|tc)$/) {
@@ -3044,11 +3046,12 @@ sub process {
 					 WARN("SPDX_LICENSE_TAG",
 					      "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr);
 				} elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) {
-					 my $spdx_license = $1;
-					 if (!is_SPDX_License_valid($spdx_license)) {
-						  WARN("SPDX_LICENSE_TAG",
-						       "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
-					 }
+					my $spdx_license = rtrim($1);
+					$spdx_license =~ s/\s*\Q$trailer\E$//;
+					if (!is_SPDX_License_valid($spdx_license)) {
+						WARN("SPDX_LICENSE_TAG",
+						     "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
+					}
 				}
 			}
 		}

Re: [B.A.T.M.A.N.] [patch 2/2] scripts/spdxcheck.py: Handle special quotation mark comments

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On Sunday, 20 January 2019 19.39.55 CET Joe Perches wrote:
> Well, maybe checkpatch should remove the comment trailer.
> ---
> Miscellanea:

In case you are waiting for me (haven't seen it popping up in linux-next): 
yes, this patch solves the problem for me.

Tested-by: Sven Eckelmann <sven@narfation.org>

Thanks,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]