2019-10-04 21:56:18

by J. Neuschäfer

[permalink] [raw]
Subject: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

Without this patch, Sphinx shows "variable arguments" as the description
of the cond argument, rather than the intended description, and prints
the following warnings:

./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'

Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
include/linux/rculist.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index 4158b7212936..61c6728a71f7 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -361,7 +361,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
* @pos: the type * to use as a loop cursor.
* @head: the head for your list.
* @member: the name of the list_head within the struct.
- * @cond: optional lockdep expression if called from non-RCU protection.
+ * @cond...: optional lockdep expression if called from non-RCU protection.
*
* This list-traversal primitive may safely run concurrently with
* the _rcu list-mutation primitives such as list_add_rcu()
@@ -636,7 +636,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* @pos: the type * to use as a loop cursor.
* @head: the head for your list.
* @member: the name of the hlist_node within the struct.
- * @cond: optional lockdep expression if called from non-RCU protection.
+ * @cond...: optional lockdep expression if called from non-RCU protection.
*
* This list-traversal primitive may safely run concurrently with
* the _rcu list-mutation primitives such as hlist_add_head_rcu()
--
2.20.1


2019-10-04 22:28:07

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neusch?fer wrote:
> Without this patch, Sphinx shows "variable arguments" as the description
> of the cond argument, rather than the intended description, and prints
> the following warnings:
>
> ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
>
> Signed-off-by: Jonathan Neusch?fer <[email protected]>

Applied for testing and review, thank you!

Joel, does this look sane to you?

Thanx, Paul

> ---
> include/linux/rculist.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index 4158b7212936..61c6728a71f7 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -361,7 +361,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> * @pos: the type * to use as a loop cursor.
> * @head: the head for your list.
> * @member: the name of the list_head within the struct.
> - * @cond: optional lockdep expression if called from non-RCU protection.
> + * @cond...: optional lockdep expression if called from non-RCU protection.
> *
> * This list-traversal primitive may safely run concurrently with
> * the _rcu list-mutation primitives such as list_add_rcu()
> @@ -636,7 +636,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * @pos: the type * to use as a loop cursor.
> * @head: the head for your list.
> * @member: the name of the hlist_node within the struct.
> - * @cond: optional lockdep expression if called from non-RCU protection.
> + * @cond...: optional lockdep expression if called from non-RCU protection.
> *
> * This list-traversal primitive may safely run concurrently with
> * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> --
> 2.20.1
>

2019-10-04 23:25:46

by J. Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Fri, Oct 04, 2019 at 03:24:39PM -0700, Paul E. McKenney wrote:
> On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neuschäfer wrote:
> > Without this patch, Sphinx shows "variable arguments" as the description
> > of the cond argument, rather than the intended description, and prints
> > the following warnings:
> >
> > ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> > ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'

Hmm, small detail that I didn't realize before: It's actually the
kernel-doc script, not Sphinx, that can't deal with variadic macro
arguments and thus requires this patch.

So it may also be possible to fix the script instead. (I have not
looked into how much work that would be.)


Thanks,
Jonathan Neuschäfer


Attachments:
(No filename) (893.00 B)
signature.asc (849.00 B)
Download all attachments

2019-10-05 13:40:10

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Sat, Oct 05, 2019 at 01:23:28AM +0200, Jonathan Neusch?fer wrote:
> On Fri, Oct 04, 2019 at 03:24:39PM -0700, Paul E. McKenney wrote:
> > On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neusch?fer wrote:
> > > Without this patch, Sphinx shows "variable arguments" as the description
> > > of the cond argument, rather than the intended description, and prints
> > > the following warnings:
> > >
> > > ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> > > ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
>
> Hmm, small detail that I didn't realize before: It's actually the
> kernel-doc script, not Sphinx, that can't deal with variadic macro
> arguments and thus requires this patch.
>
> So it may also be possible to fix the script instead. (I have not
> looked into how much work that would be.)

OK, thank you for letting me know. I will keep your patch for the
moment, but please let me know if the fix can be elsewhere.

Thanx, Paul

2019-10-05 19:37:53

by J. Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Sat, Oct 05, 2019 at 06:33:30AM -0700, Paul E. McKenney wrote:
> On Sat, Oct 05, 2019 at 01:23:28AM +0200, Jonathan Neuschäfer wrote:
> > On Fri, Oct 04, 2019 at 03:24:39PM -0700, Paul E. McKenney wrote:
> > > On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neuschäfer wrote:
> > > > Without this patch, Sphinx shows "variable arguments" as the description
> > > > of the cond argument, rather than the intended description, and prints
> > > > the following warnings:
> > > >
> > > > ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> > > > ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
> >
> > Hmm, small detail that I didn't realize before: It's actually the
> > kernel-doc script, not Sphinx, that can't deal with variadic macro
> > arguments and thus requires this patch.
> >
> > So it may also be possible to fix the script instead. (I have not
> > looked into how much work that would be.)
>
> OK, thank you for letting me know. I will keep your patch for the
> moment, but please let me know if the fix can be elsewhere.
>
> Thanx, Paul

Turns out the actual fix in scripts/kernel-doc is easy enough:

--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1449,6 +1449,10 @@ sub push_parameter($$$$) {
# handles unnamed variable parameters
$param = "...";
}
+ elsif ($param =~ /\w\.\.\.$/) {
+ # for named variable parameters of the form `x...`, remove the dots
+ $param =~ s/\.\.\.$//;
+ }
if (!defined $parameterdescs{$param} || $parameterdescs{$param} eq "") {
$parameterdescs{$param} = "variable arguments";
}

... but there are other macros in the code base that are documented
using the 'x...' syntax, so I guess it's best to take my initial patch
(or something similar) now, and I'll fix kernel-doc later, in a longer
patchset that also cleans up the fallout.


Thanks,
Jonathan Neuschäfer


Attachments:
(No filename) (2.02 kB)
signature.asc (849.00 B)
Download all attachments

2019-10-05 19:39:06

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Sat, Oct 05, 2019 at 09:31:23PM +0200, Jonathan Neusch?fer wrote:
> On Sat, Oct 05, 2019 at 06:33:30AM -0700, Paul E. McKenney wrote:
> > On Sat, Oct 05, 2019 at 01:23:28AM +0200, Jonathan Neusch?fer wrote:
> > > On Fri, Oct 04, 2019 at 03:24:39PM -0700, Paul E. McKenney wrote:
> > > > On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neusch?fer wrote:
> > > > > Without this patch, Sphinx shows "variable arguments" as the description
> > > > > of the cond argument, rather than the intended description, and prints
> > > > > the following warnings:
> > > > >
> > > > > ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> > > > > ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
> > >
> > > Hmm, small detail that I didn't realize before: It's actually the
> > > kernel-doc script, not Sphinx, that can't deal with variadic macro
> > > arguments and thus requires this patch.
> > >
> > > So it may also be possible to fix the script instead. (I have not
> > > looked into how much work that would be.)
> >
> > OK, thank you for letting me know. I will keep your patch for the
> > moment, but please let me know if the fix can be elsewhere.
> >
> > Thanx, Paul
>
> Turns out the actual fix in scripts/kernel-doc is easy enough:
>
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1449,6 +1449,10 @@ sub push_parameter($$$$) {
> # handles unnamed variable parameters
> $param = "...";
> }
> + elsif ($param =~ /\w\.\.\.$/) {
> + # for named variable parameters of the form `x...`, remove the dots
> + $param =~ s/\.\.\.$//;
> + }
> if (!defined $parameterdescs{$param} || $parameterdescs{$param} eq "") {
> $parameterdescs{$param} = "variable arguments";
> }
>
> ... but there are other macros in the code base that are documented
> using the 'x...' syntax, so I guess it's best to take my initial patch
> (or something similar) now, and I'll fix kernel-doc later, in a longer
> patchset that also cleans up the fallout.

You got it! ;-)

Thanx, Paul

2019-10-06 23:45:49

by Joel Fernandes

[permalink] [raw]
Subject: Re: [PATCH] rculist: Describe variadic macro argument in a Sphinx-compatible way

On Fri, Oct 04, 2019 at 03:24:39PM -0700, Paul E. McKenney wrote:
> On Fri, Oct 04, 2019 at 11:54:02PM +0200, Jonathan Neusch?fer wrote:
> > Without this patch, Sphinx shows "variable arguments" as the description
> > of the cond argument, rather than the intended description, and prints
> > the following warnings:
> >
> > ./include/linux/rculist.h:374: warning: Excess function parameter 'cond' description in 'list_for_each_entry_rcu'
> > ./include/linux/rculist.h:651: warning: Excess function parameter 'cond' description in 'hlist_for_each_entry_rcu'
> >
> > Signed-off-by: Jonathan Neusch?fer <[email protected]>
>
> Applied for testing and review, thank you!
>
> Joel, does this look sane to you?

Sorry for late reply due to weekend. Yes, looks good to me.

thanks,

- Joel