* [PATCH] checkpatch: validate MODULE_LICENSE content
@ 2015-04-06 18:43 Bjorn Andersson
2015-04-07 0:17 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2015-04-06 18:43 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel
There is a well defined list of expected values for MODULE_LICENSE so
warn the user upon usage of unknown values.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
scripts/checkpatch.pl | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d124359..7087b28 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5354,6 +5354,22 @@ sub process {
}
}
}
+
+ if ($line =~ /MODULE_LICENSE\(($String)\)/) {
+ my $extracted_string = get_quoted_string($line, $rawline);
+ my $valid_licenses = qr{
+ GPL|
+ GPL\ v2|
+ GPL\ and\ additional\ rights|
+ Dual\ BSD/GPL|
+ Dual\ MIT/GPL|
+ Dual\ MPL/GPL|
+ Proprietary
+ }x;
+ if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) {
+ WARN("MODULE_LICENSE", "unknown module license " . $extracted_string . "\n" . $herecurr);
+ }
+ }
}
# If we have no input at all, then there is nothing to report on
--
1.8.2.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] checkpatch: validate MODULE_LICENSE content
2015-04-06 18:43 [PATCH] checkpatch: validate MODULE_LICENSE content Bjorn Andersson
@ 2015-04-07 0:17 ` Joe Perches
2015-04-07 19:37 ` [PATCH v2] " Bjorn Andersson
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2015-04-07 0:17 UTC (permalink / raw)
To: Bjorn Andersson; +Cc: Andy Whitcroft, linux-kernel
On Mon, 2015-04-06 at 11:43 -0700, Bjorn Andersson wrote:
> There is a well defined list of expected values for MODULE_LICENSE so
> warn the user upon usage of unknown values.
Hello Bjorn
A few nits:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5354,6 +5354,22 @@ sub process {
> }
> }
> }
> +
> + if ($line =~ /MODULE_LICENSE\(($String)\)/) {
As there are uses with spaces, this would be better as:
if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) {
> + my $extracted_string = get_quoted_string($line, $rawline);
> + my $valid_licenses = qr{
> + GPL|
> + GPL\ v2|
> + GPL\ and\ additional\ rights|
> + Dual\ BSD/GPL|
> + Dual\ MIT/GPL|
> + Dual\ MPL/GPL|
> + Proprietary
Why add "Proprietary" ?
This is the list I get in the current tree:
(after collapsing spaces in a few places)
5920 MODULE_LICENSE("GPL")
1211 MODULE_LICENSE("GPL v2")
187 MODULE_LICENSE("Dual BSD/GPL")
37 MODULE_LICENSE("GPL and additional rights")
26 MODULE_LICENSE("Dual MPL/GPL")
1 MODULE_LICENSE("Dual MIT/GPL")
1 MODULE_LICENSE("BSD")
> + }x;
> + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) {
> + WARN("MODULE_LICENSE", "unknown module license " . $extracted_string . "\n" . $herecurr);
I'd write this on 2 lines in the same fashion as
all the other warnings:
WARN("MODULE_LICENSE",
"unknown module license $extracted_string\n" . $herecurr);
Other than those minor issues, it seems sensible enough.
thanks, Joe
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-04-07 0:17 ` Joe Perches
@ 2015-04-07 19:37 ` Bjorn Andersson
2015-04-07 19:48 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2015-04-07 19:37 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel
There is a well defined list of expected values for MODULE_LICENSE so
warn the user upon usage of unknown values.
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
Thanks for the review Joe!
"Proprietary" is in the list because this is the full list of "valid" values
mentioned in include/linux/module.h. So it's the list of licenses valid for
submission, but rather the list of valid values.
Changes since v1:
- Fixed nits pointed out by Joe
- Added comment to clarify the purpose, as well as origin of the values
scripts/checkpatch.pl | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a8b2bd..ca10d79 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5431,6 +5431,24 @@ sub process {
}
}
}
+
+# validate content of MODULE_LICENSE against list from include/linux/module.h
+ if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) {
+ my $extracted_string = get_quoted_string($line, $rawline);
+ my $valid_licenses = qr{
+ GPL|
+ GPL\ v2|
+ GPL\ and\ additional\ rights|
+ Dual\ BSD/GPL|
+ Dual\ MIT/GPL|
+ Dual\ MPL/GPL|
+ Proprietary
+ }x;
+ if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) {
+ WARN("MODULE_LICENSE",
+ "unknown module license " . $extracted_string . "\n" . $herecurr);
+ }
+ }
}
# If we have no input at all, then there is nothing to report on
--
1.8.2.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-04-07 19:37 ` [PATCH v2] " Bjorn Andersson
@ 2015-04-07 19:48 ` Joe Perches
2015-04-07 19:55 ` Bjorn Andersson
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2015-04-07 19:48 UTC (permalink / raw)
To: Bjorn Andersson, Andrew Morton; +Cc: Andy Whitcroft, linux-kernel
On Tue, 2015-04-07 at 12:37 -0700, Bjorn Andersson wrote:
> There is a well defined list of expected values for MODULE_LICENSE so
> warn the user upon usage of unknown values.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>
> Thanks for the review Joe!
No charge.
> "Proprietary" is in the list because this is the full list of "valid" values
> mentioned in include/linux/module.h. So it's the list of licenses valid for
> submission, but rather the list of valid values.
So the "Proprietary" entry is only for non kernel-tree uses.
I wonder if the kernel checkpatch script should account for that.
I've no strong opinion though if Andrew wants to accept this as-is.
cheers, Joe
> Changes since v1:
> - Fixed nits pointed out by Joe
> - Added comment to clarify the purpose, as well as origin of the values
>
> scripts/checkpatch.pl | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9a8b2bd..ca10d79 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5431,6 +5431,24 @@ sub process {
> }
> }
> }
> +
> +# validate content of MODULE_LICENSE against list from include/linux/module.h
> + if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) {
> + my $extracted_string = get_quoted_string($line, $rawline);
> + my $valid_licenses = qr{
> + GPL|
> + GPL\ v2|
> + GPL\ and\ additional\ rights|
> + Dual\ BSD/GPL|
> + Dual\ MIT/GPL|
> + Dual\ MPL/GPL|
> + Proprietary
> + }x;
> + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) {
> + WARN("MODULE_LICENSE",
> + "unknown module license " . $extracted_string . "\n" . $herecurr);
> + }
> + }
> }
>
> # If we have no input at all, then there is nothing to report on
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-04-07 19:48 ` Joe Perches
@ 2015-04-07 19:55 ` Bjorn Andersson
2015-04-07 20:16 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2015-04-07 19:55 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, linux-kernel@vger.kernel.org
On Tue 07 Apr 12:48 PDT 2015, Joe Perches wrote:
> On Tue, 2015-04-07 at 12:37 -0700, Bjorn Andersson wrote:
> > There is a well defined list of expected values for MODULE_LICENSE so
> > warn the user upon usage of unknown values.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> >
> > Thanks for the review Joe!
>
> No charge.
>
> > "Proprietary" is in the list because this is the full list of "valid" values
> > mentioned in include/linux/module.h. So it's the list of licenses valid for
> > submission, but rather the list of valid values.
>
> So the "Proprietary" entry is only for non kernel-tree uses.
> I wonder if the kernel checkpatch script should account for that.
>
The way I read the comment in module.h is that if you're writing a
non-free kernel module then you should mark it with "Proprietary".
If people doing so are using checkpatch that's great, but it's just
included for completeness, not to argue about the concept of proprietary
kernel modules.
> I've no strong opinion though if Andrew wants to accept this as-is.
>
Let me know and I'll drop it from the patch.
> cheers, Joe
>
Thanks,
Bjorn
> > Changes since v1:
> > - Fixed nits pointed out by Joe
> > - Added comment to clarify the purpose, as well as origin of the values
> >
> > scripts/checkpatch.pl | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 9a8b2bd..ca10d79 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -5431,6 +5431,24 @@ sub process {
> > }
> > }
> > }
> > +
> > +# validate content of MODULE_LICENSE against list from include/linux/module.h
> > + if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) {
> > + my $extracted_string = get_quoted_string($line, $rawline);
> > + my $valid_licenses = qr{
> > + GPL|
> > + GPL\ v2|
> > + GPL\ and\ additional\ rights|
> > + Dual\ BSD/GPL|
> > + Dual\ MIT/GPL|
> > + Dual\ MPL/GPL|
> > + Proprietary
> > + }x;
> > + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) {
> > + WARN("MODULE_LICENSE",
> > + "unknown module license " . $extracted_string . "\n" . $herecurr);
> > + }
> > + }
> > }
> >
> > # If we have no input at all, then there is nothing to report on
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-04-07 19:55 ` Bjorn Andersson
@ 2015-04-07 20:16 ` Andrew Morton
2015-06-09 12:11 ` Michal Simek
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2015-04-07 20:16 UTC (permalink / raw)
To: Bjorn Andersson; +Cc: Joe Perches, Andy Whitcroft, linux-kernel@vger.kernel.org
On Tue, 7 Apr 2015 12:55:49 -0700 Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
> > I've no strong opinion though if Andrew wants to accept this as-is.
> >
>
> Let me know and I'll drop it from the patch.
I'd be inclined to keep it - if someone has used "Proprietary" then
they meant to do so, and they don't want checkpatch to keep telling them
they made a typo.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-04-07 20:16 ` Andrew Morton
@ 2015-06-09 12:11 ` Michal Simek
2015-06-09 14:57 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2015-06-09 12:11 UTC (permalink / raw)
To: Andrew Morton, Bjorn Andersson
Cc: Joe Perches, Andy Whitcroft, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 828 bytes --]
Hi Joe and Andrew,
On 04/07/2015 10:16 PM, Andrew Morton wrote:
> On Tue, 7 Apr 2015 12:55:49 -0700 Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
>
>>> I've no strong opinion though if Andrew wants to accept this as-is.
>>>
>>
>> Let me know and I'll drop it from the patch.
>
> I'd be inclined to keep it - if someone has used "Proprietary" then
> they meant to do so, and they don't want checkpatch to keep telling them
> they made a typo.
Any update on this patch?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] checkpatch: validate MODULE_LICENSE content
2015-06-09 12:11 ` Michal Simek
@ 2015-06-09 14:57 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2015-06-09 14:57 UTC (permalink / raw)
To: monstr
Cc: Andrew Morton, Bjorn Andersson, Andy Whitcroft,
linux-kernel@vger.kernel.org
On Tue, 2015-06-09 at 14:11 +0200, Michal Simek wrote:
> Hi Joe and Andrew,
>
> On 04/07/2015 10:16 PM, Andrew Morton wrote:
> > On Tue, 7 Apr 2015 12:55:49 -0700 Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
> >
> >>> I've no strong opinion though if Andrew wants to accept this as-is.
> >>>
> >>
> >> Let me know and I'll drop it from the patch.
> >
> > I'd be inclined to keep it - if someone has used "Proprietary" then
> > they meant to do so, and they don't want checkpatch to keep telling them
> > they made a typo.
>
> Any update on this patch?
I forgot all about it after that discussion.
Please resend it so it's easier for Andrew to pick it up.
Thanks,
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-06-09 14:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-06 18:43 [PATCH] checkpatch: validate MODULE_LICENSE content Bjorn Andersson
2015-04-07 0:17 ` Joe Perches
2015-04-07 19:37 ` [PATCH v2] " Bjorn Andersson
2015-04-07 19:48 ` Joe Perches
2015-04-07 19:55 ` Bjorn Andersson
2015-04-07 20:16 ` Andrew Morton
2015-06-09 12:11 ` Michal Simek
2015-06-09 14:57 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox