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")
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)
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,
--
2.49.0Em 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
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> Em Tue, 1 Jul 2025 14:57:30 -0600
> Jonathan Corbet <corbet@lwn.net> escreveu:
>
>> 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 ;-)
Did you mean your preference would be to *not* break it?
There's a non-greppable piece at the break point anyway, so I wasn't
anticipating making life harder for anybody there.
Thanks,
jon
Em Thu, 03 Jul 2025 12:17:42 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
>
> > Em Tue, 1 Jul 2025 14:57:30 -0600
> > Jonathan Corbet <corbet@lwn.net> escreveu:
> >
> >> 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 ;-)
>
> Did you mean your preference would be to *not* break it?
What I meant is that I was in doubt myself of breaking long lines or
not... I opted to not break.
Yet, if you feel it looks better breaking it, go for it ;-)
> There's a non-greppable piece at the break point anyway, so I wasn't
> anticipating making life harder for anybody there.
>
> Thanks,
>
> jon
Thanks,
Mauro
© 2016 - 2025 Red Hat, Inc.