Messages in this thread | | | Subject | Re: [PATCH] checkpatch: validate MODULE_LICENSE content | From | Joe Perches <> | Date | Mon, 06 Apr 2015 17:17:02 -0700 |
| |
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
| |