linux-kernel - Re: [RFC] scripts: kernel-doc: reduce repeated regex expressions into variables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210426173102.GO235567@casper.infradead.org>
Date:   Mon, 26 Apr 2021 18:31:02 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Aditya Srivastava <yashsri421@...il.com>
Cc:     corbet@....net, lukas.bulwahn@...il.com,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC] scripts: kernel-doc: reduce repeated regex expressions
 into variables

On Sat, Apr 24, 2021 at 05:27:34PM +0530, Aditya Srivastava wrote:
> On 23/4/21 6:51 pm, Matthew Wilcox wrote:
> > On Fri, Apr 23, 2021 at 12:48:39AM +0530, Aditya Srivastava wrote:
> >> +my $pointer_function = qr{([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)};
> > 
> > Is that a pointer-to-function?  Or as people who write C usually call it,
> > a function pointer?  Wouldn't it be better to call it $function_pointer?
> > 
> Will do it.
> 
> >> @@ -1210,8 +1211,14 @@ sub dump_struct($$) {
> >>      my $decl_type;
> >>      my $members;
> >>      my $type = qr{struct|union};
> >> +    my $packed = qr{__packed};
> >> +    my $aligned = qr{__aligned};
> >> +    my $cacheline_aligned_in_smp = qr{____cacheline_aligned_in_smp};
> >> +    my $cacheline_aligned = qr{____cacheline_aligned};
> > 
> > I don't think those four definitions actually simplify anything.
> > 
> >> +    my $attribute = qr{__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)}i;
> > 
> > ... whereas this one definitely does.
> > 
> >> -	$members =~ s/\s*__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)/ /gi;
> >> -	$members =~ s/\s*__aligned\s*\([^;]*\)/ /gos;
> >> -	$members =~ s/\s*__packed\s*/ /gos;
> >> +	$members =~ s/\s*$attribute/ /gi;
> >> +	$members =~ s/\s*$aligned\s*\([^;]*\)/ /gos;
> > 
> > Maybe put the \s*\([^;]*\) into $aligned?  Then it becomes a useful
> > abstraction.
> 
> Actually, I had made these variables as they were repeated here and at
> -    my $definition_body =
> qr{\{(.*)\}(?:\s*(?:__packed|__aligned|____cacheline_aligned_in_smp|____cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*};
> +    my $definition_body =
> qr{\{(.*)\}(?:\s*(?:$packed|$aligned|$cacheline_aligned_in_smp|$cacheline_aligned|$attribute))*};
> 
> So, defining them at a place might help.
> 
> What do you think?

I don't think that seeing $packed is any easier to read than __packed.
Indeed, I think it's harder, because now I have to look up what $packed
is defined as.

Defining a variable, say

	$decorations = qr{__packed|__aligned|____cacheline_aligned_in_smp|____cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\))}
	(i didn't count brackets to be sure i got that right)

would be helpful because then we could say:

	my $definition_body = qr{\{(.*)\}...$decorations...

and have a fighting chance of understanding what it means.

Now, this other place we use it, we do the =~ operation a number of times.
Is there a way to use the $decorations variable to do the same thing
with a single operation?