Re: [PATCH 7/7] docs: kdoc: pretty up dump_enum()

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Tue,  1 Jul 2025 14:57:30 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> Add some comments to dump_enum to help the next person who has to figure
> out what it is actually doing.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  scripts/lib/kdoc/kdoc_parser.py | 39 +++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index d5ef3ce87438..50e25cf62863 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -860,39 +860,48 @@ class KernelDoc:
>          # Strip #define macros inside enums
>          proto = KernRe(r'#\s*((define|ifdef|if)\s+|endif)[^;]*;', flags=re.S).sub('', proto)
>  
> -        members = None
> -        declaration_name = None
> -
> +        #
> +        # Parse out the name and members of the enum.  Typedef form first.
> +        #
>          r = KernRe(r'typedef\s+enum\s*\{(.*)\}\s*(\w*)\s*;')
>          if r.search(proto):
>              declaration_name = r.group(2)
>              members = r.group(1).rstrip()
> +        #
> +        # Failing that, look for a straight enum
> +        #
>          else:
>              r = KernRe(r'enum\s+(\w*)\s*\{(.*)\}')
>              if r.match(proto):
>                  declaration_name = r.group(1)
>                  members = r.group(2).rstrip()
> -
> -        if not members:
> -            self.emit_msg(ln, f"{proto}: error: Cannot parse enum!")
> -            return
> -
> +        #
> +        # OK, this isn't going to work.
> +        #
> +	    else:
> +                self.emit_msg(ln, f"{proto}: error: Cannot parse enum!")
> +                return
> +        #
> +        # Make sure we found what we were expecting.
> +        #
>          if self.entry.identifier != declaration_name:
>              if self.entry.identifier == "":
>                  self.emit_msg(ln,
>                                f"{proto}: wrong kernel-doc identifier on prototype")
>              else:
>                  self.emit_msg(ln,
> -                              f"expecting prototype for enum {self.entry.identifier}. Prototype was for enum {declaration_name} instead")
> +                              f"expecting prototype for enum {self.entry.identifier}. "
> +                              f"Prototype was for enum {declaration_name} instead")

Even being a big one, my personal preference would be to break the long
string here, as keeping together is easier for grep, but yeah, I also
considered breaking it ;-)

>              return
>  
>          if not declaration_name:
>              declaration_name = "(anonymous)"
> -
> +        #
> +        # Parse out the name of each enum member, and verify that we
> +        # have a description for it.
> +        #
>          member_set = set()
> -
> -        members = KernRe(r'\([^;]*?[\)]').sub('', members)
> -
> +        members = KernRe(r'\([^;)]*\)').sub('', members)

I wonder why we had this "?" there... Not sure if it has any effect on
this particular regex. I *guess* not.

if the output is the same, I'm all for such change :-)

>          for arg in members.split(','):
>              if not arg:
>                  continue
> @@ -903,7 +912,9 @@ class KernelDoc:
>                  self.emit_msg(ln,
>                                f"Enum value '{arg}' not described in enum '{declaration_name}'")
>              member_set.add(arg)
> -
> +        #
> +        # Ensure that every described member actually exists in the enum.
> +        #
>          for k in self.entry.parameterdescs:
>              if k not in member_set:
>                  self.emit_msg(ln,

Either way, with or without changes on the above nitpicks:

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

Regards,
Mauro