[PATCH 0/7] docs: kdoc: tidy up create_parameter_list() somewhat

[PATCH 0/7] docs: kdoc: tidy up create_parameter_list() somewhat

[Jonathan Corbet]

A relatively brief series to straighten up the code in
create_parameter_list(), remove unneeded operations, and add a few
comments.  No changes to the generated output.

Jonathan Corbet (7):
  docs: kdoc: remove dead code
  docs: kdoc: tidy up space removal in create_parameter_list()
  docs: kdoc: clean up the create_parameter_list() "first arg" logic
  docs: kdoc: add a couple more comments in create_parameter_list()
  docs: kdoc: tighten up the array-of-pointers case
  docs: kdoc: tighten up the pointer-to-function case
  docs: kdoc: remove redundant comment stripping

 scripts/lib/kdoc/kdoc_parser.py | 96 +++++++++++++++------------------
 1 file changed, 42 insertions(+), 54 deletions(-)

-- 
2.50.1

[PATCH 1/7] docs: kdoc: remove dead code

[Jonathan Corbet]

create_parameter_list() tests an argument against the same regex twice, in
two different locations; remove the pointless extra tests and the
never-executed error cases that go with them.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 9e65948f8254..96e3fe4ec431 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -564,28 +564,18 @@ class KernelDoc:
                 args.insert(0, first_arg.pop())
                 dtype = ' '.join(first_arg)
 
+                bitfield_re = KernRe(r'(.*?):(\w+)')
                 for param in args:
-                    if KernRe(r'^(\*+)\s*(.*)').match(param):
-                        r = KernRe(r'^(\*+)\s*(.*)')
-                        if not r.match(param):
-                            self.emit_msg(ln, f"Invalid param: {param}")
-                            continue
-
-                        param = r.group(1)
-
+                    r = KernRe(r'^(\*+)\s*(.*)')
+                    if r.match(param):
                         self.push_parameter(ln, decl_type, r.group(2),
                                             f"{dtype} {r.group(1)}",
                                             arg, declaration_name)
 
-                    elif KernRe(r'(.*?):(\w+)').search(param):
-                        r = KernRe(r'(.*?):(\w+)')
-                        if not r.match(param):
-                            self.emit_msg(ln, f"Invalid param: {param}")
-                            continue
-
+                    elif bitfield_re.search(param):
                         if dtype != "":  # Skip unnamed bit-fields
-                            self.push_parameter(ln, decl_type, r.group(1),
-                                                f"{dtype}:{r.group(2)}",
+                            self.push_parameter(ln, decl_type, bitfield_re.group(1),
+                                                f"{dtype}:{bitfield_re.group(2)}",
                                                 arg, declaration_name)
                     else:
                         self.push_parameter(ln, decl_type, param, dtype,
-- 
2.50.1

Re: [PATCH 1/7] docs: kdoc: remove dead code

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:42 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> create_parameter_list() tests an argument against the same regex twice, in
> two different locations; remove the pointless extra tests and the
> never-executed error cases that go with them.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

LGTM.

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---
>  scripts/lib/kdoc/kdoc_parser.py | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 9e65948f8254..96e3fe4ec431 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -564,28 +564,18 @@ class KernelDoc:
>                  args.insert(0, first_arg.pop())
>                  dtype = ' '.join(first_arg)
>  
> +                bitfield_re = KernRe(r'(.*?):(\w+)')
>                  for param in args:
> -                    if KernRe(r'^(\*+)\s*(.*)').match(param):
> -                        r = KernRe(r'^(\*+)\s*(.*)')
> -                        if not r.match(param):
> -                            self.emit_msg(ln, f"Invalid param: {param}")
> -                            continue
> -
> -                        param = r.group(1)
> -
> +                    r = KernRe(r'^(\*+)\s*(.*)')
> +                    if r.match(param):
>                          self.push_parameter(ln, decl_type, r.group(2),
>                                              f"{dtype} {r.group(1)}",
>                                              arg, declaration_name)
>  
> -                    elif KernRe(r'(.*?):(\w+)').search(param):
> -                        r = KernRe(r'(.*?):(\w+)')
> -                        if not r.match(param):
> -                            self.emit_msg(ln, f"Invalid param: {param}")
> -                            continue
> -
> +                    elif bitfield_re.search(param):
>                          if dtype != "":  # Skip unnamed bit-fields
> -                            self.push_parameter(ln, decl_type, r.group(1),
> -                                                f"{dtype}:{r.group(2)}",
> +                            self.push_parameter(ln, decl_type, bitfield_re.group(1),
> +                                                f"{dtype}:{bitfield_re.group(2)}",
>                                                  arg, declaration_name)
>                      else:
>                          self.push_parameter(ln, decl_type, param, dtype,



Thanks,
Mauro

[PATCH 2/7] docs: kdoc: tidy up space removal in create_parameter_list()

[Jonathan Corbet]

Remove a redundant test and add a comment describing what the space removal
is doing.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 96e3fe4ec431..53051ce831ba 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -545,12 +545,14 @@ class KernelDoc:
                                     arg, declaration_name)
 
             elif arg:
+                #
+                # Clean up extraneous spaces and split the string at commas; the first
+                # element of the resulting list will also include the type information.
+                #
                 arg = KernRe(r'\s*:\s*').sub(":", arg)
                 arg = KernRe(r'\s*\[').sub('[', arg)
-
                 args = KernRe(r'\s*,\s*').split(arg)
-                if args[0] and '*' in args[0]:
-                    args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
+                args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
 
                 first_arg = []
                 r = KernRe(r'^(.*\s+)(.*?\[.*\].*)$')
-- 
2.50.1

Re: [PATCH 2/7] docs: kdoc: tidy up space removal in create_parameter_list()

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:43 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Remove a redundant test and add a comment describing what the space removal
> is doing.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

LGTM.
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---
>  scripts/lib/kdoc/kdoc_parser.py | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 96e3fe4ec431..53051ce831ba 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -545,12 +545,14 @@ class KernelDoc:
>                                      arg, declaration_name)
>  
>              elif arg:
> +                #
> +                # Clean up extraneous spaces and split the string at commas; the first
> +                # element of the resulting list will also include the type information.
> +                #
>                  arg = KernRe(r'\s*:\s*').sub(":", arg)
>                  arg = KernRe(r'\s*\[').sub('[', arg)
> -
>                  args = KernRe(r'\s*,\s*').split(arg)
> -                if args[0] and '*' in args[0]:
> -                    args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
> +                args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
>  
>                  first_arg = []
>                  r = KernRe(r'^(.*\s+)(.*?\[.*\].*)$')



Thanks,
Mauro

[PATCH 3/7] docs: kdoc: clean up the create_parameter_list() "first arg" logic

[Jonathan Corbet]

The logic for finding the name of the first in a series of variable names
is somewhat convoluted and, in the use of .extend(), actively buggy.
Document what is happening and simplify the logic.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 53051ce831ba..47f7ea01ed10 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -553,18 +553,18 @@ class KernelDoc:
                 arg = KernRe(r'\s*\[').sub('[', arg)
                 args = KernRe(r'\s*,\s*').split(arg)
                 args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
-
-                first_arg = []
-                r = KernRe(r'^(.*\s+)(.*?\[.*\].*)$')
-                if args[0] and r.match(args[0]):
-                    args.pop(0)
-                    first_arg.extend(r.group(1))
-                    first_arg.append(r.group(2))
+                #
+                # args[0] has a string of "type a".  If "a" includes an [array]
+                # declaration, we want to not be fooled by any white space inside
+                # the brackets, so detect and handle that case specially.
+                #
+                r = KernRe(r'^([^[\]]*\s+)' r'((?:.*?\[.*\].*)|(?:.*?))$')
+                if r.match(args[0]):
+                    args[0] = r.group(2)
+                    dtype = r.group(1)
                 else:
-                    first_arg = KernRe(r'\s+').split(args.pop(0))
-
-                args.insert(0, first_arg.pop())
-                dtype = ' '.join(first_arg)
+                    # No space in args[0]; this seems wrong but preserves previous behavior
+                    dtype = ''
 
                 bitfield_re = KernRe(r'(.*?):(\w+)')
                 for param in args:
-- 
2.50.1

Re: [PATCH 3/7] docs: kdoc: clean up the create_parameter_list() "first arg" logic

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:44 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> The logic for finding the name of the first in a series of variable names
> is somewhat convoluted and, in the use of .extend(), actively buggy.
> Document what is happening and simplify the logic.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 53051ce831ba..47f7ea01ed10 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -553,18 +553,18 @@ class KernelDoc:
>                  arg = KernRe(r'\s*\[').sub('[', arg)
>                  args = KernRe(r'\s*,\s*').split(arg)
>                  args[0] = re.sub(r'(\*+)\s*', r' \1', args[0])
> -
> -                first_arg = []
> -                r = KernRe(r'^(.*\s+)(.*?\[.*\].*)$')
> -                if args[0] and r.match(args[0]):
> -                    args.pop(0)
> -                    first_arg.extend(r.group(1))
> -                    first_arg.append(r.group(2))

I double-checked the Perl code. The Python version seems to be an exact
translation of what was there:

            $arg =~ s/\s*\[/\[/g;

            my @args = split('\s*,\s*', $arg);
            if ($args[0] =~ m/\*/) {
                $args[0] =~ s/(\*+)\s*/ $1/;
            }

	    my @first_arg;
            if ($args[0] =~ /^(.*\s+)(.*?\[.*\].*)$/) {
                shift @args;
                push(@first_arg, split('\s+', $1));
                push(@first_arg, $2);
            } else {
                @first_arg = split('\s+', shift @args);
            }

Yeah, I agree that this logic is confusing. 

> +                #
> +                # args[0] has a string of "type a".  If "a" includes an [array]
> +                # declaration, we want to not be fooled by any white space inside
> +                # the brackets, so detect and handle that case specially.
> +                #
> +                r = KernRe(r'^([^[\]]*\s+)' r'((?:.*?\[.*\].*)|(?:.*?))$')

Same comment as patch 6/7... concats in the middle of the like IMO makes it
harder to read. Better to place them on separate lines:

	r = KernRe(r'^([^[\]]*\s+)'
		   r'((?:.*?\[.*\].*)|(?:.*?))$')

> +                if r.match(args[0]):
> +                    args[0] = r.group(2)
> +                    dtype = r.group(1)
>                  else:
> -                    first_arg = KernRe(r'\s+').split(args.pop(0))
> -
> -                args.insert(0, first_arg.pop())
> -                dtype = ' '.join(first_arg)
> +                    # No space in args[0]; this seems wrong but preserves previous behavior
> +                    dtype = ''
>  
>                  bitfield_re = KernRe(r'(.*?):(\w+)')
>                  for param in args:

I didn't test your new code. On a first glance, it doesn't seem identical
to the previous one, but if you tested it and the results are the same,
the new version seems nicer once you split the concat on two lines. So,
feel free to add:

Acked-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


-

Btw, IMHO, it would make sense to have unittests to check things
like that to ensure that new patches won't cause regressions for
some particular usecases.

Thanks,
Mauro

Re: [PATCH 3/7] docs: kdoc: clean up the create_parameter_list() "first arg" logic

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

>> +                #
>> +                # args[0] has a string of "type a".  If "a" includes an [array]
>> +                # declaration, we want to not be fooled by any white space inside
>> +                # the brackets, so detect and handle that case specially.
>> +                #
>> +                r = KernRe(r'^([^[\]]*\s+)' r'((?:.*?\[.*\].*)|(?:.*?))$')
>
> Same comment as patch 6/7... concats in the middle of the like IMO makes it
> harder to read. Better to place them on separate lines:
>
> 	r = KernRe(r'^([^[\]]*\s+)'
> 		   r'((?:.*?\[.*\].*)|(?:.*?))$')

So I went to do this, and realized that the second chunk of the regex is
really just a complex way of saying "(.*)$" - so I'll make it just that,
at which point splitting up the string seems a bit excessive.

Thanks,

jon

[PATCH 4/7] docs: kdoc: add a couple more comments in create_parameter_list()

[Jonathan Corbet]

Make what the final code is doing a bit more clear to slow readers like me.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 47f7ea01ed10..36c4035343dc 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -568,12 +568,18 @@ class KernelDoc:
 
                 bitfield_re = KernRe(r'(.*?):(\w+)')
                 for param in args:
+                    #
+                    # For pointers, shift the star(s) from the variable name to the
+                    # type declaration.
+                    #
                     r = KernRe(r'^(\*+)\s*(.*)')
                     if r.match(param):
                         self.push_parameter(ln, decl_type, r.group(2),
                                             f"{dtype} {r.group(1)}",
                                             arg, declaration_name)
-
+                    #
+                    # Perform a similar shift for bitfields.
+                    #
                     elif bitfield_re.search(param):
                         if dtype != "":  # Skip unnamed bit-fields
                             self.push_parameter(ln, decl_type, bitfield_re.group(1),
-- 
2.50.1

Re: [PATCH 4/7] docs: kdoc: add a couple more comments in create_parameter_list()

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:45 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Make what the final code is doing a bit more clear to slow readers like me.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 47f7ea01ed10..36c4035343dc 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -568,12 +568,18 @@ class KernelDoc:
>  
>                  bitfield_re = KernRe(r'(.*?):(\w+)')
>                  for param in args:
> +                    #
> +                    # For pointers, shift the star(s) from the variable name to the
> +                    # type declaration.
> +                    #
>                      r = KernRe(r'^(\*+)\s*(.*)')
>                      if r.match(param):
>                          self.push_parameter(ln, decl_type, r.group(2),
>                                              f"{dtype} {r.group(1)}",
>                                              arg, declaration_name)
> -
> +                    #
> +                    # Perform a similar shift for bitfields.
> +                    #
>                      elif bitfield_re.search(param):
>                          if dtype != "":  # Skip unnamed bit-fields
>                              self.push_parameter(ln, decl_type, bitfield_re.group(1),



Thanks,
Mauro

[PATCH 5/7] docs: kdoc: tighten up the array-of-pointers case

[Jonathan Corbet]

Simplify one gnarly regex and remove another altogether; add a comment
describing what is going on.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index 36c4035343dc..d7fb79a64487 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -527,23 +527,19 @@ class KernelDoc:
                 dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
                 self.push_parameter(ln, decl_type, param, dtype,
                                     arg, declaration_name)
-
+            #
+            # The array-of-pointers case.  Dig the parameter name out from the middle
+            # of the declaration.
+            #
             elif KernRe(r'\(.+\)\s*\[').search(arg):
-                # Array-of-pointers
-
-                arg = arg.replace('#', ',')
-                r = KernRe(r'[^\(]+\(\s*\*\s*([\w\[\].]*?)\s*(\s*\[\s*[\w]+\s*\]\s*)*\)')
+                r = KernRe(r'[^\(]+\(\s*\*\s*' r'([\w.]*?)' r'\s*(\[\s*\w+\s*\]\s*)*\)')
                 if r.match(arg):
                     param = r.group(1)
                 else:
                     self.emit_msg(ln, f"Invalid param: {arg}")
                     param = arg
-
-                dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
-
-                self.push_parameter(ln, decl_type, param, dtype,
-                                    arg, declaration_name)
-
+                dtype = arg.replace(param, '')
+                self.push_parameter(ln, decl_type, param, dtype, arg, declaration_name)
             elif arg:
                 #
                 # Clean up extraneous spaces and split the string at commas; the first
-- 
2.50.1

Re: [PATCH 5/7] docs: kdoc: tighten up the array-of-pointers case

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:46 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Simplify one gnarly regex and remove another altogether; add a comment
> describing what is going on.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 36c4035343dc..d7fb79a64487 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -527,23 +527,19 @@ class KernelDoc:
>                  dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
>                  self.push_parameter(ln, decl_type, param, dtype,
>                                      arg, declaration_name)
> -
> +            #
> +            # The array-of-pointers case.  Dig the parameter name out from the middle
> +            # of the declaration.
> +            #
>              elif KernRe(r'\(.+\)\s*\[').search(arg):
> -                # Array-of-pointers
> -
> -                arg = arg.replace('#', ',')

Hmm... if I'm not mistaken, there is(was?) a previous code that replaced
commas by "#". Such statement is needed to catch some corner case.

This like here is(was?) needed to restore the original arg string.

> -                r = KernRe(r'[^\(]+\(\s*\*\s*([\w\[\].]*?)\s*(\s*\[\s*[\w]+\s*\]\s*)*\)')
> +                r = KernRe(r'[^\(]+\(\s*\*\s*' r'([\w.]*?)' r'\s*(\[\s*\w+\s*\]\s*)*\)')

As mentioned on patch 6/7, IMHO doing concats like that at the same line
IMO makes it harder to understand. This works best:

                r = KernRe(r'[^\(]+\(\s*\*\s*'
			   r'([\w.]*?)'
			   r'\s*(\[\s*\w+\s*\]\s*)*\)')


>                  if r.match(arg):
>                      param = r.group(1)
>                  else:
>                      self.emit_msg(ln, f"Invalid param: {arg}")
>                      param = arg
> -
> -                dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
> -
> -                self.push_parameter(ln, decl_type, param, dtype,
> -                                    arg, declaration_name)
> -
> +                dtype = arg.replace(param, '')
> +                self.push_parameter(ln, decl_type, param, dtype, arg, declaration_name)
>              elif arg:
>                  #
>                  # Clean up extraneous spaces and split the string at commas; the first



Thanks,
Mauro

Re: [PATCH 5/7] docs: kdoc: tighten up the array-of-pointers case

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> On Tue, 12 Aug 2025 13:57:46 -0600
> Jonathan Corbet <corbet@lwn.net> wrote:
>>              elif KernRe(r'\(.+\)\s*\[').search(arg):
>> -                # Array-of-pointers
>> -
>> -                arg = arg.replace('#', ',')
>
> Hmm... if I'm not mistaken, there is(was?) a previous code that replaced
> commas by "#". Such statement is needed to catch some corner case.
>
> This like here is(was?) needed to restore the original arg string.

That (hackish :) replacement is still there ... but there will be no
commas in anything matched by the regex here, so the restoration is not
needed.  I can add that to the changelog for curious readers in the
future. 

>> -                r = KernRe(r'[^\(]+\(\s*\*\s*([\w\[\].]*?)\s*(\s*\[\s*[\w]+\s*\]\s*)*\)')
>> +                r = KernRe(r'[^\(]+\(\s*\*\s*' r'([\w.]*?)' r'\s*(\[\s*\w+\s*\]\s*)*\)')
>
> As mentioned on patch 6/7, IMHO doing concats like that at the same line
> IMO makes it harder to understand. This works best:
>
>                 r = KernRe(r'[^\(]+\(\s*\*\s*'
> 			   r'([\w.]*?)'
> 			   r'\s*(\[\s*\w+\s*\]\s*)*\)')

I'll do that.

Thanks,

jon

[PATCH 6/7] docs: kdoc: tighten up the pointer-to-function case

[Jonathan Corbet]

Tighten up the code and remove an unneeded regex operation.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index d7fb79a64487..ceb38b59fb4c 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -511,22 +511,19 @@ class KernelDoc:
                 # Treat preprocessor directive as a typeless variable
                 self.push_parameter(ln, decl_type, arg, "",
                                     "", declaration_name)
-
+            #
+            # The pointer-to-function case.
+            #
             elif KernRe(r'\(.+\)\s*\(').search(arg):
-                # Pointer-to-function
-
                 arg = arg.replace('#', ',')
-
-                r = KernRe(r'[^\(]+\(\*?\s*([\w\[\].]*)\s*\)')
+                r = KernRe(r'[^\(]+\(\*?\s*' r'([\w\[\].]*)' r'\s*\)')
                 if r.match(arg):
                     param = r.group(1)
                 else:
                     self.emit_msg(ln, f"Invalid param: {arg}")
                     param = arg
-
-                dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
-                self.push_parameter(ln, decl_type, param, dtype,
-                                    arg, declaration_name)
+                dtype = arg.replace(param, '')
+                self.push_parameter(ln, decl_type, param, dtype, arg, declaration_name)
             #
             # The array-of-pointers case.  Dig the parameter name out from the middle
             # of the declaration.
-- 
2.50.1

Re: [PATCH 6/7] docs: kdoc: tighten up the pointer-to-function case

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:47 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Tighten up the code and remove an unneeded regex operation.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index d7fb79a64487..ceb38b59fb4c 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -511,22 +511,19 @@ class KernelDoc:
>                  # Treat preprocessor directive as a typeless variable
>                  self.push_parameter(ln, decl_type, arg, "",
>                                      "", declaration_name)
> -
> +            #
> +            # The pointer-to-function case.
> +            #
>              elif KernRe(r'\(.+\)\s*\(').search(arg):
> -                # Pointer-to-function
> -
>                  arg = arg.replace('#', ',')
> -
> -                r = KernRe(r'[^\(]+\(\*?\s*([\w\[\].]*)\s*\)')
> +                r = KernRe(r'[^\(]+\(\*?\s*' r'([\w\[\].]*)' r'\s*\)')

Heh, it took me a couple of seconds to understand this concat, as I haven't
seem concat pattern like that before (maybe except for some old C book
I read a millennium ago that I barely remember).  So, IMO, it became harder
to understand this way. I would either remove the extra two ' r' from the
string or write it as:

               r = KernRe(r'[^\(]+\(\*?\s*' 
			  r'([\w\[\].]*)'
			  r'\s*\)')

Eventually adding a comment for the capture group. 

With that, feel free to add:

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

>                  if r.match(arg):
>                      param = r.group(1)
>                  else:
>                      self.emit_msg(ln, f"Invalid param: {arg}")
>                      param = arg
> -
> -                dtype = KernRe(r'([^\(]+\(\*?)\s*' + re.escape(param)).sub(r'\1', arg)
> -                self.push_parameter(ln, decl_type, param, dtype,
> -                                    arg, declaration_name)
> +                dtype = arg.replace(param, '')

Nice cleanup. 

> +                self.push_parameter(ln, decl_type, param, dtype, arg, declaration_name)
>              #
>              # The array-of-pointers case.  Dig the parameter name out from the middle
>              # of the declaration.



Thanks,
Mauro

Re: [PATCH 6/7] docs: kdoc: tighten up the pointer-to-function case

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> On Tue, 12 Aug 2025 13:57:47 -0600
> Jonathan Corbet <corbet@lwn.net> wrote:
>
>> -                r = KernRe(r'[^\(]+\(\*?\s*([\w\[\].]*)\s*\)')
>> +                r = KernRe(r'[^\(]+\(\*?\s*' r'([\w\[\].]*)' r'\s*\)')
>
> Heh, it took me a couple of seconds to understand this concat, as I haven't
> seem concat pattern like that before (maybe except for some old C book
> I read a millennium ago that I barely remember).  So, IMO, it became harder
> to understand this way. I would either remove the extra two ' r' from the
> string or write it as:
>
>                r = KernRe(r'[^\(]+\(\*?\s*' 
> 			  r'([\w\[\].]*)'
> 			  r'\s*\)')

By remove the " r" you mean glom the pieces back together into a single
string?  These long regexes are hard to make sense of, I do think it
helps to break them into logical chunks.

Certainly I can split it across multiple lines if that helps.

Thanks,

jon

Re: [PATCH 6/7] docs: kdoc: tighten up the pointer-to-function case

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 17:22:35 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> 
> > On Tue, 12 Aug 2025 13:57:47 -0600
> > Jonathan Corbet <corbet@lwn.net> wrote:
> >  
> >> -                r = KernRe(r'[^\(]+\(\*?\s*([\w\[\].]*)\s*\)')
> >> +                r = KernRe(r'[^\(]+\(\*?\s*' r'([\w\[\].]*)' r'\s*\)')  
> >
> > Heh, it took me a couple of seconds to understand this concat, as I haven't
> > seem concat pattern like that before (maybe except for some old C book
> > I read a millennium ago that I barely remember).  So, IMO, it became harder
> > to understand this way. I would either remove the extra two ' r' from the
> > string or write it as:
> >
> >                r = KernRe(r'[^\(]+\(\*?\s*' 
> > 			  r'([\w\[\].]*)'
> > 			  r'\s*\)')  
> 
> By remove the " r" you mean glom the pieces back together into a single
> string?  These long regexes are hard to make sense of, I do think it
> helps to break them into logical chunks.

At least for me, it doesn't make much difference: I still need to
read the entire thing to understand what it does. The extra ' r' in
the middle of it, adds extra noise.

> Certainly I can split it across multiple lines if that helps.

It is OK to split on multiple lines. 

Btw, I tried to use re.X:
	https://docs.python.org/3/library/re.html#re.X

on one of the most complex regexes, which would be the standard way to 
document complex regular expression (Perl had some using x flag), but
it didn't work.

Still, I think that we should aim to have something similar to it,
e.g.:

                r = KernRe(r'[^\(]+\(\*?\s*'  # some explanation
 			   r'([\w\[\].]*)'    # some explanation
 			   r'\s*\)')          # some explanation  

for the most complex ones.

---

Side note: reading just this out of the context - as one would do
when reviewing such patch makes is hard for one to tell exactly
what c prototype patterns the above is trying to catch.

Adding some explanation for the pattern may help, but, IMO, we should 
go one step further and create something like the enclosed unittest
logic (placed at tools/docs or tools/docs_unittest).

Before merging kernel-doc patches (and/or inside a CI job), run the
unit test to check if this won't break anything.

I suspect that creating something like this won't be that hard,
as we can modify kernel-doc command line to generate a list of
all cases it currently handles, storing the original kernel-doc
macro. Then, ask some LLM to clean it up, dropping cases that
have the same pattern.


Thanks,
Mauro

---

The code below is just a prototype. On its current version, the
tests run, but them all fail. My main goal here is just to give you
a more precise example of the kind of tool I'm thinking. If you
think it is worth, I can work on it later on.

Btw, I have somewhere on my internal scripts a replacement method for 
unittest.main() that provides a better summary when verbose is disabled
and have a command line argument to enable verbose. If we're willing
to do that, I'll add it to the final patchset.


#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0
# Copyright(c) 2025: Mauro Carvalho Chehab <mchehab@kernel.org>.
# Generated with the help of DeepSeek / Deepthink (R1)

import argparse
import logging
import os
import tempfile
import unittest
import sys

from textwrap import dedent

# Import KernelDoc parser class
SRC_DIR = os.path.dirname(os.path.realpath(__file__))
LIB_DIR = "../../scripts/lib/kdoc"

sys.path.insert(0, os.path.join(SRC_DIR, LIB_DIR))

from kdoc_parser import KernelDoc


TEST_CASES = [
    # 1. Basic function parsing
    {
        "input": dedent("""
            /**
             * simple_function - Basic function
             * @param: Description
             */
            void simple_function(int param)
        """),
        "expected": {
            "identifier": "simple_function",
            "return_type": "void",
            "func_macro": False
        }
    },

    # 2. Static prefix stripping (pattern: r"^static +")
    {
        "input": dedent("""
            /**
             * static_fn - Static function
             * @arg: String argument
             */
            static int static_fn(char *arg)
        """),
        "expected": {
            "identifier": "static_fn",
            "return_type": "int",
            "func_macro": False
        }
    },

    # 3. Function macro (#define)
    {
        "input": dedent("""
            /**
             * macro_func() - Macro function
             * @a: First param
             */
            #define macro_func(a) do_something(a)
        """),
        "expected": {
            "identifier": "macro_func",
            "return_type": "",
            "func_macro": True
        }
    },

    # 4. Complex return type
    {
        "input": dedent("""
            /**
             * complex_return - Long return type
             * Returns: Pointer to foo structure
             */
            struct foo *complex_return(void)
        """),
        "expected": {
            "identifier": "complex_return",
            "return_type": "struct foo *",
            "func_macro": False
        }
    },

    # 5. __attribute_const__ stripping (pattern: r"__attribute_const__ +")
    {
        "input": dedent("""
            /**
             * const_attr - Function with const attribute
             */
            __attribute_const__ int const_attr(void)
        """),
        "expected": {
            "identifier": "const_attr",
            "return_type": "int",
            "func_macro": False
        }
    },

    # 6. __printf attribute stripping (pattern: r"__printf\s*\(\s*\d*\s*,\s*\d*\s*\) +")
    {
        "input": dedent("""
            /**
             * printf_fn - Printf-style function
             * @fmt: Format string
             * @...: Variable arguments
             */
            __printf(2, 3) int printf_fn(const char *fmt, ...)
        """),
        "expected": {
            "identifier": "printf_fn",
            "return_type": "int",
            "func_macro": False
        }
    },

    # 7. Multiple prefixes (static + inline)
    {
        "input": dedent("""
            /**
             * multi_prefix - Multiple prefixes
             */
            static __always_inline int multi_prefix(int a)
        """),
        "expected": {
            "identifier": "multi_prefix",
            "return_type": "int",
            "func_macro": False
        }
    },

    # 8. DECL_BUCKET_PARAMS handling
    {
        "input": dedent("""
            /**
             * bucket_fn - Bucket params function
             */
            DECL_BUCKET_PARAMS(a, b) void bucket_fn(void)
        """),
        "expected": {
            "identifier": "bucket_fn",
            "return_type": "void",
            "func_macro": False
        }
    },

    # 9. Function pointer parameter
    {
        "input": dedent("""
            /**
             * fp_param - Function with callback
             * @cb: Callback function
             */
            void fp_param(int (*cb)(int))
        """),
        "expected": {
            "identifier": "fp_param",
            "return_type": "void",
            "func_macro": False
        }
    },

    # 10. Malformed prototype (error case)
    {
        "input": dedent("""
            /**
             * bad_func - Bad prototype
             */
            int bad_func(missing_paren int a
        """),
        "expected": {
            "expected_error": "cannot understand function prototype: 'int bad_func(missing_paren int a'"
        }
    },

    # 11. Name mismatch (error case)
    {
        "input": dedent("""
            /**
             * expected_name - Wrong prototype
             */
            void actual_name(void)
        """),
        "expected": {
            "expected_error": "expecting prototype for expected_name(). Prototype was for actual_name() instead"
        }
    },

    # 12. No kernel-doc comment (edge case)
    {
        "input": "void undocumented_func(void)",
        "expected": {
            "expected_error": "missing kernel-doc comment"
        }
    },

    # Add a lot more here
]

class TestKernelDocParsing(unittest.TestCase):
    """Dynamically generated test cases for kernel-doc parsing"""

    def setUp(self):
        self.config = argparse.Namespace()
        self.config.verbose = False
        self.config.werror = False
        self.config.wreturn = False
        self.config.wshort_desc = False
        self.config.wcontents_before_sections = False
        self.config.log = logging.getLogger("kernel-doc-test")
        self.config.src_tree = None

def make_test_function(input_str, expected):
    """Factory function to create kernel-doc test cases with file handling"""
    def test(self):
        # Create temp file with test content
        with tempfile.NamedTemporaryFile(mode='w+', delete=False) as tmp:
            tmp.write(input_str)
            tmp_path = tmp.name

        try:
            # Initialize parser with the temp file
            doc = KernelDoc(self.config, tmp_path)

            # FIXME: this part still need adjustments
            export_table, entries = doc.parse_kdoc()

            # Verify parsing results
            if "expected_error" in expected:
                self.fail(f"Expected error but parsing succeeded: {input_str}")

            # Find our test entry (assuming single entry for simplicity)
            test_entry = next((e for e in entries if e.identifier == expected["identifier"]), None)
            self.assertIsNotNone(test_entry, f"Expected entry not found for: {input_str}")

            # Validate results
            self.assertEqual(test_entry.return_type, expected["return_type"],
                            f"Return type mismatch for: {input_str}")
            self.assertEqual(getattr(test_entry, 'func_macro', False), expected["func_macro"],
                            f"Function macro flag mismatch for: {input_str}")

        except Exception as e:
            if "expected_error" not in expected:
                raise
            self.assertIn(expected["expected_error"], str(e),
                         f"Expected error not found in: {str(e)}")
        finally:
            # Clean up temp file
            if os.path.exists(tmp_path):
                os.unlink(tmp_path)

    return test

# Dynamically add test methods to the TestCase class
# This has to be outside __main__
for i, test_case in enumerate(TEST_CASES):
    test_name = f"test_case_{i+1:02d}"
    if "identifier" in test_case["expected"]:
        test_name += f"_{test_case['expected']['identifier']}"
    elif "expected_error" in test_case["expected"]:
        test_name += "_error_case"

    test_func = make_test_function(test_case["input"], test_case["expected"])
    setattr(TestKernelDocParsing, test_name, test_func)

if __name__ == "__main__":
    unittest.main()

[PATCH 7/7] docs: kdoc: remove redundant comment stripping

[Jonathan Corbet]

By the time stuff gets to create_parameter_list(), comments have long since
been stripped out, so we do not need to do it again here.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 scripts/lib/kdoc/kdoc_parser.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
index ceb38b59fb4c..900c028687cc 100644
--- a/scripts/lib/kdoc/kdoc_parser.py
+++ b/scripts/lib/kdoc/kdoc_parser.py
@@ -493,9 +493,6 @@ class KernelDoc:
             args = arg_expr.sub(r"\1#", args)
 
         for arg in args.split(splitter):
-            # Strip comments
-            arg = KernRe(r'/\*.*\*/').sub('', arg)
-
             # Ignore argument attributes
             arg = KernRe(r'\sPOS0?\s').sub(' ', arg)
 
-- 
2.50.1

Re: [PATCH 7/7] docs: kdoc: remove redundant comment stripping

[Mauro Carvalho Chehab]

On Tue, 12 Aug 2025 13:57:48 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> By the time stuff gets to create_parameter_list(), comments have long since
> been stripped out, so we do not need to do it again here.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index ceb38b59fb4c..900c028687cc 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -493,9 +493,6 @@ class KernelDoc:
>              args = arg_expr.sub(r"\1#", args)
>  
>          for arg in args.split(splitter):
> -            # Strip comments
> -            arg = KernRe(r'/\*.*\*/').sub('', arg)
> -
>              # Ignore argument attributes
>              arg = KernRe(r'\sPOS0?\s').sub(' ', arg)
>  

Didn't check, but yeah, there are some places already doing
it, so:

Acked-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


Thanks,
Mauro