[PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[Valdis Klētnieks]

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) {

Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[Joe Perches]

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*'

Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[Valdis Klētnieks]

[-- 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 --]

Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[Joe Perches]

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? ;)

Re: [PATCH] scripts/checkpatch.pl - fix *_NOTIFIER_HEAD handling

[Joe Perches]

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.