* Re: checkpatch.pl error might be false positive
[not found] <CAN1v_PvS3dH6HVQ90d=3cfsSdMgieyoDNp3VN-vrHv-aqSFEAw@mail.gmail.com>
@ 2013-12-19 22:15 ` Joe Perches
2013-12-20 7:56 ` Josh Triplett
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-12-19 22:15 UTC (permalink / raw)
To: Ravi Patel, Josh Triplett, Jesse Brandeburg
Cc: Andy Whitcroft, Kumar Sankaran, Loc Ho, Keyur Chudgar, LKML
(resending adding lkml)
(adding Josh Triplett and Jesse Brandeburg)
On Thu, 2013-12-19 at 13:01 -0800, Ravi Patel wrote:
> Hi Andy, Joe
>
> My name is Ravi Patel and I am working for AppliedMicro.
> I am planning to submit APM X-Gene SoC QMTM drivers to open source after
> running checkpatch.pl
> I am seeing following error saying remove FSF address from my patch, which
> I don't have in my source/header files.
>
> ERROR: Do not include the paragraph about writing to the Free Software
> Foundation's mailing address from the sample GPL notice. The FSF has
> changed addresses in the past, and may do so again. Linux already includes
> a copy of the GPL.
> #1580: FILE: include/misc/xgene/xgene_qmtm.h:19:
> + * You should have received a copy of the GNU General Public License$
>
> Here is my License banner in xgene_qmtm.h
> There are no tabs in my banner
>
> /*
> * AppliedMicro X-Gene SOC Queue Manager/Traffic Manager driver
> *
> * Copyright (c) 2013 Applied Micro Circuits Corporation.
> * Author: Ravi Patel <rapatel@apm.com>
> * Keyur Chudgar <kchudgar@apm.com>
> * Fushen Chen <fchen@apm.com>
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License as published by the
> * Free Software Foundation; either version 2 of the License, or (at your
> * option) any later version.
> *
> * This program is distributed in the hope that it will be useful,
> * but WITHOUT ANY WARRANTY; without even the implied warranty of
> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> * GNU General Public License for more details.
> *
> * You should have received a copy of the GNU General Public License
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> *
> */
>
> Can you please check if the error is false positive.
Josh?
Is the intent that the "copy of the GNU General Public License"
statement should also be removed?
Jesse had a question recently about the appropriateness of the
removal given the license text.
http://www.spinics.net/lists/netdev/msg262152.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: checkpatch.pl error might be false positive
2013-12-19 22:15 ` checkpatch.pl error might be false positive Joe Perches
@ 2013-12-20 7:56 ` Josh Triplett
2013-12-20 8:37 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2013-12-20 7:56 UTC (permalink / raw)
To: Joe Perches
Cc: Ravi Patel, Jesse Brandeburg, Andy Whitcroft, Kumar Sankaran,
Loc Ho, Keyur Chudgar, LKML
On Thu, Dec 19, 2013 at 02:15:48PM -0800, Joe Perches wrote:
> On Thu, 2013-12-19 at 13:01 -0800, Ravi Patel wrote:
> > My name is Ravi Patel and I am working for AppliedMicro.
> > I am planning to submit APM X-Gene SoC QMTM drivers to open source after
> > running checkpatch.pl
> > I am seeing following error saying remove FSF address from my patch, which
> > I don't have in my source/header files.
> >
> > ERROR: Do not include the paragraph about writing to the Free Software
> > Foundation's mailing address from the sample GPL notice. The FSF has
> > changed addresses in the past, and may do so again. Linux already includes
> > a copy of the GPL.
> > #1580: FILE: include/misc/xgene/xgene_qmtm.h:19:
> > + * You should have received a copy of the GNU General Public License$
> >
> > Here is my License banner in xgene_qmtm.h
> > There are no tabs in my banner
> >
> > /*
> > * AppliedMicro X-Gene SOC Queue Manager/Traffic Manager driver
> > *
> > * Copyright (c) 2013 Applied Micro Circuits Corporation.
> > * Author: Ravi Patel <rapatel@apm.com>
> > * Keyur Chudgar <kchudgar@apm.com>
> > * Fushen Chen <fchen@apm.com>
> > *
> > * This program is free software; you can redistribute it and/or modify it
> > * under the terms of the GNU General Public License as published by the
> > * Free Software Foundation; either version 2 of the License, or (at your
> > * option) any later version.
> > *
> > * This program is distributed in the hope that it will be useful,
> > * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > * GNU General Public License for more details.
> > *
> > * You should have received a copy of the GNU General Public License
> > * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > *
> > */
> >
> > Can you please check if the error is false positive.
>
> Josh?
>
> Is the intent that the "copy of the GNU General Public License"
> statement should also be removed?
While it does seem preferable to drop that paragraph, it isn't nearly as
important as dropping the mailing address. The match on "You should
have received a copy" should probably be reduced to CHK level or
dropped.
> Jesse had a question recently about the appropriateness of the
> removal given the license text.
>
> http://www.spinics.net/lists/netdev/msg262152.html
Changing the license text is not OK (though the FSF has said it's OK to
make new licenses based on the legal text minus the preamble and sample
instructions, but that doesn't apply here).
But that's not the same as the license *notice*, to which the "but
changing it is not allowed" does not apply.
- Josh Triplett
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: checkpatch.pl error might be false positive
2013-12-20 7:56 ` Josh Triplett
@ 2013-12-20 8:37 ` Joe Perches
2013-12-20 9:16 ` Josh Triplett
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-12-20 8:37 UTC (permalink / raw)
To: Josh Triplett
Cc: Ravi Patel, Jesse Brandeburg, Andy Whitcroft, Kumar Sankaran,
Loc Ho, Keyur Chudgar, LKML
On Thu, 2013-12-19 at 23:56 -0800, Josh Triplett wrote:
> On Thu, Dec 19, 2013 at 02:15:48PM -0800, Joe Perches wrote:
> > On Thu, 2013-12-19 at 13:01 -0800, Ravi Patel wrote:
> > > My name is Ravi Patel and I am working for AppliedMicro.
> > > I am planning to submit APM X-Gene SoC QMTM drivers to open source after
> > > running checkpatch.pl
> > > I am seeing following error saying remove FSF address from my patch, which
> > > I don't have in my source/header files.
> > >
> > > ERROR: Do not include the paragraph about writing to the Free Software
> > > Foundation's mailing address from the sample GPL notice. The FSF has
> > > changed addresses in the past, and may do so again. Linux already includes
> > > a copy of the GPL.
> > > #1580: FILE: include/misc/xgene/xgene_qmtm.h:19:
> > > + * You should have received a copy of the GNU General Public License$
> > >
> > > Here is my License banner in xgene_qmtm.h
> > > There are no tabs in my banner
> > >
> > > /*
> > > * AppliedMicro X-Gene SOC Queue Manager/Traffic Manager driver
> > > *
> > > * Copyright (c) 2013 Applied Micro Circuits Corporation.
> > > * Author: Ravi Patel <rapatel@apm.com>
> > > * Keyur Chudgar <kchudgar@apm.com>
> > > * Fushen Chen <fchen@apm.com>
> > > *
> > > * This program is free software; you can redistribute it and/or modify it
> > > * under the terms of the GNU General Public License as published by the
> > > * Free Software Foundation; either version 2 of the License, or (at your
> > > * option) any later version.
> > > *
> > > * This program is distributed in the hope that it will be useful,
> > > * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > * GNU General Public License for more details.
> > > *
> > > * You should have received a copy of the GNU General Public License
> > > * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > > *
> > > */
> > >
> > > Can you please check if the error is false positive.
> >
> > Josh?
> >
> > Is the intent that the "copy of the GNU General Public License"
> > statement should also be removed?
>
> While it does seem preferable to drop that paragraph, it isn't nearly as
> important as dropping the mailing address. The match on "You should
> have received a copy" should probably be reduced to CHK level or
> dropped.
Maybe something like this, but maybe dropping the
"you should have received a copy" is ok too.
I did a few greps for various address patterns and
it seems using case insensitive matches and shorter
street types is better.
There are a few places where number and street name
are on different lines. Oh well, nothing's perfect.
---
scripts/checkpatch.pl | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8f3aecd..b02d6ad 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1963,15 +1963,16 @@ sub process {
}
# Check for FSF mailing addresses.
- if ($rawline =~ /You should have received a copy/ ||
- $rawline =~ /write to the Free Software/ ||
- $rawline =~ /59 Temple Place/ ||
- $rawline =~ /51 Franklin Street/) {
+ if ($rawline =~ /\bYou should have received a copy/i ||
+ $rawline =~ /\bwrite to the Free/i ||
+ $rawline =~ /\b59\s+Temple\s+Pl/i ||
+ $rawline =~ /\b51\s+Franklin\s+St/i) {
my $herevet = "$here\n" . cat_vet($rawline) . "\n";
my $msg_type = \&ERROR;
$msg_type = \&CHK if ($file);
+ $msg_type = \&CHK if ($rawline =~ /\bYou should have received a copy/i);
&{$msg_type}("FSF_MAILING_ADDRESS",
- "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
+ "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
}
# check for Kconfig help text having a real description
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: checkpatch.pl error might be false positive
2013-12-20 8:37 ` Joe Perches
@ 2013-12-20 9:16 ` Josh Triplett
2013-12-20 9:23 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2013-12-20 9:16 UTC (permalink / raw)
To: Joe Perches
Cc: Ravi Patel, Jesse Brandeburg, Andy Whitcroft, Kumar Sankaran,
Loc Ho, Keyur Chudgar, LKML
On Fri, Dec 20, 2013 at 12:37:10AM -0800, Joe Perches wrote:
> On Thu, 2013-12-19 at 23:56 -0800, Josh Triplett wrote:
> > On Thu, Dec 19, 2013 at 02:15:48PM -0800, Joe Perches wrote:
> > > On Thu, 2013-12-19 at 13:01 -0800, Ravi Patel wrote:
> > > > My name is Ravi Patel and I am working for AppliedMicro.
> > > > I am planning to submit APM X-Gene SoC QMTM drivers to open source after
> > > > running checkpatch.pl
> > > > I am seeing following error saying remove FSF address from my patch, which
> > > > I don't have in my source/header files.
> > > >
> > > > ERROR: Do not include the paragraph about writing to the Free Software
> > > > Foundation's mailing address from the sample GPL notice. The FSF has
> > > > changed addresses in the past, and may do so again. Linux already includes
> > > > a copy of the GPL.
> > > > #1580: FILE: include/misc/xgene/xgene_qmtm.h:19:
> > > > + * You should have received a copy of the GNU General Public License$
> > > >
> > > > Here is my License banner in xgene_qmtm.h
> > > > There are no tabs in my banner
> > > >
> > > > /*
> > > > * AppliedMicro X-Gene SOC Queue Manager/Traffic Manager driver
> > > > *
> > > > * Copyright (c) 2013 Applied Micro Circuits Corporation.
> > > > * Author: Ravi Patel <rapatel@apm.com>
> > > > * Keyur Chudgar <kchudgar@apm.com>
> > > > * Fushen Chen <fchen@apm.com>
> > > > *
> > > > * This program is free software; you can redistribute it and/or modify it
> > > > * under the terms of the GNU General Public License as published by the
> > > > * Free Software Foundation; either version 2 of the License, or (at your
> > > > * option) any later version.
> > > > *
> > > > * This program is distributed in the hope that it will be useful,
> > > > * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > * GNU General Public License for more details.
> > > > *
> > > > * You should have received a copy of the GNU General Public License
> > > > * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > > > *
> > > > */
> > > >
> > > > Can you please check if the error is false positive.
> > >
> > > Josh?
> > >
> > > Is the intent that the "copy of the GNU General Public License"
> > > statement should also be removed?
> >
> > While it does seem preferable to drop that paragraph, it isn't nearly as
> > important as dropping the mailing address. The match on "You should
> > have received a copy" should probably be reduced to CHK level or
> > dropped.
>
> Maybe something like this, but maybe dropping the
> "you should have received a copy" is ok too.
>
> I did a few greps for various address patterns and
> it seems using case insensitive matches and shorter
> street types is better.
>
> There are a few places where number and street name
> are on different lines. Oh well, nothing's perfect.
Duplicating the regex seems awful. Could you put it in a variable?
> ---
> scripts/checkpatch.pl | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8f3aecd..b02d6ad 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1963,15 +1963,16 @@ sub process {
> }
>
> # Check for FSF mailing addresses.
> - if ($rawline =~ /You should have received a copy/ ||
> - $rawline =~ /write to the Free Software/ ||
> - $rawline =~ /59 Temple Place/ ||
> - $rawline =~ /51 Franklin Street/) {
> + if ($rawline =~ /\bYou should have received a copy/i ||
> + $rawline =~ /\bwrite to the Free/i ||
> + $rawline =~ /\b59\s+Temple\s+Pl/i ||
> + $rawline =~ /\b51\s+Franklin\s+St/i) {
> my $herevet = "$here\n" . cat_vet($rawline) . "\n";
> my $msg_type = \&ERROR;
> $msg_type = \&CHK if ($file);
> + $msg_type = \&CHK if ($rawline =~ /\bYou should have received a copy/i);
> &{$msg_type}("FSF_MAILING_ADDRESS",
> - "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
> + "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
> }
>
> # check for Kconfig help text having a real description
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: checkpatch.pl error might be false positive
2013-12-20 9:16 ` Josh Triplett
@ 2013-12-20 9:23 ` Joe Perches
2013-12-20 17:03 ` Josh Triplett
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-12-20 9:23 UTC (permalink / raw)
To: Josh Triplett
Cc: Ravi Patel, Jesse Brandeburg, Andy Whitcroft, Kumar Sankaran,
Loc Ho, Keyur Chudgar, LKML
On Fri, 2013-12-20 at 01:16 -0800, Josh Triplett wrote:
> Duplicating the regex seems awful. Could you put it in a variable?
awful is a strong word and I don't have any
issue with the duplication. Using a variable
is just as confusing as any duplication.
Using a separate test and message for the GPL
vs the FSF bits would likely be better anyway.
You're the original FSF address patch submitter,
fix it as you think appropriate.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: checkpatch.pl error might be false positive
2013-12-20 9:23 ` Joe Perches
@ 2013-12-20 17:03 ` Josh Triplett
0 siblings, 0 replies; 6+ messages in thread
From: Josh Triplett @ 2013-12-20 17:03 UTC (permalink / raw)
To: Joe Perches
Cc: Ravi Patel, Jesse Brandeburg, Andy Whitcroft, Kumar Sankaran,
Loc Ho, Keyur Chudgar, LKML
On Fri, Dec 20, 2013 at 01:23:52AM -0800, Joe Perches wrote:
> On Fri, 2013-12-20 at 01:16 -0800, Josh Triplett wrote:
> > Duplicating the regex seems awful. Could you put it in a variable?
>
> awful is a strong word and I don't have any
> issue with the duplication. Using a variable
> is just as confusing as any duplication.
Fair enough; I don't care that much. For your patch:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
- Josh Triplett
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-20 17:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAN1v_PvS3dH6HVQ90d=3cfsSdMgieyoDNp3VN-vrHv-aqSFEAw@mail.gmail.com>
2013-12-19 22:15 ` checkpatch.pl error might be false positive Joe Perches
2013-12-20 7:56 ` Josh Triplett
2013-12-20 8:37 ` Joe Perches
2013-12-20 9:16 ` Josh Triplett
2013-12-20 9:23 ` Joe Perches
2013-12-20 17:03 ` Josh Triplett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox