[PATCH v2] scripts: kernel-doc: fix nexted handling

[PATCH v2] scripts: kernel-doc: fix nexted handling

[Mauro Carvalho Chehab]

At DVB, we have, at some structs, things like (see
struct dvb_demux_feed, at dvb_demux.h):

	union {
		struct dmx_ts_feed ts;
		struct dmx_section_feed sec;
	} feed;

	union {
		dmx_ts_cb ts;
		dmx_section_cb sec;
	} cb;

Fix the nested parser to avoid it to eat the first union.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---

v2:  handle embedded structs/unions from inner to outer

When we have multiple levels of embedded structs, like
(see v4l2-async.h):

struct v4l2_async_subdev {
	enum v4l2_async_match_type match_type;
	union {
		struct {
			struct fwnode_handle *fwnode;
		} fwnode;
		struct {
			const char *name;
		} device_name;
		struct {
			int adapter_id;
			unsigned short address;
		} i2c;
		struct {
			bool (*match)(struct device *,
				      struct v4l2_async_subdev *);
			void *priv;
		} custom;
	} match;
...
}

we need a smarter rule that will be removing nested structs
from the inner to the outer ones. So, changed the parsing rule to
remove nested structs/unions from the inner ones to the outer
ones, while it matches.

 scripts/kernel-doc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 9d3eafea58f0..443e1bcc78db 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2173,7 +2173,7 @@ sub dump_struct($$) {
 	my $members = $3;
 
 	# ignore embedded structs or unions
-	$members =~ s/({.*})//g;
+	while ($members =~ s/({[^\{\}]*})//g) {};
 	$nested = $1;
 
 	# ignore members marked private:
-- 
2.13.5

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Markus Heiser]

> Am 24.09.2017 um 12:23 schrieb Mauro Carvalho Che
> 
> v2:  handle embedded structs/unions from inner to outer
> 
> When we have multiple levels of embedded structs,
> 
> we need a smarter rule that will be removing nested structs
> from the inner to the outer ones. So, changed the parsing rule to
> remove nested structs/unions from the inner ones to the outer
> ones, while it matches.

argh, sub-nested I forgot.

> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 9d3eafea58f0..443e1bcc78db 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2173,7 +2173,7 @@ sub dump_struct($$) {
> 	my $members = $3;
> 
> 	# ignore embedded structs or unions
> -	$members =~ s/({.*})//g;
> +	while ($members =~ s/({[^\{\}]*})//g) {};
> 	$nested = $1;

I haven't tested this patch, but I guess with you might get
some "excess struct member" warnings, since the value of
'nested' is just the content of the last match.

Here is what I have done in the python version:

 https://github.com/return42/linuxdoc/commit/8d9394

and here is the impact:

 https://github.com/return42/sphkerneldoc/commit/2ff22cf82de3236c1ec7616bd4b65ce2aedd2a90

As you can see in my linked patch above, I implemented it by
hand and collected all the 'nested' stuff. I guess this
is impossible with regexpr.

I recommend to do something similar with the perl script.

Since your perl is better than my; could you please prepare such a v3 patch?

Thanks!

-- Markus --




> 
> 	# ignore members marked private:
> -- 
> 2.13.5
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-doc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Mauro Carvalho Chehab]

Em Sun, 24 Sep 2017 17:13:13 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > Am 24.09.2017 um 12:23 schrieb Mauro Carvalho Che
> > 
> > v2:  handle embedded structs/unions from inner to outer
> > 
> > When we have multiple levels of embedded structs,
> > 
> > we need a smarter rule that will be removing nested structs
> > from the inner to the outer ones. So, changed the parsing rule to
> > remove nested structs/unions from the inner ones to the outer
> > ones, while it matches.  
> 
> argh, sub-nested I forgot.
> 
> > scripts/kernel-doc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> > index 9d3eafea58f0..443e1bcc78db 100755
> > --- a/scripts/kernel-doc
> > +++ b/scripts/kernel-doc
> > @@ -2173,7 +2173,7 @@ sub dump_struct($$) {
> > 	my $members = $3;
> > 
> > 	# ignore embedded structs or unions
> > -	$members =~ s/({.*})//g;
> > +	while ($members =~ s/({[^\{\}]*})//g) {};
> > 	$nested = $1;  
> 
> I haven't tested this patch, but I guess with you might get
> some "excess struct member" warnings, since the value of
> 'nested' is just the content of the last match.
> 
> Here is what I have done in the python version:
> 
>  https://github.com/return42/linuxdoc/commit/8d9394
> 
> and here is the impact:
> 
>  https://github.com/return42/sphkerneldoc/commit/2ff22cf82de3236c1ec7616bd4b65ce2aedd2a90
> 
> As you can see in my linked patch above, I implemented it by
> hand and collected all the 'nested' stuff. I guess this
> is impossible with regexpr.
> 
> I recommend to do something similar with the perl script.
> 
> Since your perl is better than my; could you please prepare such a v3 patch?
> 
> Thanks!

Hmm... after looking at the "impact" URL, I guess we should, instead,
*not* ignore nested arguments, but also parse them, as this would allow
adding documentation for them, e. g. something like the (incomplete)
patch.

As reference, I'm testing it with:

	$ scripts/kernel-doc -rst ./drivers/clk/ingenic/cgu.h 

PS.: I'm pretty sure it can be improved. Also, only the ReST output
is working properly on this current version.

Regards,
Mauro

[PATCH RFC] scripts: kernel-doc: parse nexted structs/unions
    
At DVB, we have, at some structs, things like (see
struct dvb_demux_feed, at dvb_demux.h):
    
            union {
                    struct dmx_ts_feed ts;
                    struct dmx_section_feed sec;
            } feed;
    
            union {
                    dmx_ts_cb ts;
                    dmx_section_cb sec;
            } cb;
    
Fix the nested parser to avoid it to eat the first union.
    
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 9d3eafea58f0..102d5ec200a4 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2036,26 +2036,18 @@ sub output_struct_rst(%) {
     print "**Definition**\n\n";
     print "::\n\n";
     print "  " . $args{'type'} . " " . $args{'struct'} . " {\n";
-    foreach $parameter (@{$args{'parameterlist'}}) {
-	if ($parameter =~ /^#/) {
-	    print "  " . "$parameter\n";
-	    next;
-	}
 
-	my $parameter_name = $parameter;
-	$parameter_name =~ s/\[.*//;
-
-	($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
-	$type = $args{'parametertypes'}{$parameter};
-	if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
-	    # pointer-to-function
-	    print "    $1 $parameter) ($2);\n";
-	} elsif ($type =~ m/^(.*?)\s*(:.*)/) {
-	    # bitfield
-	    print "    $1 $parameter$2;\n";
-	} else {
-	    print "    " . $type . " " . $parameter . ";\n";
-	}
+    my $def = $args{'definition'};
+    $def =~ s/([{,;])/$1\n/g;
+    $def =~ s/}\s+;/};/g;
+    my @def_args = split /\n/, $def;
+    my $level = 1;
+    foreach my $clause (@def_args) {
+	$clause =~ s/^\s+//;
+	$clause =~ s/\s+$//;
+	$level-- if ($clause =~ m/}/ && $level > 1);
+	print "  " . "  " x $level. "$clause\n" if ($clause);
+	$level++ if ($clause =~ m/{/);
     }
     print "  };\n\n";
 
@@ -2168,13 +2160,22 @@ sub dump_struct($$) {
     my $nested;
 
     if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
-	#my $decl_type = $1;
+	my $decl_type = $1;
 	$declaration_name = $2;
 	my $members = $3;
-
-	# ignore embedded structs or unions
-	$members =~ s/({.*})//g;
-	$nested = $1;
+	my $clause = $3;
+
+	# Split nested struct/union elements as newer ones
+	my $cont = 1;
+	while ($cont) {
+		$cont = 0;
+		while ($members =~ s/(struct|union)\s+{([^{}]*)}(\s*\S+\s*)\;?/$1 $3; $2 /g) {
+			$cont = 1;
+		};
+	};
+	# Ignore other nested elements, like enums
+	$members =~ s/({[^\{\}]*})//g;
+	$nested = $decl_type;
 
 	# ignore members marked private:
 	$members =~ s/\/\*\s*private:.*?\/\*\s*public:.*?\*\///gosi;
@@ -2200,6 +2201,7 @@ sub dump_struct($$) {
 			   'struct',
 			   {'struct' => $declaration_name,
 			    'module' => $modulename,
+			    'definition' => $clause,
 			    'parameterlist' => \@parameterlist,
 			    'parameterdescs' => \%parameterdescs,
 			    'parametertypes' => \%parametertypes,



Thanks,
Mauro

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Markus Heiser]

> Am 24.09.2017 um 19:38 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> 
> Em Sun, 24 Sep 2017 17:13:13 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
> 
>>> Am 24.09.2017 um 12:23 schrieb Mauro Carvalho Che
>>> 
>>> v2:  handle embedded structs/unions from inner to outer
>>> 
>>> When we have multiple levels of embedded structs,
>>> 
>>> we need a smarter rule that will be removing nested structs
>>> from the inner to the outer ones. So, changed the parsing rule to
>>> remove nested structs/unions from the inner ones to the outer
>>> ones, while it matches.  
>> 
>> argh, sub-nested I forgot.
>> 
>>> scripts/kernel-doc | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>>> index 9d3eafea58f0..443e1bcc78db 100755
>>> --- a/scripts/kernel-doc
>>> +++ b/scripts/kernel-doc
>>> @@ -2173,7 +2173,7 @@ sub dump_struct($$) {
>>> 	my $members = $3;
>>> 
>>> 	# ignore embedded structs or unions
>>> -	$members =~ s/({.*})//g;
>>> +	while ($members =~ s/({[^\{\}]*})//g) {};
>>> 	$nested = $1;  
>> 
>> I haven't tested this patch, but I guess with you might get
>> some "excess struct member" warnings, since the value of
>> 'nested' is just the content of the last match.
>> 
>> Here is what I have done in the python version:
>> 
>> https://github.com/return42/linuxdoc/commit/8d9394
>> 
>> and here is the impact:
>> 
>> https://github.com/return42/sphkerneldoc/commit/2ff22cf82de3236c1ec7616bd4b65ce2aedd2a90
>> 
>> As you can see in my linked patch above, I implemented it by
>> hand and collected all the 'nested' stuff. I guess this
>> is impossible with regexpr.
>> 
>> I recommend to do something similar with the perl script.
>> 
>> Since your perl is better than my; could you please prepare such a v3 patch?
>> 
>> Thanks!
> 
> Hmm... after looking at the "impact" URL, I guess we should, instead,
> *not* ignore nested arguments, but also parse them, as this would allow
> adding documentation for them

good point, Thanks! ... I will give it a try ...

> , e. g. something like the (incomplete)
> patch.
> 
> As reference, I'm testing it with:
> 
> 	$ scripts/kernel-doc -rst ./drivers/clk/ingenic/cgu.h 
> 
> PS.: I'm pretty sure it can be improved. Also, only the ReST output
> is working properly on this current version.

But other outputs are also benefit from. Anyway, my question is; do
we really need to support other output formats any longer? .. since
we can reach other outputs from the reST base.


> Regards,
> Mauro
> 
> [PATCH RFC] scripts: kernel-doc: parse nexted structs/unions
> 
> At DVB, we have, at some structs, things like (see
> struct dvb_demux_feed, at dvb_demux.h):
> 
>            union {
>                    struct dmx_ts_feed ts;
>                    struct dmx_section_feed sec;
>            } feed;
> 
>            union {
>                    dmx_ts_cb ts;
>                    dmx_section_cb sec;
>            } cb;
> 
> Fix the nested parser to avoid it to eat the first union.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 9d3eafea58f0..102d5ec200a4 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2036,26 +2036,18 @@ sub output_struct_rst(%) {
>     print "**Definition**\n\n";
>     print "::\n\n";
>     print "  " . $args{'type'} . " " . $args{'struct'} . " {\n";
> -    foreach $parameter (@{$args{'parameterlist'}}) {
> -	if ($parameter =~ /^#/) {
> -	    print "  " . "$parameter\n";
> -	    next;
> -	}
> 
> -	my $parameter_name = $parameter;
> -	$parameter_name =~ s/\[.*//;
> -
> -	($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
> -	$type = $args{'parametertypes'}{$parameter};
> -	if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
> -	    # pointer-to-function
> -	    print "    $1 $parameter) ($2);\n";
> -	} elsif ($type =~ m/^(.*?)\s*(:.*)/) {
> -	    # bitfield
> -	    print "    $1 $parameter$2;\n";
> -	} else {
> -	    print "    " . $type . " " . $parameter . ";\n";
> -	}
> +    my $def = $args{'definition'};
> +    $def =~ s/([{,;])/$1\n/g;

Do we need to split at comma (',') ? If we do so, we also split lines like:

     void   (*find_idlest)(struct clk *, void __iomem **, u8 *, u8 *);                           


> +    $def =~ s/}\s+;/};/g;
> +    my @def_args = split /\n/, $def;
> +    my $level = 1;
> +    foreach my $clause (@def_args) {
> +	$clause =~ s/^\s+//;
> +	$clause =~ s/\s+$//;
> +	$level-- if ($clause =~ m/}/ && $level > 1);
> +	print "  " . "  " x $level. "$clause\n" if ($clause);
> +	$level++ if ($clause =~ m/{/);
>     }
>     print "  };\n\n";
> 
> @@ -2168,13 +2160,22 @@ sub dump_struct($$) {
>     my $nested;
> 
>     if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
> -	#my $decl_type = $1;
> +	my $decl_type = $1;
> 	$declaration_name = $2;
> 	my $members = $3;
> -
> -	# ignore embedded structs or unions
> -	$members =~ s/({.*})//g;
> -	$nested = $1;
> +	my $clause = $3;

IMO 'definition' would be a better name.

> +
> +	# Split nested struct/union elements as newer ones
> +	my $cont = 1;
> +	while ($cont) {
> +		$cont = 0;
> +		while ($members =~ s/(struct|union)\s+{([^{}]*)}(\s*\S+\s*)\;?/$1 $3; $2 /g) {

Very tricky, it took me awhile to understand, but I guess there 
is a mistake with the semicolon at the end: substitution is *leftmost*
so \;? at the end will never match (replaced) and the replacement $3;
inserts one additional semicolon. Try:

  s/(struct|union)\s+{([^{}]*)}(\s*\S+\s*)/$1 $3 $2 /g

I also replaced \S with [^\s;] since \S will also match:

"union { const char *name; u32 rate; }  sys_clk;int (*scale)"
-----------------------------------------------^------------- 

here is the expression I ended with:

  s/(struct|union)\s+{([^{}]*)}(\s*[^\s;]\s*)/$1 $3 $2 /g

> +			$cont = 1;
> +		};
> +	};
> +	# Ignore other nested elements, like enums
> +	$members =~ s/({[^\{\}]*})//g;
> +	$nested = $decl_type;

What is the latter good for? I guess the 'nested' trick to suppress
such 'excess' warnings from nested types is no longer needed .. right?

I mean the if-not-in-nested-statement in check_sections():

				if ($nested !~ m/\Q$sects[$sx]\E/) {
				    print STDERR "${file}:$.: warning: " .
					"Excess struct/union/enum/typedef member " .
					"'$sects[$sx]' " .
					"description in '$decl_name'\n";
				    ++$warnings;

So, should we drop this 'nested' stuff also?

I also recommend to first apply all substitutions and after parse
the nested stuff (see my linked patch below).

OK, I spend a day and here is what I have done with the kernel-doc.py:

patch:   https://github.com/return42/linuxdoc/commit/518b4ef65707b4a
impact:  https://github.com/return42/sphkerneldoc/commit/ef5bf69

ATM documentation of nested data types is rare, the most diffs
you will find in the *impact* link are coming from the change of
the prototype, but there are also some good examples e.g.:

  linux_src_doc/include/uapi/linux/videodev2_h.rst

And here is how it is rendered in HTML (e.g. struct-v4l2-plane):

new:  https://h2626237.stratoserver.net/kernel/linux_src_doc/include/uapi/linux/videodev2_h.html#struct-v4l2-plane
old:  https://h2626237.stratoserver.net/kernel_old/linux_src_doc/include/uapi/linux/videodev2_h.html#struct-v4l2-plane

After all I would say that it was a bit hard to implement & test,
but at the end I think, this is really a improvement.

What do you think, can you implement similar in perl (I like
to pass the ball back to you since your perl is much better
than my) .. or should we consider to replace the kernel-doc
parser with the python one [1] ;)

-- Markus --

[1] https://return42.github.io/linuxdoc/linux.html





> 	# ignore members marked private:
> 	$members =~ s/\/\*\s*private:.*?\/\*\s*public:.*?\*\///gosi;
> @@ -2200,6 +2201,7 @@ sub dump_struct($$) {
> 			   'struct',
> 			   {'struct' => $declaration_name,
> 			    'module' => $modulename,
> +			    'definition' => $clause,
> 			    'parameterlist' => \@parameterlist,
> 			    'parameterdescs' => \%parameterdescs,
> 			    'parametertypes' => \%parametertypes,
> 
> 
> 
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-doc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Mauro Carvalho Chehab]

Em Mon, 25 Sep 2017 18:58:14 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > As reference, I'm testing it with:
> > 
> > 	$ scripts/kernel-doc -rst ./drivers/clk/ingenic/cgu.h 
> > 
> > PS.: I'm pretty sure it can be improved. Also, only the ReST output
> > is working properly on this current version.  
> 
> But other outputs are also benefit from. Anyway, my question is; do
> we really need to support other output formats any longer? .. since
> we can reach other outputs from the reST base.

IMHO, the only other output format we currently doesn't support
is man pages.

My vote is to remove everything else, but ReST and manpages.


> > +    $def =~ s/([{,;])/$1\n/g;  
> 
> Do we need to split at comma (',') ? If we do so, we also split lines like:
> 
>      void   (*find_idlest)(struct clk *, void __iomem **, u8 *, u8 *);   

Yes, I noticed. I have a patch removing commas on a place where I
can't access right now. It is not as simple as just removing, as it
needs to do something else for enums to look nice.
                        
> > +	my $clause = $3;  
> 
> IMO 'definition' would be a better name.

Agreed. The version I just sent renamed it to definition :-)

> 
> > +
> > +	# Split nested struct/union elements as newer ones
> > +	my $cont = 1;
> > +	while ($cont) {
> > +		$cont = 0;
> > +		while ($members =~ s/(struct|union)\s+{([^{}]*)}(\s*\S+\s*)\;?/$1 $3; $2 /g) {  
> 
> Very tricky, it took me awhile to understand, but I guess there 
> is a mistake with the semicolon at the end: substitution is *leftmost*
> so \;? at the end will never match (replaced) and the replacement $3;
> inserts one additional semicolon. Try:
> 
>   s/(struct|union)\s+{([^{}]*)}(\s*\S+\s*)/$1 $3 $2 /g
> 
> I also replaced \S with [^\s;] since \S will also match:
> 
> "union { const char *name; u32 rate; }  sys_clk;int (*scale)"
> -----------------------------------------------^------------- 
> 
> here is the expression I ended with:
> 
>   s/(struct|union)\s+{([^{}]*)}(\s*[^\s;]\s*)/$1 $3 $2 /g
> 

It is actually worse than that... there are some places that do
things like (drivers/w1/w1_netlink.h):

	struct w1_netlink_msg
	{
		__u8				type;
		__u8				status;
		__u16				len;
		union {
			__u8			id[8];
			struct w1_mst {
				__u32		id;
				__u32		res;
			} mst;
		} id;
		__u8				data[0];
	};


There, there's a name after struct (w1_mst). While that sounds weird
on my eyes, it is a valid syntax.

Also, on the above example, there are three "id" identifiers:

	union id;
	id.mst;
	id.mst.id.

So, the logic has to allow specifying descriptions for all tree.

The patch I just sent should address it.														

> > +			$cont = 1;
> > +		};
> > +	};
> > +	# Ignore other nested elements, like enums
> > +	$members =~ s/({[^\{\}]*})//g;
> > +	$nested = $decl_type;  
> 
> What is the latter good for? I guess the 'nested' trick to suppress
> such 'excess' warnings from nested types is no longer needed .. right?

For things like:

	enum { foo, bar } type;

Granted, a good documentation should also describe "foo" and "bar",
but that could be easily done by moving enums out of the struct, or
by add descriptions for "foo" and "bar" at @type: markup.

> 
> I mean the if-not-in-nested-statement in check_sections():
> 
> 				if ($nested !~ m/\Q$sects[$sx]\E/) {
> 				    print STDERR "${file}:$.: warning: " .
> 					"Excess struct/union/enum/typedef member " .
> 					"'$sects[$sx]' " .
> 					"description in '$decl_name'\n";
> 				    ++$warnings;
> 
> So, should we drop this 'nested' stuff also?
> 
> I also recommend to first apply all substitutions and after parse
> the nested stuff (see my linked patch below).
> 
> OK, I spend a day and here is what I have done with the kernel-doc.py:
> 
> patch:   https://github.com/return42/linuxdoc/commit/518b4ef65707b4a
> impact:  https://github.com/return42/sphkerneldoc/commit/ef5bf69
> 
> ATM documentation of nested data types is rare, the most diffs
> you will find in the *impact* link are coming from the change of
> the prototype, but there are also some good examples e.g.:
> 
>   linux_src_doc/include/uapi/linux/videodev2_h.rst
> 
> And here is how it is rendered in HTML (e.g. struct-v4l2-plane):
> 
> new:  https://h2626237.stratoserver.net/kernel/linux_src_doc/include/uapi/linux/videodev2_h.html#struct-v4l2-plane
> old:  https://h2626237.stratoserver.net/kernel_old/linux_src_doc/include/uapi/linux/videodev2_h.html#struct-v4l2-plane
> 
> After all I would say that it was a bit hard to implement & test,
> but at the end I think, this is really a improvement.
> 
> What do you think, can you implement similar in perl (I like
> to pass the ball back to you since your perl is much better
> than my) .. or should we consider to replace the kernel-doc
> parser with the python one [1] ;)

Replacing it is up to Jon ;)

I'll take a look on your implementation as well, and see how
can I improve what I did.

> 
> -- Markus --
> 
> [1] https://return42.github.io/linuxdoc/linux.html
> 
> 
> 
> 
> 
> > 	# ignore members marked private:
> > 	$members =~ s/\/\*\s*private:.*?\/\*\s*public:.*?\*\///gosi;
> > @@ -2200,6 +2201,7 @@ sub dump_struct($$) {
> > 			   'struct',
> > 			   {'struct' => $declaration_name,
> > 			    'module' => $modulename,
> > +			    'definition' => $clause,
> > 			    'parameterlist' => \@parameterlist,
> > 			    'parameterdescs' => \%parameterdescs,
> > 			    'parametertypes' => \%parametertypes,
> > 
> > 
> > 
> > Thanks,
> > Mauro
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-doc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> 



Thanks,
Mauro

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Markus Heiser]

> Am 25.09.2017 um 20:41 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:

>>> +			$cont = 1;
>>> +		};
>>> +	};
>>> +	# Ignore other nested elements, like enums
>>> +	$members =~ s/({[^\{\}]*})//g;
>>> +	$nested = $decl_type;  
>> 
>> What is the latter good for? I guess the 'nested' trick to suppress
>> such 'excess' warnings from nested types is no longer needed .. right?
> 
> For things like:
> 
> 	enum { foo, bar } type;
> 
> Granted, a good documentation should also describe "foo" and "bar",
> but that could be easily done by moving enums out of the struct, or
> by add descriptions for "foo" and "bar" at @type: markup.


Hm .. I suppose you are misunderstanding me. I didn't asked about 
$members, I asked about $nested. There is only one place where
$nested is used, and this is in the check_sections function ...

@@ -2531,9 +2527,7 @@ sub check_sections($$$$$$) {
 			} else {
-				if ($nested !~ m/\Q$sects[$sx]\E/) {
-				    print STDERR "${file}:$.: warning: " .
-					"Excess struct/union/enum/typedef member " .
-					"'$sects[$sx]' " .
-					"description in '$decl_name'\n";
-				    ++$warnings;
-				}
+                            print STDERR "${file}:$.: warning: " .
+                                "Excess struct/union/enum/typedef member " .
+                                "'$sects[$sx]' " .
+                                "description in '$decl_name'\n";
+                            ++$warnings;
 			}

Since this is the only place where $nested is use, we can drop all
the occurrence of $nested in the kernel-doc script .. or I'am
totally wrong?

  -- Markus --

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[Mauro Carvalho Chehab]

Em Tue, 26 Sep 2017 14:45:08 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > Am 25.09.2017 um 20:41 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:  
> 
> >>> +			$cont = 1;
> >>> +		};
> >>> +	};
> >>> +	# Ignore other nested elements, like enums
> >>> +	$members =~ s/({[^\{\}]*})//g;
> >>> +	$nested = $decl_type;    
> >> 
> >> What is the latter good for? I guess the 'nested' trick to suppress
> >> such 'excess' warnings from nested types is no longer needed .. right?  
> > 
> > For things like:
> > 
> > 	enum { foo, bar } type;
> > 
> > Granted, a good documentation should also describe "foo" and "bar",
> > but that could be easily done by moving enums out of the struct, or
> > by add descriptions for "foo" and "bar" at @type: markup.  
> 
> 
> Hm .. I suppose you are misunderstanding me. I didn't asked about 
> $members, I asked about $nested. There is only one place where
> $nested is used, and this is in the check_sections function ...
> 
> @@ -2531,9 +2527,7 @@ sub check_sections($$$$$$) {
>  			} else {
> -				if ($nested !~ m/\Q$sects[$sx]\E/) {
> -				    print STDERR "${file}:$.: warning: " .
> -					"Excess struct/union/enum/typedef member " .
> -					"'$sects[$sx]' " .
> -					"description in '$decl_name'\n";
> -				    ++$warnings;
> -				}
> +                            print STDERR "${file}:$.: warning: " .
> +                                "Excess struct/union/enum/typedef member " .
> +                                "'$sects[$sx]' " .
> +                                "description in '$decl_name'\n";
> +                            ++$warnings;
>  			}
> 
> Since this is the only place where $nested is use, we can drop all
> the occurrence of $nested in the kernel-doc script .. or I'am
> totally wrong?

Ah, now I understood you! Yeah, this can be removed. I'll put it into
a separate cleanup patch.

See below.

Regards,
Mauro


[PATCH] scripts: kernel-doc: get rid of $nested parameter

The check_sections() function has a $nested parameter, meant
to identify when a nested struct is present. As we now have
a logic that handles it, get rid of such parameter.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 880a196c7dc7..cff66ee91f2c 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -979,7 +979,6 @@ sub dump_union($$) {
 sub dump_struct($$) {
     my $x = shift;
     my $file = shift;
-    my $nested;
 
     if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
 	my $decl_type = $1;
@@ -1034,11 +1033,9 @@ sub dump_struct($$) {
 
 	# Ignore other nested elements, like enums
 	$members =~ s/({[^\{\}]*})//g;
-	$nested = $decl_type;
-	$nested =~ s/\/\*.*?\*\///gos;
 
 	create_parameterlist($members, ';', $file);
-	check_sections($file, $declaration_name, "struct", $sectcheck, $struct_actual, $nested);
+	check_sections($file, $declaration_name, "struct", $sectcheck, $struct_actual);
 
 	# Adjust declaration for better display
 	$declaration =~ s/([{;])/$1\n/g;
@@ -1334,8 +1331,8 @@ sub push_parameter($$$) {
 	$parametertypes{$param} = $type;
 }
 
-sub check_sections($$$$$$) {
-	my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck, $nested) = @_;
+sub check_sections($$$$$) {
+	my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck) = @_;
 	my @sects = split ' ', $sectcheck;
 	my @prms = split ' ', $prmscheck;
 	my $err;
@@ -1369,14 +1366,6 @@ sub check_sections($$$$$$) {
 					"'$sects[$sx]' " .
 					"description in '$decl_name'\n";
 				++$warnings;
-			} else {
-				if ($nested !~ m/\Q$sects[$sx]\E/) {
-				    print STDERR "${file}:$.: warning: " .
-					"Excess struct/union/enum/typedef member " .
-					"'$sects[$sx]' " .
-					"description in '$decl_name'\n";
-				    ++$warnings;
-				}
 			}
 		}
 	}
@@ -1487,7 +1476,7 @@ sub dump_function($$) {
     }
 
 	my $prms = join " ", @parameterlist;
-	check_sections($file, $declaration_name, "function", $sectcheck, $prms, "");
+	check_sections($file, $declaration_name, "function", $sectcheck, $prms);
 
         # This check emits a lot of warnings at the moment, because many
         # functions don't have a 'Return' doc section. So until the number


Thanks,
Mauro

Re: [PATCH v2] scripts: kernel-doc: fix nexted handling

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4840 bytes --]

Hi Mauro,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.14-rc2 next-20170926]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/scripts-kernel-doc-fix-nexted-handling/20170927-091127
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   kernel/trace/blktrace.c:824: warning: No description found for parameter 'cgid'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef member 'adjust' description in 'cfg80211_bss_selection'

vim +2056 include/net/cfg80211.h

04a773ad Johannes Berg    2009-04-19  2041  
04a773ad Johannes Berg    2009-04-19  2042  /**
38de03d2 Arend van Spriel 2016-03-02  2043   * struct cfg80211_bss_selection - connection parameters for BSS selection.
38de03d2 Arend van Spriel 2016-03-02  2044   *
38de03d2 Arend van Spriel 2016-03-02  2045   * @behaviour: requested BSS selection behaviour.
38de03d2 Arend van Spriel 2016-03-02  2046   * @param: parameters for requestion behaviour.
38de03d2 Arend van Spriel 2016-03-02  2047   * @band_pref: preferred band for %NL80211_BSS_SELECT_ATTR_BAND_PREF.
38de03d2 Arend van Spriel 2016-03-02  2048   * @adjust: parameters for %NL80211_BSS_SELECT_ATTR_RSSI_ADJUST.
38de03d2 Arend van Spriel 2016-03-02  2049   */
38de03d2 Arend van Spriel 2016-03-02  2050  struct cfg80211_bss_selection {
38de03d2 Arend van Spriel 2016-03-02  2051  	enum nl80211_bss_select_attr behaviour;
38de03d2 Arend van Spriel 2016-03-02  2052  	union {
57fbcce3 Johannes Berg    2016-04-12  2053  		enum nl80211_band band_pref;
38de03d2 Arend van Spriel 2016-03-02  2054  		struct cfg80211_bss_select_adjust adjust;
38de03d2 Arend van Spriel 2016-03-02  2055  	} param;
38de03d2 Arend van Spriel 2016-03-02 @2056  };
38de03d2 Arend van Spriel 2016-03-02  2057  

:::::: The code at line 2056 was first introduced by commit
:::::: 38de03d2a28925b489c11546804e2f5418cc17a4 nl80211: add feature for BSS selection support

:::::: TO: Arend van Spriel <arend@broadcom.com>
:::::: CC: Johannes Berg <johannes.berg@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6761 bytes --]