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

[Eduardo Habkost <ehabkost@redhat.com>]

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 <pbonzini@redhat.com> wrote:
> > 
> > > From: Eduardo Habkost <ehabkost@redhat.com>
> > > 
> > > Example of typedef that was not parsed by kernel-doc:
> > > 
> > >   typedef void (ObjectUnparent)(Object *obj);
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > 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 <ehabkost@redhat.com>
---
 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