2020-03-11 19:13:49

by Scott Branden

[permalink] [raw]
Subject: [PATCH] checkpatch: always allow C99 SPDX License Identifer comments

Always allow C99 comment styles if SPDK-License-Identifier is in comment
even if C99_COMMENT_TOLERANCE is specified in the --ignore options.

Signed-off-by: Scott Branden <[email protected]>
---
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a63380c6b0d2..c8b429dd6b51 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3852,8 +3852,8 @@ sub process {
}
}

-# no C99 // comments
- if ($line =~ m{//}) {
+# no C99 // comments except for SPDX-License-Identifier
+ if ($line =~ m{//} && $rawline !~ /SPDX-License-Identifier:/) {
if (ERROR("C99_COMMENTS",
"do not use C99 // comments\n" . $herecurr) &&
$fix) {
--
2.17.1


2020-03-12 02:29:01

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: always allow C99 SPDX License Identifer comments

On Wed, 2020-03-11 at 12:11 -0700, Scott Branden wrote:
> Always allow C99 comment styles if SPDK-License-Identifier is in comment
> even if C99_COMMENT_TOLERANCE is specified in the --ignore options.

Why is this useful?

> Signed-off-by: Scott Branden <[email protected]>
> ---
> scripts/checkpatch.pl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index a63380c6b0d2..c8b429dd6b51 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3852,8 +3852,8 @@ sub process {
> }
> }
>
> -# no C99 // comments
> - if ($line =~ m{//}) {
> +# no C99 // comments except for SPDX-License-Identifier
> + if ($line =~ m{//} && $rawline !~ /SPDX-License-Identifier:/) {
> if (ERROR("C99_COMMENTS",
> "do not use C99 // comments\n" . $herecurr) &&
> $fix) {

2020-03-12 02:50:06

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: always allow C99 SPDX License Identifer comments

Hi Joe,

On 2020-03-11 7:26 p.m., Joe Perches wrote:
> On Wed, 2020-03-11 at 12:11 -0700, Scott Branden wrote:
>> Always allow C99 comment styles if SPDK-License-Identifier is in comment
>> even if C99_COMMENT_TOLERANCE is specified in the --ignore options.
> Why is this useful?
This is useful because if you run checkpatch with
--ignore=C99_COMMENT_TOLERANCE
right now it will warn on almost every .c file in the linux kernel due
to the decision to
use // SPDX-License-Identifier: at the start of every c file

With this change checkpatch will stop complaining about this single
outlier // in the file
and allow you to enforce no other C99 // style comments in the patch.

It would have been a lot nicer if /* SPDX-License-Identifier: xxxx */
was used instead...
>
>> Signed-off-by: Scott Branden <[email protected]>
>> ---
>> scripts/checkpatch.pl | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index a63380c6b0d2..c8b429dd6b51 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -3852,8 +3852,8 @@ sub process {
>> }
>> }
>>
>> -# no C99 // comments
>> - if ($line =~ m{//}) {
>> +# no C99 // comments except for SPDX-License-Identifier
>> + if ($line =~ m{//} && $rawline !~ /SPDX-License-Identifier:/) {
>> if (ERROR("C99_COMMENTS",
>> "do not use C99 // comments\n" . $herecurr) &&
>> $fix) {

2020-03-12 04:45:01

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: always allow C99 SPDX License Identifer comments

On Wed, 2020-03-11 at 19:48 -0700, Scott Branden wrote:
> Hi Joe,
>
> On 2020-03-11 7:26 p.m., Joe Perches wrote:
> > On Wed, 2020-03-11 at 12:11 -0700, Scott Branden wrote:
> > > Always allow C99 comment styles if SPDK-License-Identifier is in comment
> > > even if C99_COMMENT_TOLERANCE is specified in the --ignore options.
> > Why is this useful?
> This is useful because if you run checkpatch with
> --ignore=C99_COMMENT_TOLERANCE
> right now it will warn on almost every .c file in the linux kernel due
> to the decision to
> use // SPDX-License-Identifier: at the start of every c file

Maybe this is better:

Just don't perform any other per-line checks on a valid or invalid
SPDX line.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 529c892..3f2ae7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3171,6 +3171,7 @@ sub process {
WARN("SPDX_LICENSE_TAG",
"'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
}
+ next;
}
}
}


2020-03-13 02:48:27

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: always allow C99 SPDX License Identifer comments

Hi Joe,

On 2020-03-11 9:42 p.m., Joe Perches wrote:
> On Wed, 2020-03-11 at 19:48 -0700, Scott Branden wrote:
>> Hi Joe,
>>
>> On 2020-03-11 7:26 p.m., Joe Perches wrote:
>>> On Wed, 2020-03-11 at 12:11 -0700, Scott Branden wrote:
>>>> Always allow C99 comment styles if SPDK-License-Identifier is in comment
>>>> even if C99_COMMENT_TOLERANCE is specified in the --ignore options.
>>> Why is this useful?
>> This is useful because if you run checkpatch with
>> --ignore=C99_COMMENT_TOLERANCE
>> right now it will warn on almost every .c file in the linux kernel due
>> to the decision to
>> use // SPDX-License-Identifier: at the start of every c file
> Maybe this is better:
>
> Just don't perform any other per-line checks on a valid or invalid
> SPDX line.
I tried your change and it works as well.
Probably better/simpler if no other processing is needed on lines with
SPDX-License-Indentifier: on it.

Would you like to just submit your patch or do I need to construct
something?
> ---
> scripts/checkpatch.pl | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 529c892..3f2ae7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3171,6 +3171,7 @@ sub process {
> WARN("SPDX_LICENSE_TAG",
> "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
> }
> + next;
> }
> }
> }
>
>

2020-03-13 08:25:43

by Joe Perches

[permalink] [raw]
Subject: [PATCH] checkpatch: No additional checking of SPDX License Identifier necessary

If checkpatch is run with --ignore=C99_COMMENT_TOLERANCE it
will warn on almost every .c file because linux kernel style
requires use of a C99 comment // SPDX-License-Identifier:

checkpatch does not need to do any additional per-line checking
after checking the SPDX-License-Identifier line.

This allows the C99 comment style for SPDK-License-Identifier
even if C99_COMMENT_TOLERANCE is specified by an --ignore option.

Original-patch-by: Scott Branden <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 529c892..3f2ae7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3171,6 +3171,7 @@ sub process {
WARN("SPDX_LICENSE_TAG",
"'$spdx_license' is not supported in LICENSES/...\n" . $herecurr);
}
+ next;
}
}
}