2020-10-30 14:49:23

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 0/2] kernel-doc: Handle function typedefs

Hi all,

QEMU has been using kernel-doc for a while and we're very happy with it. :)
These two patches are relatively simple regex changes that were done to
support QEMU header files; they handle function typedefs (i.e. not
function _pointer_ typedefs).

These are basically the only difference between Linux and QEMU kernel-doc,
so I thought I'd just send them out and see what you people think.

Paolo

Eduardo Habkost (2):
kernel-doc: Handle function typedefs that return pointers
kernel-doc: Handle function typedefs without asterisks

scripts/kernel-doc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

--
2.28.0


2020-10-30 14:50:20

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

From: Eduardo Habkost <[email protected]>

Example of typedef that was not parsed by kernel-doc:

typedef void (ObjectUnparent)(Object *obj);

Signed-off-by: Eduardo Habkost <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 5b5caa7642f7..1a9c918aa653 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1302,7 +1302,7 @@ sub dump_typedef($$) {
$x =~ s@/\*.*?\*/@@gos; # strip comments.

# Parse function prototypes
- if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
+ if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
$x =~ /typedef\s+(\w+\s*\**)\s*(\w\S+)\s*\s*\((.*)\);/) {

# Function typedefs
--
2.28.0

2020-10-30 14:50:52

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 1/2] kernel-doc: Handle function typedefs that return pointers

From: Eduardo Habkost <[email protected]>

One example that was not being parsed correctly by kernel-doc is:

typedef Object *(ObjectPropertyResolve)(Object *obj,
void *opaque,
const char *part);

Signed-off-by: Eduardo Habkost <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
scripts/kernel-doc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index f68d76dd97ba..5b5caa7642f7 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1302,8 +1302,8 @@ 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*\**)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
+ $x =~ /typedef\s+(\w+\s*\**)\s*(\w\S+)\s*\s*\((.*)\);/) {

# Function typedefs
$return_type = $1;
--
2.28.0


2020-11-13 22:24:14

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

On Fri, 30 Oct 2020 15:47:13 +0100
Paolo Bonzini <[email protected]> wrote:

> From: Eduardo Habkost <[email protected]>
>
> Example of typedef that was not parsed by kernel-doc:
>
> typedef void (ObjectUnparent)(Object *obj);
>
> Signed-off-by: Eduardo Habkost <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>

So as you've undoubtedly noticed, reading those kernel-doc regexes is ... a
wee bit on the painful side. Trying to compare two of them in a patch to
figure out what you have done is even worse. I suspect we want these
patches, but can you please supply a changelog that describes the change?

> ---
> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 5b5caa7642f7..1a9c918aa653 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1302,7 +1302,7 @@ sub dump_typedef($$) {
> $x =~ s@/\*.*?\*/@@gos; # strip comments.
>
> # Parse function prototypes
> - if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*\s*(\w\S+)\s*\)\s*\((.*)\);/ ||
> + if ($x =~ /typedef\s+(\w+\s*\**)\s*\(\*?\s*(\w\S+)\s*\)\s*\((.*)\);/ ||

Here it appears that you are making the "*" before the function-pointer
name optional, right? It really would help to say so in the changelog.

This is true for the other patch as well.

> $x =~ /typedef\s+(\w+\s*\**)\s*(\w\S+)\s*\s*\((.*)\);/) {
>
> # Function typedefs

Thanks,

jon

2020-11-13 22:40:13

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

On 13/11/20 23:21, Jonathan Corbet wrote:
>>
>> Signed-off-by: Eduardo Habkost<[email protected]>
>> Signed-off-by: Paolo Bonzini<[email protected]>
> So as you've undoubtedly noticed, reading those kernel-doc regexes is ... a
> wee bit on the painful side. Trying to compare two of them in a patch to
> figure out what you have done is even worse. I suspect we want these
> patches, but can you please supply a changelog that describes the change?
>

Seems like some of Mauro's recent patches take care of the same thing.
I'm going to update QEMU's kernel-doc, and if there's anything left to
do I'll resend.

Thanks,

Paolo

2020-11-13 22:42:02

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

On Fri, Nov 13, 2020 at 03:21:06PM -0700, Jonathan Corbet wrote:
> On Fri, 30 Oct 2020 15:47:13 +0100
> Paolo Bonzini <[email protected]> wrote:
>
> > From: Eduardo Habkost <[email protected]>
> >
> > Example of typedef that was not parsed by kernel-doc:
> >
> > typedef void (ObjectUnparent)(Object *obj);
> >
> > Signed-off-by: Eduardo Habkost <[email protected]>
> > Signed-off-by: Paolo Bonzini <[email protected]>
>
> So as you've undoubtedly noticed, reading those kernel-doc regexes is ... a
> wee bit on the painful side. Trying to compare two of them in a patch to
> figure out what you have done is even worse. I suspect we want these
> patches, but can you please supply a changelog that describes the change?

Better ... can we have a test suite for the regexes and make patches to
them include updates to the test suite? They have clearly passed the
point of human understanding ;-)

2020-11-17 21:29:35

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

On Fri, Nov 13, 2020 at 10:39:12PM +0000, Matthew Wilcox wrote:
> On Fri, Nov 13, 2020 at 03:21:06PM -0700, Jonathan Corbet wrote:
> > On Fri, 30 Oct 2020 15:47:13 +0100
> > Paolo Bonzini <[email protected]> wrote:
> >
> > > From: Eduardo Habkost <[email protected]>
> > >
> > > Example of typedef that was not parsed by kernel-doc:
> > >
> > > typedef void (ObjectUnparent)(Object *obj);
> > >
> > > Signed-off-by: Eduardo Habkost <[email protected]>
> > > Signed-off-by: Paolo Bonzini <[email protected]>
> >
> > So as you've undoubtedly noticed, reading those kernel-doc regexes is ... a
> > wee bit on the painful side. Trying to compare two of them in a patch to
> > figure out what you have done is even worse. I suspect we want these
> > patches, but can you please supply a changelog that describes the change?
>
> Better ... can we have a test suite for the regexes and make patches to
> them include updates to the test suite? They have clearly passed the
> point of human understanding ;-)

Would a simple black box test script like this be desirable?

Signed-off-by: Eduardo Habkost <[email protected]>
---
scripts/kernel-doc-test-case.h | 89 ++++++++++++
scripts/kernel-doc-test-case.rst.expected | 167 ++++++++++++++++++++++
scripts/kernel-doc-test.sh | 15 ++
3 files changed, 271 insertions(+)
create mode 100644 scripts/kernel-doc-test-case.h
create mode 100644 scripts/kernel-doc-test-case.rst.expected
create mode 100755 scripts/kernel-doc-test.sh

diff --git a/scripts/kernel-doc-test-case.h b/scripts/kernel-doc-test-case.h
new file mode 100644
index 0000000000000..5cea705d85392
--- /dev/null
+++ b/scripts/kernel-doc-test-case.h
@@ -0,0 +1,89 @@
+/**
+ * DOC: kernel-doc test case
+ *
+ * ``kernel-doc-test-case.h`` contains a series of declarations
+ * and kernel-doc comments. The expected kernel-doc output can be
+ * found at ``kernel-doc-test-case.rst.expected``.
+ */
+
+/**
+ * typedef void_func_ptr - pointer to a function
+ * @a: first argument
+ * @b: second argument
+ */
+typedef void (*void_func_ptr)(int a, struct struct_name1 *b);
+
+/**
+ * typedef int_ptr_func_ptr - a pointer to a function returning a pointer
+ * @a: argument
+ */
+typedef int *(*int_ptr_func_ptr)(int a);
+
+/**
+ * typedef func_par - a function, with parenthesis
+ * @a: argument
+ *
+ * A typedef for a function type (not a function pointer), with
+ * parenthesis around the function name.
+ */
+typedef void (func_par)(int a);
+
+/**
+ * struct struct_name1 - a struct
+ * @i: an int field
+ * @j: an int pointer
+ * @u: an union field
+ * @sptr: pointer to a `struct_name1`
+ *
+ * A simple struct with multiple fields.
+ *
+ * Here's a reference to another struct type: &struct struct_name2.
+ */
+struct struct_name1 {
+ int i, *j;
+ union {
+ int i;
+ const char *s;
+ } u;
+ struct struct_name1 *sptr;
+ /**
+ * @field_with_inline_doc: another way to document struct fields
+ *
+ * This field is documented inside the struct definition,
+ * closer to the field declaration instead the doc comment at
+ * the top.
+ */
+ int field_with_inline_doc;
+ /**
+ * @func: a function pointer
+ *
+ * Parsing a function pointer field involves some tricks to handle
+ * the commas properly.
+ */
+ int (*func)(int x, struct struct_name1 *p);
+ /** @bitmap: a bitmap */
+ DECLARE_BITMAP(bitmap, 128);
+};
+
+/**
+ * struct struct_name2 - another struct
+ * @x: first field
+ * @y: second field
+ * @another: another struct
+ *
+ * This struct is defined inside a typedef declaration.
+ */
+typedef struct struct_name2 {
+ int x, y;
+ struct struct_name1 another;
+} struct_name2;
+
+/**
+ * SOME_MACRO - a macro that takes a few arguments
+ * @a: first argument
+ * @b: second argument
+ */
+#define SOME_MACRO(a, b) \
+ { multi_line_macro_definition(a); \
+ second_line(b); \
+ }
diff --git a/scripts/kernel-doc-test-case.rst.expected b/scripts/kernel-doc-test-case.rst.expected
new file mode 100644
index 0000000000000..4f68931121bb7
--- /dev/null
+++ b/scripts/kernel-doc-test-case.rst.expected
@@ -0,0 +1,167 @@
+**kernel-doc test case**
+
+
+``kernel-doc-test-case.h`` contains a series of declarations
+and kernel-doc comments. The expected kernel-doc output can be
+found at ``kernel-doc-test-case.rst.expected``.
+
+.. c:macro:: void_func_ptr
+
+ **Typedef**: pointer to a function
+
+
+**Syntax**
+
+ ``void void_func_ptr (int a, struct struct_name1 *b)``
+
+**Parameters**
+
+``int a``
+ first argument
+
+``struct struct_name1 *b``
+ second argument
+
+
+.. c:macro:: int_ptr_func_ptr
+
+ **Typedef**: a pointer to a function returning a pointer
+
+
+**Syntax**
+
+ ``int * int_ptr_func_ptr (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+
+.. c:macro:: func_par
+
+ **Typedef**: a function, with parenthesis
+
+
+**Syntax**
+
+ ``void func_par (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+**Description**
+
+A typedef for a function type (not a function pointer), with
+parenthesis around the function name.
+
+
+
+
+.. c:struct:: struct_name1
+
+ a struct
+
+**Definition**
+
+::
+
+ struct struct_name1 {
+ int i, *j;
+ union {
+ int i;
+ const char *s;
+ } u;
+ struct struct_name1 *sptr;
+ int field_with_inline_doc;
+ int (*func)(int x, struct struct_name1 *p);
+ unsigned long bitmap[BITS_TO_LONGS(128)];
+ };
+
+**Members**
+
+``i``
+ an int field
+
+``j``
+ an int pointer
+
+``u``
+ an union field
+
+``sptr``
+ pointer to a `struct_name1`
+
+``field_with_inline_doc``
+ another way to document struct fields
+
+ This field is documented inside the struct definition,
+ closer to the field declaration instead the doc comment at
+ the top.
+
+``func``
+ a function pointer
+
+ Parsing a function pointer field involves some tricks to handle
+ the commas properly.
+
+``bitmap``
+ a bitmap
+
+
+**Description**
+
+A simple struct with multiple fields.
+
+Here's a reference to another struct type: :c:type:`struct struct_name2 <struct_name2>`.
+
+
+
+
+.. c:struct:: struct_name2
+
+ another struct
+
+**Definition**
+
+::
+
+ struct struct_name2 {
+ int x, y;
+ struct struct_name1 another;
+ };
+
+**Members**
+
+``x``
+ first field
+
+``y``
+ second field
+
+``another``
+ another struct
+
+
+**Description**
+
+This struct is defined inside a typedef declaration.
+
+
+.. c:macro:: SOME_MACRO
+
+``SOME_MACRO (a, b)``
+
+ a macro that takes a few arguments
+
+**Parameters**
+
+``a``
+ first argument
+
+``b``
+ second argument
+
+
diff --git a/scripts/kernel-doc-test.sh b/scripts/kernel-doc-test.sh
new file mode 100755
index 0000000000000..4c96592649451
--- /dev/null
+++ b/scripts/kernel-doc-test.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+set -e
+mydir="$(dirname "$0")"
+kerneldoc="$mydir/kernel-doc"
+
+tmp="$(mktemp -d)"
+trap 'rm -rf "$tmp"' EXIT
+
+"$kerneldoc" -sphinx-version 3.2.1 -rst "$mydir/kernel-doc-test-case.h" > "$tmp/kernel-doc-test-case.rst.actual" 2>&1
+if diff -u "$mydir/kernel-doc-test-case.rst.expected" "$tmp/kernel-doc-test-case.rst.actual";then
+ echo OK
+else
+ echo kernel-doc output mismatch
+ exit 1
+fi
--
2.28.0

--
Eduardo

2020-11-17 21:35:32

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 2/2] kernel-doc: Handle function typedefs without asterisks

On Tue, Nov 17, 2020 at 04:24:52PM -0500, Eduardo Habkost wrote:
> On Fri, Nov 13, 2020 at 10:39:12PM +0000, Matthew Wilcox wrote:
> > Better ... can we have a test suite for the regexes and make patches to
> > them include updates to the test suite? They have clearly passed the
> > point of human understanding ;-)
>
> Would a simple black box test script like this be desirable?

I think this is fantastic! Yes please!

Can I add one more test case?

/**
* radix_tree_lookup_slot - lookup a slot in a radix tree
* @root: radix tree root
* @index: index key
*
* Returns: the slot corresponding to the position @index in the
* radix tree @root. This is useful for update-if-exists operations.
*
* This function can be called under rcu_read_lock iff the slot is not
* modified by radix_tree_replace_slot(), otherwise it must be called
* exclusive from other writers. Any dereference of the slot must be done
* using radix_tree_deref_slot().
*/
void __rcu **radix_tree_lookup_slot(const struct radix_tree_root *root,
unsigned long index)
{ }

(we used to have a problem with multiple '*' in the return type, and
we've also had problems with adornments like __rcu)

2020-11-17 22:38:12

by Eduardo Habkost

[permalink] [raw]
Subject: [RFC] Add kernel-doc test script

Add a kernel-doc test script to tools/testing/kernel-doc.

radix_tree_lookup_slot test case provided by Matthew Wilcox.

Signed-off-by: Eduardo Habkost <[email protected]>
---
tools/testing/kernel-doc/test-case.h | 111 ++++++++++
.../testing/kernel-doc/test-case.man.expected | 150 ++++++++++++++
.../kernel-doc/test-case.none.expected | 0
.../kernel-doc/test-case.rst2.expected | 195 ++++++++++++++++++
.../kernel-doc/test-case.rst3.expected | 195 ++++++++++++++++++
tools/testing/kernel-doc/test.sh | 90 ++++++++
6 files changed, 741 insertions(+)
create mode 100644 tools/testing/kernel-doc/test-case.h
create mode 100644 tools/testing/kernel-doc/test-case.man.expected
create mode 100644 tools/testing/kernel-doc/test-case.none.expected
create mode 100644 tools/testing/kernel-doc/test-case.rst2.expected
create mode 100644 tools/testing/kernel-doc/test-case.rst3.expected
create mode 100755 tools/testing/kernel-doc/test.sh

diff --git a/tools/testing/kernel-doc/test-case.h b/tools/testing/kernel-doc/test-case.h
new file mode 100644
index 0000000000000..6d4e1f6b99631
--- /dev/null
+++ b/tools/testing/kernel-doc/test-case.h
@@ -0,0 +1,111 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/**
+ * DOC: kernel-doc test case
+ *
+ * ``test-case.h`` contains a series of declarations and
+ * kernel-doc comments. The expected kernel-doc output can be
+ * found at ``test-case.rst.expected``.
+ */
+
+/**
+ * typedef void_func_ptr - pointer to a function
+ * @a: first argument
+ * @b: second argument
+ */
+typedef void (*void_func_ptr)(int a, struct struct_name1 *b);
+
+/**
+ * typedef int_ptr_func_ptr - a pointer to a function returning a pointer
+ * @a: argument
+ */
+typedef int *(*int_ptr_func_ptr)(int a);
+
+/**
+ * typedef func_par - a function, with parenthesis
+ * @a: argument
+ *
+ * A typedef for a function type (not a function pointer), wit
+ * parenthesis around the function name.
+ */
+typedef void (func_par)(int a);
+
+/**
+ * struct struct_name1 - a struct
+ * @i: an int field
+ * @j: an int pointer
+ * @u: a union field
+ * @sptr: pointer to a `struct_name1`
+ *
+ * A simple struct with multiple fields.
+ *
+ * Here's a reference to another struct type: &struct struct_name2.
+ */
+struct struct_name1 {
+ int i, *j;
+ union {
+ int i;
+ const char *s;
+ } u;
+ struct struct_name1 *sptr;
+ /**
+ * @field_with_inline_doc: another way to document struct fields
+ *
+ * This field is documented inside the struct definition,
+ * closer to the field declaration instead the doc comment at
+ * the top.
+ */
+ int field_with_inline_doc;
+ /**
+ * @func: a function pointer
+ *
+ * Parsing a function pointer field involves some tricks to handle
+ * the commas properly.
+ */
+ int (*func)(int x, struct struct_name1 *p);
+ /** @bitmap: a bitmap */
+ DECLARE_BITMAP(bitmap, 128);
+};
+
+/**
+ * struct struct_name2 - another struct
+ * @x: first field
+ * @y: second field
+ * @another: another struct
+ *
+ * This struct is defined inside a typedef declaration.
+ */
+typedef struct struct_name2 {
+ int x, y;
+ struct struct_name1 another;
+} struct_name2;
+
+/**
+ * radix_tree_lookup_slot - lookup a slot in a radix tree
+ * @root: radix tree root
+ * @index: index key
+ *
+ * Returns: the slot corresponding to the position @index in the
+ * radix tree @root. This is useful for update-if-exists operations.
+ *
+ * This function can be called under rcu_read_lock iff the slot is not
+ * modified by radix_tree_replace_slot(), otherwise it must be called
+ * exclusive from other writers. Any dereference of the slot must be done
+ * using radix_tree_deref_slot().
+ *
+ * We used to have a problem with multiple ``*`` in the return type, and
+ * we've also had problems with adornments like __rcu).
+ */
+void __rcu **radix_tree_lookup_slot(const struct radix_tree_root *root,
+ unsigned long index)
+{ }
+
+/**
+ * SOME_MACRO - a macro that takes a few arguments
+ * @a: first argument
+ * @b: second argument
+ */
+#define SOME_MACRO(a, b) \
+ { multi_line_macro_definition(a); \
+ second_line(b); \
+ }
diff --git a/tools/testing/kernel-doc/test-case.man.expected b/tools/testing/kernel-doc/test-case.man.expected
new file mode 100644
index 0000000000000..6be2583469f04
--- /dev/null
+++ b/tools/testing/kernel-doc/test-case.man.expected
@@ -0,0 +1,150 @@
+.TH "Kernel API" 9 "Kernel API" "August 1991" "API Manual" LINUX
+.SH "kernel-doc test case"
+``test-case.h contains a series of declarations and
+kernel-doc comments. The expected kernel-doc output can be
+found at test-case.rst.expected``.
+.TH "void_func_ptr" 9 "void_func_ptr" "August 1991" "Kernel Hacker's Manual" LINUX
+.SH NAME
+void_func_ptr \- pointer to a function
+.SH SYNOPSIS
+.B "void" void_func_ptr
+.BI "(int a " ","
+.BI "struct struct_name1 *b " ");"
+.SH ARGUMENTS
+.IP "a" 12
+first argument
+.IP "b" 12
+second argument
+.TH "int_ptr_func_ptr" 9 "int_ptr_func_ptr" "August 1991" "Kernel Hacker's Manual" LINUX
+.SH NAME
+int_ptr_func_ptr \- a pointer to a function returning a pointer
+.SH SYNOPSIS
+.B "int *" int_ptr_func_ptr
+.BI "(int a " ");"
+.SH ARGUMENTS
+.IP "a" 12
+argument
+.TH "func_par" 9 "func_par" "August 1991" "Kernel Hacker's Manual" LINUX
+.SH NAME
+func_par \- a function, with parenthesis
+.SH SYNOPSIS
+.B "void" func_par
+.BI "(int a " ");"
+.SH ARGUMENTS
+.IP "a" 12
+argument
+.SH "DESCRIPTION"
+A typedef for a function type (not a function pointer), wit
+parenthesis around the function name.
+.TH "Kernel API" 9 "struct struct_name1" "August 1991" "API Manual" LINUX
+.SH NAME
+struct struct_name1 \- a struct
+.SH SYNOPSIS
+struct struct_name1 {
+.br
+.BI " int i, *j;"
+.br
+.BI " union {"
+.br
+.BI " int i;"
+.br
+.BI " const char *s;"
+.br
+.BI " } u;"
+.br
+.BI " struct struct_name1 *sptr;"
+.br
+.BI " int field_with_inline_doc;"
+.br
+.BI " int (*func)(int x, struct struct_name1 *p);"
+.br
+.BI " unsigned long bitmap[BITS_TO_LONGS(128)];"
+.br
+.BI "
+};
+.br
+
+.SH Members
+.IP "i" 12
+an int field
+.IP "j" 12
+an int pointer
+.IP "u" 12
+a union field
+.IP "sptr" 12
+pointer to a `struct_name1`
+.IP "field_with_inline_doc" 12
+another way to document struct fields
+
+This field is documented inside the struct definition,
+closer to the field declaration instead the doc comment at
+the top.
+.IP "func" 12
+a function pointer
+
+Parsing a function pointer field involves some tricks to handle
+the commas properly.
+.IP "bitmap" 12
+a bitmap
+.SH "Description"
+A simple struct with multiple fields.
+
+Here's a reference to another struct type: \fIstruct struct_name2\fP.
+.TH "Kernel API" 9 "struct struct_name2" "August 1991" "API Manual" LINUX
+.SH NAME
+struct struct_name2 \- another struct
+.SH SYNOPSIS
+struct struct_name2 {
+.br
+.BI " int x, y;"
+.br
+.BI " struct struct_name1 another;"
+.br
+.BI "
+};
+.br
+
+.SH Members
+.IP "x" 12
+first field
+.IP "y" 12
+second field
+.IP "another" 12
+another struct
+.SH "Description"
+This struct is defined inside a typedef declaration.
+.TH "radix_tree_lookup_slot" 9 "radix_tree_lookup_slot" "August 1991" "Kernel Hacker's Manual" LINUX
+.SH NAME
+radix_tree_lookup_slot \- lookup a slot in a radix tree
+.SH SYNOPSIS
+.B "void __rcu **" radix_tree_lookup_slot
+.BI "(const struct radix_tree_root *root " ","
+.BI "unsigned long index " ");"
+.SH ARGUMENTS
+.IP "root" 12
+radix tree root
+.IP "index" 12
+index key
+.SH "RETURN"
+the slot corresponding to the position \fIindex\fP in the
+radix tree \fIroot\fP. This is useful for update-if-exists operations.
+.SH "DESCRIPTION"
+This function can be called under rcu_read_lock iff the slot is not
+modified by \fBradix_tree_replace_slot\fP, otherwise it must be called
+exclusive from other writers. Any dereference of the slot must be done
+using \fBradix_tree_deref_slot\fP.
+
+We used to have a problem with multiple ``*`` in the return type, and
+we've also had problems with adornments like __rcu).
+.TH "SOME_MACRO" 9 "SOME_MACRO" "August 1991" "Kernel Hacker's Manual" LINUX
+.SH NAME
+SOME_MACRO \- a macro that takes a few arguments
+.SH SYNOPSIS
+.B "SOME_MACRO
+.BI "(a " ","
+.BI "b " ");"
+.SH ARGUMENTS
+.IP "a" 12
+first argument
+.IP "b" 12
+second argument
diff --git a/tools/testing/kernel-doc/test-case.none.expected b/tools/testing/kernel-doc/test-case.none.expected
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/tools/testing/kernel-doc/test-case.rst2.expected b/tools/testing/kernel-doc/test-case.rst2.expected
new file mode 100644
index 0000000000000..66ae737a808f2
--- /dev/null
+++ b/tools/testing/kernel-doc/test-case.rst2.expected
@@ -0,0 +1,195 @@
+**kernel-doc test case**
+
+
+``test-case.h`` contains a series of declarations and
+kernel-doc comments. The expected kernel-doc output can be
+found at ``test-case.rst.expected``.
+
+.. c:macro:: void_func_ptr
+
+ **Typedef**: pointer to a function
+
+
+**Syntax**
+
+ ``void void_func_ptr (int a, struct struct_name1 *b)``
+
+**Parameters**
+
+``int a``
+ first argument
+
+``struct struct_name1 *b``
+ second argument
+
+
+.. c:macro:: int_ptr_func_ptr
+
+ **Typedef**: a pointer to a function returning a pointer
+
+
+**Syntax**
+
+ ``int * int_ptr_func_ptr (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+
+.. c:macro:: func_par
+
+ **Typedef**: a function, with parenthesis
+
+
+**Syntax**
+
+ ``void func_par (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+**Description**
+
+A typedef for a function type (not a function pointer), wit
+parenthesis around the function name.
+
+
+
+
+.. c:struct:: struct_name1
+
+ a struct
+
+**Definition**
+
+::
+
+ struct struct_name1 {
+ int i, *j;
+ union {
+ int i;
+ const char *s;
+ } u;
+ struct struct_name1 *sptr;
+ int field_with_inline_doc;
+ int (*func)(int x, struct struct_name1 *p);
+ unsigned long bitmap[BITS_TO_LONGS(128)];
+ };
+
+**Members**
+
+``i``
+ an int field
+
+``j``
+ an int pointer
+
+``u``
+ a union field
+
+``sptr``
+ pointer to a `struct_name1`
+
+``field_with_inline_doc``
+ another way to document struct fields
+
+ This field is documented inside the struct definition,
+ closer to the field declaration instead the doc comment at
+ the top.
+
+``func``
+ a function pointer
+
+ Parsing a function pointer field involves some tricks to handle
+ the commas properly.
+
+``bitmap``
+ a bitmap
+
+
+**Description**
+
+A simple struct with multiple fields.
+
+Here's a reference to another struct type: :c:type:`struct struct_name2 <struct_name2>`.
+
+
+
+
+.. c:struct:: struct_name2
+
+ another struct
+
+**Definition**
+
+::
+
+ struct struct_name2 {
+ int x, y;
+ struct struct_name1 another;
+ };
+
+**Members**
+
+``x``
+ first field
+
+``y``
+ second field
+
+``another``
+ another struct
+
+
+**Description**
+
+This struct is defined inside a typedef declaration.
+
+
+.. c:function:: void __rcu ** radix_tree_lookup_slot (const struct radix_tree_root *root, unsigned long index)
+
+ lookup a slot in a radix tree
+
+**Parameters**
+
+``const struct radix_tree_root *root``
+ radix tree root
+
+``unsigned long index``
+ index key
+
+**Return**
+
+the slot corresponding to the position **index** in the
+radix tree **root**. This is useful for update-if-exists operations.
+
+**Description**
+
+This function can be called under rcu_read_lock iff the slot is not
+modified by radix_tree_replace_slot(), otherwise it must be called
+exclusive from other writers. Any dereference of the slot must be done
+using radix_tree_deref_slot().
+
+We used to have a problem with multiple ``*`` in the return type, and
+we've also had problems with adornments like __rcu).
+
+
+.. c:macro:: SOME_MACRO
+
+``SOME_MACRO (a, b)``
+
+ a macro that takes a few arguments
+
+**Parameters**
+
+``a``
+ first argument
+
+``b``
+ second argument
+
+
diff --git a/tools/testing/kernel-doc/test-case.rst3.expected b/tools/testing/kernel-doc/test-case.rst3.expected
new file mode 100644
index 0000000000000..66ae737a808f2
--- /dev/null
+++ b/tools/testing/kernel-doc/test-case.rst3.expected
@@ -0,0 +1,195 @@
+**kernel-doc test case**
+
+
+``test-case.h`` contains a series of declarations and
+kernel-doc comments. The expected kernel-doc output can be
+found at ``test-case.rst.expected``.
+
+.. c:macro:: void_func_ptr
+
+ **Typedef**: pointer to a function
+
+
+**Syntax**
+
+ ``void void_func_ptr (int a, struct struct_name1 *b)``
+
+**Parameters**
+
+``int a``
+ first argument
+
+``struct struct_name1 *b``
+ second argument
+
+
+.. c:macro:: int_ptr_func_ptr
+
+ **Typedef**: a pointer to a function returning a pointer
+
+
+**Syntax**
+
+ ``int * int_ptr_func_ptr (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+
+.. c:macro:: func_par
+
+ **Typedef**: a function, with parenthesis
+
+
+**Syntax**
+
+ ``void func_par (int a)``
+
+**Parameters**
+
+``int a``
+ argument
+
+**Description**
+
+A typedef for a function type (not a function pointer), wit
+parenthesis around the function name.
+
+
+
+
+.. c:struct:: struct_name1
+
+ a struct
+
+**Definition**
+
+::
+
+ struct struct_name1 {
+ int i, *j;
+ union {
+ int i;
+ const char *s;
+ } u;
+ struct struct_name1 *sptr;
+ int field_with_inline_doc;
+ int (*func)(int x, struct struct_name1 *p);
+ unsigned long bitmap[BITS_TO_LONGS(128)];
+ };
+
+**Members**
+
+``i``
+ an int field
+
+``j``
+ an int pointer
+
+``u``
+ a union field
+
+``sptr``
+ pointer to a `struct_name1`
+
+``field_with_inline_doc``
+ another way to document struct fields
+
+ This field is documented inside the struct definition,
+ closer to the field declaration instead the doc comment at
+ the top.
+
+``func``
+ a function pointer
+
+ Parsing a function pointer field involves some tricks to handle
+ the commas properly.
+
+``bitmap``
+ a bitmap
+
+
+**Description**
+
+A simple struct with multiple fields.
+
+Here's a reference to another struct type: :c:type:`struct struct_name2 <struct_name2>`.
+
+
+
+
+.. c:struct:: struct_name2
+
+ another struct
+
+**Definition**
+
+::
+
+ struct struct_name2 {
+ int x, y;
+ struct struct_name1 another;
+ };
+
+**Members**
+
+``x``
+ first field
+
+``y``
+ second field
+
+``another``
+ another struct
+
+
+**Description**
+
+This struct is defined inside a typedef declaration.
+
+
+.. c:function:: void __rcu ** radix_tree_lookup_slot (const struct radix_tree_root *root, unsigned long index)
+
+ lookup a slot in a radix tree
+
+**Parameters**
+
+``const struct radix_tree_root *root``
+ radix tree root
+
+``unsigned long index``
+ index key
+
+**Return**
+
+the slot corresponding to the position **index** in the
+radix tree **root**. This is useful for update-if-exists operations.
+
+**Description**
+
+This function can be called under rcu_read_lock iff the slot is not
+modified by radix_tree_replace_slot(), otherwise it must be called
+exclusive from other writers. Any dereference of the slot must be done
+using radix_tree_deref_slot().
+
+We used to have a problem with multiple ``*`` in the return type, and
+we've also had problems with adornments like __rcu).
+
+
+.. c:macro:: SOME_MACRO
+
+``SOME_MACRO (a, b)``
+
+ a macro that takes a few arguments
+
+**Parameters**
+
+``a``
+ first argument
+
+``b``
+ second argument
+
+
diff --git a/tools/testing/kernel-doc/test.sh b/tools/testing/kernel-doc/test.sh
new file mode 100755
index 0000000000000..37042c5453823
--- /dev/null
+++ b/tools/testing/kernel-doc/test.sh
@@ -0,0 +1,90 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020, Red Hat Inc.
+# Author: Eduardo Habkost <[email protected]>
+#
+# Black box test script for kernel-doc
+
+set -e
+mydir="$(dirname "$0")"
+
+usage()
+{
+ echo "Usage: $0 [--refresh] [--kernel-doc KERNELDOC_BINARY]"
+}
+
+refresh=no
+kerneldoc="$mydir/../../../scripts/kernel-doc"
+while [ "$#" -gt 0 ];do
+ case "$1" in
+ -h|--help)
+ usage
+ exit 0
+ ;;
+ --refresh)
+ refresh=yes
+ shift
+ ;;
+ --kernel-doc)
+ shift
+ kerneldoc="$1"
+ shift
+ ;;
+ *)
+ echo "Invalid argument: $1" >&2
+ usage >&2
+ exit 1
+ ;;
+ esac
+done
+
+tmp="$(mktemp -d)"
+trap 'rm -rf "$tmp"' EXIT
+
+test()
+{
+ local suffix="$1"
+ shift
+
+ if ! "$kerneldoc" "$@" "$mydir/test-case.h" \
+ > "$tmp/test-case.$suffix.actual" \
+ 2> "$tmp/test-case.$suffix.stderr" || \
+ [ -s "$tmp/test-case.$suffix.stderr" ];then
+ cat "$tmp/test-case.$suffix.stderr" >&2
+ echo "$suffix: kernel-doc $* failed" >&2
+ return 1
+ fi
+
+ if diff -u "$mydir/test-case.$suffix.expected" \
+ "$tmp/test-case.$suffix.actual";then
+ echo "$suffix: OK" >&2
+ else
+ echo "kernel-doc output mismatch for: $*" >&2
+ if [ "$refresh" = yes ];then
+ cp "$tmp/test-case.$suffix.actual" \
+ "$mydir/test-case.$suffix.expected"
+ fi
+ return 1
+ fi
+}
+
+if [ ! -x "$kerneldoc" ];then
+ echo "kernel-doc is not executable: $kerneldoc" >&2
+ exit 1
+fi
+
+# the -man output includes the build date
+export KBUILD_BUILD_TIMESTAMP=1991-08-25
+
+ok=yes
+
+# don't even try to test other formats if -none fails:
+test none -none || exit 1
+
+test rst2 -rst -sphinx-version 3.0.0 || ok=no
+test rst3 -rst -sphinx-version 3.0.0 || ok=no
+test man -man || ok=no
+
+if [ "$ok" != "yes" ];then
+ exit 1
+fi
--
2.28.0

2020-11-18 00:27:55

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On 11/17/20 2:36 PM, Eduardo Habkost wrote:
> Add a kernel-doc test script to tools/testing/kernel-doc.
>
> radix_tree_lookup_slot test case provided by Matthew Wilcox.
>
> Signed-off-by: Eduardo Habkost <[email protected]>

Very good idea.

I have had a kernel-doc test source file for (?) 10-12 years,
while I was the docs maintainer.

I didn't have the "expected" files for comparison.
I just used $web_browser.

> ---
> tools/testing/kernel-doc/test-case.h | 111 ++++++++++
> .../testing/kernel-doc/test-case.man.expected | 150 ++++++++++++++
> .../kernel-doc/test-case.none.expected | 0
> .../kernel-doc/test-case.rst2.expected | 195 ++++++++++++++++++
> .../kernel-doc/test-case.rst3.expected | 195 ++++++++++++++++++
> tools/testing/kernel-doc/test.sh | 90 ++++++++
> 6 files changed, 741 insertions(+)
> create mode 100644 tools/testing/kernel-doc/test-case.h
> create mode 100644 tools/testing/kernel-doc/test-case.man.expected
> create mode 100644 tools/testing/kernel-doc/test-case.none.expected
> create mode 100644 tools/testing/kernel-doc/test-case.rst2.expected
> create mode 100644 tools/testing/kernel-doc/test-case.rst3.expected
> create mode 100755 tools/testing/kernel-doc/test.sh

thanks.
--
~Randy

2020-11-18 08:25:23

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On 17/11/20 23:36, Eduardo Habkost wrote:
> +# the -man output includes the build date
> +export KBUILD_BUILD_TIMESTAMP=1991-08-25

Nice :)

> +ok=yes
> +
> +# don't even try to test other formats if -none fails:
> +test none -none || exit 1
> +
> +test rst2 -rst -sphinx-version 3.0.0 || ok=no

Do you want 3.0.0 here too?

> +test rst3 -rst -sphinx-version 3.0.0 || ok=no
> +test man -man || ok=no

Also since you are at it you might as well rename the function to
something other than "test", it's a bit confusing due to the "test"
shell builtin.

Paolo

2020-11-18 12:08:14

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On Wed, Nov 18, 2020 at 09:21:11AM +0100, Paolo Bonzini wrote:
> On 17/11/20 23:36, Eduardo Habkost wrote:
> > +# the -man output includes the build date
> > +export KBUILD_BUILD_TIMESTAMP=1991-08-25
>
> Nice :)
>
> > +ok=yes
> > +
> > +# don't even try to test other formats if -none fails:
> > +test none -none || exit 1
> > +
> > +test rst2 -rst -sphinx-version 3.0.0 || ok=no
>
> Do you want 3.0.0 here too?

Oops! No! I'll fix it.

>
> > +test rst3 -rst -sphinx-version 3.0.0 || ok=no
> > +test man -man || ok=no
>
> Also since you are at it you might as well rename the function to something
> other than "test", it's a bit confusing due to the "test" shell builtin.

Good point. I'll do.

--
Eduardo

2020-11-18 13:06:44

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On Tue, Nov 17, 2020 at 04:23:49PM -0800, Randy Dunlap wrote:
> On 11/17/20 2:36 PM, Eduardo Habkost wrote:
> > Add a kernel-doc test script to tools/testing/kernel-doc.
> >
> > radix_tree_lookup_slot test case provided by Matthew Wilcox.
> >
> > Signed-off-by: Eduardo Habkost <[email protected]>
>
> Very good idea.
>
> I have had a kernel-doc test source file for (?) 10-12 years,
> while I was the docs maintainer.

Is that test source file recoverable somewhere? It probably has
useful test cases not included here.

--
Eduardo

2020-11-18 16:29:28

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On Tue, 17 Nov 2020 17:36:12 -0500
Eduardo Habkost <[email protected]> wrote:

> Add a kernel-doc test script to tools/testing/kernel-doc.
>
> radix_tree_lookup_slot test case provided by Matthew Wilcox.
>
> Signed-off-by: Eduardo Habkost <[email protected]>
> ---
> tools/testing/kernel-doc/test-case.h | 111 ++++++++++
> .../testing/kernel-doc/test-case.man.expected | 150 ++++++++++++++
> .../kernel-doc/test-case.none.expected | 0
> .../kernel-doc/test-case.rst2.expected | 195 ++++++++++++++++++
> .../kernel-doc/test-case.rst3.expected | 195 ++++++++++++++++++
> tools/testing/kernel-doc/test.sh | 90 ++++++++
> 6 files changed, 741 insertions(+)
> create mode 100644 tools/testing/kernel-doc/test-case.h
> create mode 100644 tools/testing/kernel-doc/test-case.man.expected
> create mode 100644 tools/testing/kernel-doc/test-case.none.expected
> create mode 100644 tools/testing/kernel-doc/test-case.rst2.expected
> create mode 100644 tools/testing/kernel-doc/test-case.rst3.expected
> create mode 100755 tools/testing/kernel-doc/test.sh

Seems like a good thing to have overall.

I do worry a bit that the test will be sensitive to *any* change to
kernel-doc output, including formatting changes that might be deliberate.
But if that turns out to be a problem in the real world, we can deal with
it then.

Thanks,

jon

2020-11-18 16:37:14

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On 11/18/20 5:03 AM, Eduardo Habkost wrote:
> On Tue, Nov 17, 2020 at 04:23:49PM -0800, Randy Dunlap wrote:
>> On 11/17/20 2:36 PM, Eduardo Habkost wrote:
>>> Add a kernel-doc test script to tools/testing/kernel-doc.
>>>
>>> radix_tree_lookup_slot test case provided by Matthew Wilcox.
>>>
>>> Signed-off-by: Eduardo Habkost <[email protected]>
>>
>> Very good idea.
>>
>> I have had a kernel-doc test source file for (?) 10-12 years,
>> while I was the docs maintainer.
>
> Is that test source file recoverable somewhere? It probably has
> useful test cases not included here.

Sure. Of course, it may be out of date by quite a bit.
I last updated it in 2016:
-rw-r--r-- 1 rdunlap users 41737 Jun 17 2016 megatest.c

(attached)

--
~Randy


Attachments:
megatest.c (41.93 kB)

2020-11-18 16:59:51

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On Wed, Nov 18, 2020 at 08:32:35AM -0800, Randy Dunlap wrote:
> On 11/18/20 5:03 AM, Eduardo Habkost wrote:
> > On Tue, Nov 17, 2020 at 04:23:49PM -0800, Randy Dunlap wrote:
> >> On 11/17/20 2:36 PM, Eduardo Habkost wrote:
> >>> Add a kernel-doc test script to tools/testing/kernel-doc.
> >>>
> >>> radix_tree_lookup_slot test case provided by Matthew Wilcox.
> >>>
> >>> Signed-off-by: Eduardo Habkost <[email protected]>
> >>
> >> Very good idea.
> >>
> >> I have had a kernel-doc test source file for (?) 10-12 years,
> >> while I was the docs maintainer.
> >
> > Is that test source file recoverable somewhere? It probably has
> > useful test cases not included here.
>
> Sure. Of course, it may be out of date by quite a bit.
> I last updated it in 2016:
> -rw-r--r-- 1 rdunlap users 41737 Jun 17 2016 megatest.c
>
> (attached)
>

Thanks! Are the contents licensed under GPL 2.0?

--
Eduardo

2020-11-18 17:04:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC] Add kernel-doc test script

On 11/18/20 8:57 AM, Eduardo Habkost wrote:
> On Wed, Nov 18, 2020 at 08:32:35AM -0800, Randy Dunlap wrote:
>> On 11/18/20 5:03 AM, Eduardo Habkost wrote:
>>> On Tue, Nov 17, 2020 at 04:23:49PM -0800, Randy Dunlap wrote:
>>>> On 11/17/20 2:36 PM, Eduardo Habkost wrote:
>>>>> Add a kernel-doc test script to tools/testing/kernel-doc.
>>>>>
>>>>> radix_tree_lookup_slot test case provided by Matthew Wilcox.
>>>>>
>>>>> Signed-off-by: Eduardo Habkost <[email protected]>
>>>>
>>>> Very good idea.
>>>>
>>>> I have had a kernel-doc test source file for (?) 10-12 years,
>>>> while I was the docs maintainer.
>>>
>>> Is that test source file recoverable somewhere? It probably has
>>> useful test cases not included here.
>>
>> Sure. Of course, it may be out of date by quite a bit.
>> I last updated it in 2016:
>> -rw-r--r-- 1 rdunlap users 41737 Jun 17 2016 megatest.c
>>
>> (attached)
>>
>
> Thanks! Are the contents licensed under GPL 2.0?

Sure!

--
~Randy