Re: [PATCH v3 01/56] scripts: kernel-doc: fix typedef parsing

[Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>]

Em Mon, 26 Oct 2020 20:55:35 -0700
Joe Perches <joe@xxxxxxxxxxx> escreveu:

> On Mon, 2020-10-26 at 08:03 +0100, Mauro Carvalho Chehab wrote:
> []
> > Well, this can help:
> > 	my $typedef_type = qr { ((?:\w+\s+){1,}) }x;  
> 
> unbounded captures are generally bad, I suggest a limit like {1,5}

Ok. 5 is likely too low, if "*" starts to be counted as part of the type.
Maybe 8 would be ok.

> 
> >     if ($x =~ /typedef\s+((?:\w+\s+){1,})\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
> > 	$x =~ /typedef\s+((?:\w+\s+){1,})\s*\*?(\w\S+)\s*\s*\((.*)\);/) {  
> []
> > Fix the regex in order to accept composite types when
> > defining a typedef for a function pointer.  
> [] 
> > diff --git a/scripts/kernel-doc b/scripts/kernel-doc  
> []
> > @@ -1438,13 +1438,14 @@ sub dump_typedef($$) {
> >      $x =~ s@/\*.*?\*/@@gos;	# strip comments.
> >  
> > 
> >      # Parse function prototypes
> > -    if ($x =~ /typedef\s+(\w+)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
> > -	$x =~ /typedef\s+(\w+)\s*(\w\S+)\s*\s*\((.*)\);/) {
> > +    if ($x =~ /typedef\s+((?:\w+\s+){1,})\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
> > +	$x =~ /typedef\s+((?:\w+\s+){1,})\s*\*?(\w\S+)\s*\s*\((.*)\);/) {  
> 
> This typedef does not allow * returns like
> 
> 	const unsigned char *(*string)(args...);
> or
> 	unsigned char *const(*fn)(args...);
> or
> 	void *(*alloc)(args...);

Supporting those shouldn't be hard. See enclosed.

> 
> (not to mention the truly unusual stuff like the typedefs in
>  tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c)
> 
> typedef void (* (*signal_t)(int, void (*)(int)))(int);
> typedef char * (*fn_ptr_arr1_t[10])(int **);
> typedef char * (* const (* const fn_ptr_arr2_t[5])())(char * (*)(int));

Parsing those using a single regex, though, is a lot more complex.
The logic would likely require some loop or a real lexical
analyzer in order to properly parse it.

In the specific case of userspace tools (and, in special, selftests),
it is probably not worth the effort to add support for C expressions
that only exists there, as those won't likely gain kernel-doc entries
for their source code to become part of the Kernel documentation.

Thanks,
Mauro

[PATH] scripts: kernel-doc: fix typedef parsing

The include/linux/genalloc.h file defined this typedef:

        typedef unsigned long (*genpool_algo_t)(unsigned long *map,unsigned long size,unsigned long start,unsigned int nr,void *data, struct gen_pool *pool, unsigned long start_addr);

Because it has a type composite of two words (unsigned long),
the parser gets the typedef name wrong:

.. c:macro:: long

   **Typedef**: Allocation callback function type definition

Fix the regex in order to accept composite types when
defining a typedef for a function pointer.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 99cd8418ff8a..f699cf05d409 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1431,20 +1431,25 @@ sub dump_enum($$) {
     }
 }
 
+my $typedef_type = qr { ((?:\s+[\w\*]+){1,8})\s* }x;
+my $typedef_ident = qr { \*?\s*(\w\S+)\s* }x;
+my $typedef_args = qr { \s*\((.*)\); }x;
+
+my $typedef1 = qr { typedef$typedef_type\($typedef_ident\)$typedef_args }x;
+my $typedef2 = qr { typedef$typedef_type$typedef_ident$typedef_args }x;
+
 sub dump_typedef($$) {
     my $x = shift;
     my $file = shift;
 
     $x =~ s@/\*.*?\*/@@gos;	# strip comments.
 
-    # Parse function prototypes
-    if ($x =~ /typedef\s+(\w+)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
-	$x =~ /typedef\s+(\w+)\s*(\w\S+)\s*\s*\((.*)\);/) {
-
-	# Function typedefs
+    # Parse function typedef prototypes
+    if ($x =~ $typedef1 || $x =~ $typedef2) {
 	$return_type = $1;
 	$declaration_name = $2;
 	my $args = $3;
+	$return_type =~ s/^\s+//;
 
 	create_parameterlist($args, ',', $file, $declaration_name);