* [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling
@ 2019-08-08 1:36 Valdis Klētnieks
2019-08-08 5:50 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Valdis Klētnieks @ 2019-08-08 1:36 UTC (permalink / raw)
To: Gilad Ben-Yossef, Andrew Morton; +Cc: linux-kernel
commit 81398d99e9de80d9dbe65dfe7aadec9497f88242
Author: Gilad Ben-Yossef <gilad@benyossef.com>
Date: Wed Jul 31 14:44:23 2019 +1000
checkpatch: add *_NOTIFIER_HEAD as var definition
has a typo, resulting in a truly amazing error message:
Unescaped left brace in regex is passed through in regex; marked by <-- HERE in m/(?:
\n.}\s*$|
^.DEFINE_(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)\(rtl_usb_probe\)|
^.DECLARE_(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)\(rtl_usb_probe\)|
^.LIST_HEAD\(rtl_usb_probe\)|
^.{ <-- HERE (?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)}_NOTIFIER_HEAD\(rtl_usb_probe\)|
(Rexexp dump continues for 236 lines total)
Fix the typo.
Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5c00151cdee8..dd095d1e5a19 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3891,7 +3891,7 @@ sub process {
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
- ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
+ ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
)/x) {
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling
2019-08-08 1:36 [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling Valdis Klētnieks
@ 2019-08-08 5:50 ` Joe Perches
2019-08-08 6:08 ` Valdis Klētnieks
0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2019-08-08 5:50 UTC (permalink / raw)
To: Valdis Klētnieks, Gilad Ben-Yossef, Andrew Morton; +Cc: linux-kernel
On Wed, 2019-08-07 at 21:36 -0400, Valdis Klētnieks wrote:
> commit 81398d99e9de80d9dbe65dfe7aadec9497f88242
> Author: Gilad Ben-Yossef <gilad@benyossef.com>
> Date: Wed Jul 31 14:44:23 2019 +1000
>
> checkpatch: add *_NOTIFIER_HEAD as var definition
>
> has a typo, resulting in a truly amazing error message:
Ouch, thanks.
> Unescaped left brace in regex is passed through in regex; marked by <-- HERE in m/(?:
[]
> Fix the typo.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3891,7 +3891,7 @@ sub process {
> ^.DEFINE_$Ident\(\Q$name\E\)|
> ^.DECLARE_$Ident\(\Q$name\E\)|
> ^.LIST_HEAD\(\Q$name\E\)|
> - ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
> + ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
Perhaps also better to convert all the '\Q$name\E' to '\s*\Q$name\E\s*'
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling
2019-08-08 5:50 ` Joe Perches
@ 2019-08-08 6:08 ` Valdis Klētnieks
2019-08-08 6:17 ` Joe Perches
2019-08-08 12:45 ` Joe Perches
0 siblings, 2 replies; 5+ messages in thread
From: Valdis Klētnieks @ 2019-08-08 6:08 UTC (permalink / raw)
To: Joe Perches; +Cc: Gilad Ben-Yossef, Andrew Morton, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=us-ascii, Size: 1003 bytes --]
On Wed, 07 Aug 2019 22:50:47 -0700, Joe Perches said:
> On Wed, 2019-08-07 at 21:36 -0400, Valdis Klētnieks wrote:
> > ^.DEFINE_$Ident\(\Q$name\E\)|
> > ^.DECLARE_$Ident\(\Q$name\E\)|
> > ^.LIST_HEAD\(\Q$name\E\)|
> > - ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
> > + ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
>
> Perhaps also better to convert all the '\Q$name\E' to '\s*\Q$name\E\s*'
Yes, but that would need to be a separate patch. The question would be if we
consider 'DEFINE_foo( barbaz )' and similar with whitespace to be desirable
style or not.
[/usr/src/linux-next] grep '\\Q$name\\E' scripts/checkpatch.pl
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
We already have the \s* in one place. Somebody else can decide if it should
be in the other 5 places or not. :)
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling
2019-08-08 6:08 ` Valdis Klētnieks
@ 2019-08-08 6:17 ` Joe Perches
2019-08-08 12:45 ` Joe Perches
1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2019-08-08 6:17 UTC (permalink / raw)
To: Valdis Klētnieks; +Cc: Gilad Ben-Yossef, Andrew Morton, linux-kernel
On Thu, 2019-08-08 at 02:08 -0400, Valdis Klētnieks wrote:
> On Wed, 07 Aug 2019 22:50:47 -0700, Joe Perches said:
> > On Wed, 2019-08-07 at 21:36 -0400, Valdis Kltnieks wrote:
> > > ^.DEFINE_$Ident\(\Q$name\E\)|
> > > ^.DECLARE_$Ident\(\Q$name\E\)|
> > > ^.LIST_HEAD\(\Q$name\E\)|
> > > - ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
> > > + ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
> >
> > Perhaps also better to convert all the '\Q$name\E' to '\s*\Q$name\E\s*'
>
> Yes, but that would need to be a separate patch.
Maybe so.
I'm just not a big fan of micro patches.
> The question would be if we
> consider 'DEFINE_foo( barbaz )' and similar with whitespace to be desirable
> style or not.
Oh, it would definitely be an uncomfortable style,
it's just allowing it in the regex.
btw: I'm also fine with it being a separate global patch.
cheers, Joe
> We already have the \s* in one place. Somebody else can decide if it should
> be in the other 5 places or not. :)
Is that supposed to be me? ;)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling
2019-08-08 6:08 ` Valdis Klētnieks
2019-08-08 6:17 ` Joe Perches
@ 2019-08-08 12:45 ` Joe Perches
1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2019-08-08 12:45 UTC (permalink / raw)
To: Valdis Klētnieks; +Cc: Gilad Ben-Yossef, Andrew Morton, linux-kernel
On Thu, 2019-08-08 at 02:08 -0400, Valdis Klētnieks wrote:
> On Wed, 07 Aug 2019 22:50:47 -0700, Joe Perches said:
> > On Wed, 2019-08-07 at 21:36 -0400, Valdis Kltnieks wrote:
> > > ^.DEFINE_$Ident\(\Q$name\E\)|
> > > ^.DECLARE_$Ident\(\Q$name\E\)|
> > > ^.LIST_HEAD\(\Q$name\E\)|
> > > - ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
> > > + ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
oops.
I did reply with the correct form for Gilad's original patch.
https://lore.kernel.org/lkml/613b7f1dd7244df22f677777cc946758cbd7e61c.camel@perches.com/
My mistake not noticing again in Gilad's v2.
Oh well.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-08 12:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 1:36 [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling Valdis Klētnieks
2019-08-08 5:50 ` Joe Perches
2019-08-08 6:08 ` Valdis Klētnieks
2019-08-08 6:17 ` Joe Perches
2019-08-08 12:45 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).