Em Mon, 24 Feb 2025 16:38:58 -0700 Jonathan Corbet <[email protected]> escreveu:
> Mauro Carvalho Chehab <[email protected]> writes: > > > Maintaining kernel-doc has been a challenge, as there aren't many > > perl developers among maintainers. Also, the logic there is too > > complex. Having lots of global variables and using pure functions > > doesn't help. > > > > Rewrite the script in Python, placing most global variables > > inside classes. This should help maintaining the script in long > > term. > > [...] > > > diff --git a/scripts/kernel-doc.py b/scripts/kernel-doc.py > > new file mode 100755 > > index 000000000000..5cf5ed63f215 > > --- /dev/null > > +++ b/scripts/kernel-doc.py > > @@ -0,0 +1,2757 @@ > > +#!/usr/bin/env python3 > > +# pylint: > > disable=R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0917,R1702 > > +# pylint: disable=C0302,C0103,C0301 > > +# pylint: disable=C0116,C0115,W0511,W0613 > > +# Copyright(c) 2025: Mauro Carvalho Chehab <[email protected]>. > > +# SPDX-License-Identifier: GPL-2.0 > > The SPDX tag is supposed to be up top, right under the shebang I'll move it. > > I also think you should give consideration to preserving the other > copyright notices in the Perl version. A language translation doesn't > remove existing copyrights...who knows how much creativity went into > some of those regexes? Makes sense, but the copyrights at kernel-doc.pl: ## Copyright (c) 1998 Michael Zucchi, All Rights Reserved ## ## Copyright (C) 2000, 1 Tim Waugh <[email protected]> ## ## Copyright (C) 2001 Simon Huggins ## ## Copyright (C) 2005-2012 Randy Dunlap ## ## Copyright (C) 2012 Dan Luedtke ## ## ## ## #define enhancements by Armin Kuster <[email protected]> ## ## Copyright (c) 2000 MontaVista Software, Inc. ## # # Copyright (C) 2022 Tomasz Warniełło (POD) Also doesn't preserve all copyrights from people that worked hard to maintain it all over those years. A quick check with git log shows 68 different authors touching it (didn't check how trivial/hard were the changes): $ git log --follow --pretty="%an" scripts/kernel-doc.pl|sort|uniq -c|sort -n 1 Alexander A. Klimov 1 Alexander Lobakin 1 Anna-Maria Behnsen 1 Bart Van Assche 1 Chen-Yu Tsai 1 Coco Li 1 Dan Luedtke 1 Donald Hunter 1 Gabriel Krisman Bertazi 1 Greg Kroah-Hartman 1 Harvey Harrison 1 Horia Geanta 1 Jason Gunthorpe 1 Jérémy Bobbio 1 Johannes Weiner 1 Jonathan Cameron 1 Kamil Rytarowski 1 Laurent Pinchart 1 Levin, Alexander (Sasha Levin) 1 Linus Torvalds 1 Lucas De Marchi 1 Mark Rutland 1 Masahiro Yamada 1 Michal Wajdeczko 1 Niklas Söderlund 1 Nishanth Menon 1 Peter Maydell 1 Pierre-Louis Bossart 1 Randy.Dunlap 1 Richard Kennedy 1 Rich Walker 1 Rolf Eike Beer 1 Silvio Fricke 1 Utkarsh Tripathi 1 [email protected] 1 Will Deacon 2 Ilya Dryomov 2 Jakub Kicinski 2 Jason Baron 2 Jonathan Neuschäfer 2 Markus Heiser 2 Pavan Kumar Linga 2 Pavel Pisa 2 Sakari Ailus 2 Yacine Belkadi 2 Yujie Liu 3 Akira Yokosawa 3 André Almeida 3 Andy Shevchenko 3 Ben Hutchings 3 Borislav Petkov 3 Conchúr Navid 3 Daniel Santos 3 Danilo Cesar Lemes de Paula 3 Mike Rapoport 4 Daniel Vetter 4 Matthew Wilcox 5 Martin Waitz 5 Paolo Bonzini 6 Aditya Srivastava 6 Vegard Nossum 7 Kees Cook 11 Johannes Berg 11 Tomasz Warniełło 20 Jonathan Corbet 32 Jani Nikula 57 Mauro Carvalho Chehab 65 Randy Dunlap So, picking the latest e-mails from the above authors, maybe we can add some credit lines like these: # Converted from the kernel-doc script originally written in Perl # under GPLv2, copyrighted since 1998 by the following authors: # # Aditya Srivastava <[email protected]> # Akira Yokosawa <[email protected]> # Alexander A. Klimov <[email protected]> # Alexander Lobakin <[email protected]> # André Almeida <[email protected]> # Andy Shevchenko <[email protected]> # Anna-Maria Behnsen <[email protected]> # Armin Kuster <[email protected]> # Bart Van Assche <[email protected]> # Ben Hutchings <[email protected]> # Borislav Petkov <[email protected]> # Chen-Yu Tsai <[email protected]> # Coco Li <[email protected]> # Conchúr Navid <[email protected]> # Daniel Santos <[email protected]> # Danilo Cesar Lemes de Paula <[email protected]> # Dan Luedtke <[email protected]> # Donald Hunter <[email protected]> # Gabriel Krisman Bertazi <[email protected]> # Greg Kroah-Hartman <[email protected]> # Harvey Harrison <[email protected]> # Horia Geanta <[email protected]> # Ilya Dryomov <[email protected]> # Jakub Kicinski <[email protected]> # Jani Nikula <[email protected]> # Jason Baron <[email protected]> # Jason Gunthorpe <[email protected]> # Jérémy Bobbio <[email protected]> # Johannes Berg <[email protected]> # Johannes Weiner <[email protected]> # Jonathan Cameron <[email protected]> # Jonathan Corbet <[email protected]> # Jonathan Neuschäfer <[email protected]> # Kamil Rytarowski <[email protected]> # Kees Cook <[email protected]> # Laurent Pinchart <[email protected]> # Levin, Alexander (Sasha Levin) <[email protected]> # Linus Torvalds <[email protected]> # Lucas De Marchi <[email protected]> # Mark Rutland <[email protected]> # Markus Heiser <[email protected]> # Martin Waitz <[email protected]> # Masahiro Yamada <[email protected]> # Matthew Wilcox <[email protected]> # Mauro Carvalho Chehab <[email protected]> # Michal Wajdeczko <[email protected]> # Michael Zucchi # Mike Rapoport <[email protected]> # Niklas Söderlund <[email protected]> # Nishanth Menon <[email protected]> # Paolo Bonzini <[email protected]> # Pavan Kumar Linga <[email protected]> # Pavel Pisa <[email protected]> # Peter Maydell <[email protected]> # Pierre-Louis Bossart <[email protected]> # Randy Dunlap <[email protected]> # Richard Kennedy <[email protected]> # Rich Walker <[email protected]> # Rolf Eike Beer <[email protected]> # Sakari Ailus <[email protected]> # Silvio Fricke <[email protected]> # Simon Huggins # Tim Waugh <[email protected]> # Tomasz Warniełło <[email protected]> # Utkarsh Tripathi <[email protected]> # [email protected] <[email protected]> # Vegard Nossum <[email protected]> # Will Deacon <[email protected]> # Yacine Belkadi <[email protected]> # Yujie Liu <[email protected]> Note: unfortunately, two of the original authors didn't send any patches with their names since when we migrated to git. So, I'm placing them without e-mails. On a side note, I would keep the above credits line only at the main kernel-doc.py, as it would be really hard to distribute it along the several kernel-doc classes. > > > +# TODO: implement warning filtering > > + > > +""" > > +kernel_doc > > +========== > > + > > +Print formatted kernel documentation to stdout > > + > > +Read C language source or header FILEs, extract embedded > > +documentation comments, and print formatted documentation > > +to standard output. > > + > > +The documentation comments are identified by the "/**" > > +opening comment mark. > > + > > +See Documentation/doc-guide/kernel-doc.rst for the > > +documentation comment syntax. > > +""" > > + > > +import argparse > > +import logging > > +import os > > +import re > > +import sys > > + > > +from datetime import datetime > > +from pprint import pformat > > + > > +from dateutil import tz > > + > > +# Local cache for regular expressions > > +re_cache = {} > > + > > + > > +class Re: > > So I have to say this bugs me a bit ... the class is fine, but the > one-letter case-only difference from the standard "re" class is just > going to make the code harder for others to approach. "kern_re" or > something like that? Or even "kre" if you really want it to be as short > as possible. A short name close to "re" made easier to convert the script ;-) I'll rename the class to KernRe (*) (*) I opted to follow pylint convention for class names, which is using camel case. As we don't have classes in C, this doesn't strictly conflicts with our Kernel conventions ;-) To make easier, I'll do such change as a separate patch at the end of the series. > > > + """ > > + Helper class to simplify regex declaration and usage, > > + > > + It calls re.compile for a given pattern. It also allows adding > > + regular expressions and define sub at class init time. > > + > > + Regular expressions can be cached via an argument, helping to speedup > > + searches. > > + """ > > [...] > > > + > > +class KernelDoc: > > + # Parser states > > + STATE_NORMAL = 0 # normal code > > + STATE_NAME = 1 # looking for function name > > + STATE_BODY_MAYBE = 2 # body - or maybe more description > > + STATE_BODY = 3 # the body of the comment > > + STATE_BODY_WITH_BLANK_LINE = 4 # the body which has a blank line > > + STATE_PROTO = 5 # scanning prototype > > + STATE_DOCBLOCK = 6 # documentation block > > + STATE_INLINE = 7 # gathering doc outside main block > > + > > + st_name = [ > > + "NORMAL", > > + "NAME", > > + "BODY_MAYBE", > > + "BODY", > > + "BODY_WITH_BLANK_LINE", > > + "PROTO", > > + "DOCBLOCK", > > + "INLINE", > > + ] > > So these ... kind of look like enums? Yes. I considered using Python enum there: https://docs.python.org/3/library/enum.html but it would simply be: from enum import Enum class state(enum): NORMAL = 0 # normal code NAME = 1 # looking for function name BODY_MAYBE = 2 # body - or maybe more description BODY = 3 # the body of the comment BODY_WITH_BLANK_LINE = 4 # the body which has a blank line PROTO = 5 # scanning prototype DOCBLOCK = 6 # documentation block INLINE = 7 # gathering doc outside main block and we would replace: self.STATE_NORMAL ... to: state.NORMAL (and a similar change for inline states as well) Except for an ancillary number-to-string conversion with this syntax: Color = Enum('Color', [('RED', 1), ('GREEN', 2), ('BLUE', 3)]) I didn't see much advantage of using it. See, we only use the string on a single line, when --debug is used to show the per-line state machine. Yet, perhaps we can split the states on a separate class anyway. > That's kind of it for nits ... I do have one wish that will kind of hard > to grant overall ... for the long-term maintenance of this code, it > would be really nice if every non-trivial regex were described by a > comment explaining what it is trying to do. It's not reasonable to > expect that as a condition for accepting this rewrite, but it sure would > be a nice goal to be working toward. Agreed. For the future, the best would be to request a description for every new regex added. For the future, my plan is to split the C source parser (e.g. the code that calls output_declaration() and/or modifies self.entry at the KernelDoc class inside kdoc_parser to a separate file and class. We can then work to implement some regexes to a more lexical way. For instance, all those regexes: (Re(r'\bstruct_group\s*\(([^,]*,)', re.S), r'STRUCT_GROUP('), (Re(r'\bstruct_group_attr\s*\(([^,]*,){2}', re.S), r'STRUCT_GROUP('), (Re(r'\bstruct_group_tagged\s*\(([^,]*),([^,]*),', re.S), r'struct \1 \2; STRUCT_GROUP('), (Re(r'\b__struct_group\s*\(([^,]*,){3}', re.S), r'STRUCT_GROUP('), (Re(r'__ETHTOOL_DECLARE_LINK_MODE_MASK\s*\(([^\)]+)\)', re.S), r'DECLARE_BITMAP(\1, __ETHTOOL_LINK_MODE_MASK_NBITS)'), (Re(r'DECLARE_PHY_INTERFACE_MASK\s*\(([^\)]+)\)', re.S), r'DECLARE_BITMAP(\1, PHY_INTERFACE_MODE_MAX)'), (Re(r'DECLARE_BITMAP\s*\(' + args_pattern + r',\s*' + args_pattern + r'\)', re.S), r'unsigned long \1[BITS_TO_LONGS(\2)]'), (Re(r'DECLARE_HASHTABLE\s*\(' + args_pattern + r',\s*' + args_pattern + r'\)', re.S), r'unsigned long \1[1 << ((\2) - 1)]'), (Re(r'DECLARE_KFIFO\s*\(' + args_pattern + r',\s*' + args_pattern + r',\s*' + args_pattern + r'\)', re.S), r'\2 *\1'), (Re(r'DECLARE_KFIFO_PTR\s*\(' + args_pattern + r',\s*' + args_pattern + r'\)', re.S), r'\2 *\1'), (Re(r'(?:__)?DECLARE_FLEX_ARRAY\s*\(' + args_pattern + r',\s*' + args_pattern + r'\)', re.S), r'\1 \2[]'), (Re(r'DEFINE_DMA_UNMAP_ADDR\s*\(' + args_pattern + r'\)', re.S), r'dma_addr_t \1'), (Re(r'DEFINE_DMA_UNMAP_LEN\s*\(' + args_pattern + r'\)', re.S), r'__u32 \1'), could be converted on a more lexical representation where the macro name with their arguments could be represented on a clearer expression. Something like: ReplaceMatch("DECLARE_HASHTABLE($1, $2)", "unsigned long $1[1 << (($2) - 1)]") We have already the NestedMatch class that do something similar to that(*) could be improved to do things like that - or we can come up with something new. (*) NestedMatch currently miss one feature: it doesn't split the contents inside parenthesis on multiple match groups. I guess it won't be hard to add such features to it. Thanks, Mauro
