Some kernel-doc fixes

PATCH Superseded Mauro Carvalho Chehab 2 participants 10 messages May 20, 2025

Subsystems (3)

Documentation Kbuild Documentation scripts

lore

Share & tools

lore

Mauro Carvalho Chehab

author

2025-05-20 13:33 UTC #0 lore raw

cover Some kernel-doc fixes

Hi Jon,

Let me consolidate some patches on a single PR to make life simpler for you. Those should address Stephen and Akira's concerns with regards to KernelDoc class usage via sphinx kerneldoc.py extension.

Patch 1: don't let Sphinx suppress errors/warnings;
Patch 2: fix a KeyError when trying to acess data from non-existing files;
Patch 3: add try/except blocks to avoid crashes when handling bad

	kernel-doc markups;

Patch 4: makes Lore and kernel-doc ML receive patches related
to kernel-doc.py and get_abi.py.

Patches 1 to 3 were already submitted on separate series. Patch 4 is new.

Regards,
Mauro

Mauro Carvalho Chehab (4):
docs: kerneldoc.py: don't use Sphinx logger
scripts: kernel-doc: prevent a KeyError when checking output
docs: kerneldoc.py: add try/except blocks for kernel-doc class errors
MAINTAINERS: update linux-doc entry to cover new Python scripts

 Documentation/sphinx/kerneldoc.py | 27 ++++++++++++++++++++++-----
 MAINTAINERS                       |  5 ++++-
 scripts/lib/kdoc/kdoc_files.py    |  4 ++++
 3 files changed, 30 insertions(+), 6 deletions(-)

Signature · 1 line

2.49.0

Mauro Carvalho Chehab

author

2025-05-20 13:33 UTC #1 lore raw

PATCH 1/4 docs: kerneldoc.py: don't use Sphinx logger

Unfortunately, currently Sphinx logger is suppressing too much, not allowing warnings to be displayed. Disable it.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 Documentation/sphinx/kerneldoc.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Documentation/sphinx/kerneldoc.py +5 -1

index b713a2c4a615..314479718a01 100644
@@ -311,7 +311,11 @@ def setup_kfiles(app):
     if kerneldoc_bin and kerneldoc_bin.endswith("kernel-doc.py"):
         print("Using Python kernel-doc")
         out_style = RestFormat()
-        kfiles = KernelFiles(out_style=out_style, logger=logger)
+
+        # Ideally, we should be using Sphinx logger here, but its filtering
+        # rules ending filtering out warnings and errors. So, let's use
+        # Python default logger instead.
+        kfiles = KernelFiles(out_style=out_style)
     else:
         print(f"Using {kerneldoc_bin}")

Signature · 1 line

2.49.0

Mauro Carvalho Chehab

author

2025-05-20 13:33 UTC #2 lore raw

PATCH 2/4 scripts: kernel-doc: prevent a KeyError when checking output

In reply to Mauro Carvalho Chehab

If a file sent to KernelFiles.msg() method doesn't exist, instead of producing a KeyError, output an error message.

Reported-by Randy Dunlap <[email protected]>

Closes https://lore.kernel.org/linux-doc/[email protected]/T/#ma43ae9d8d0995b535cf5099e5381dace0410de04 open here

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 scripts/lib/kdoc/kdoc_files.py | 4 ++++
 1 file changed, 4 insertions(+)

scripts/lib/kdoc/kdoc_files.py +4 -0

index 630aa5ca6460..9be4a64df71d 100644
@@ -271,6 +271,10 @@ class KernelFiles():
                                       no_doc_sections)
 
             msg = ""
+            if fname not in self.results:
+                self.config.log.warning("No kernel-doc for file %s", fname)
+                continue
+
             for name, arg in self.results[fname]:
                 m = self.out_msg(fname, name, arg)

Signature · 1 line

2.49.0

Mauro Carvalho Chehab

author

2025-05-20 13:33 UTC #3 lore raw

PATCH 3/4 docs: kerneldoc.py: add try/except blocks for kernel-doc class errors

In reply to Mauro Carvalho Chehab

Replicate the same behavior as what's done with kernel-doc.pl:
continue building docs even when there are exceptions.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 Documentation/sphinx/kerneldoc.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Documentation/sphinx/kerneldoc.py +17 -4

index 314479718a01..4de667d4d95b 100644
@@ -278,14 +278,27 @@ class KernelDocDirective(Directive):
 
         node = nodes.section()
 
-        kfiles.parse(**self.parse_args)
-        filenames = self.parse_args["file_list"]
+        try:
+            kfiles.parse(**self.parse_args)
+            filenames = self.parse_args["file_list"]
+            msgs = kfiles.msg(**self.msg_args, filenames=filenames)
 
-        for filename, out in kfiles.msg(**self.msg_args, filenames=filenames):
+        except Exception as e:  # pylint: disable=W0703
+            logger.warning("kernel-doc '%s' processing failed with: %s" %
+                           (cmd_str(cmd), str(e)))
+
+        for filename, out in msgs:
             if self.verbose >= 1:
                 print(cmd_str(cmd))
 
-            ret = self.parse_msg(filename, node, out, cmd)
+            try:
+                ret = self.parse_msg(filename, node, out, cmd)
+
+            except Exception as e:  # pylint: disable=W0703
+                logger.warning("kernel-doc '%s' processing failed with: %s" %
+                               (cmd_str(cmd), str(e)))
+                return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
+
             if ret:
                 return ret

Signature · 1 line

2.49.0

Andy Shevchenko

2025-05-20 14:22 UTC #4 lore raw

Re: [PATCH 3/4] docs: kerneldoc.py: add try/except blocks for kernel-doc class errors

Quoted 2 lines · Mauro Carvalho Chehab

Replicate the same behavior as what's done with kernel-doc.pl:
continue building docs even when there are exceptions.

...

Documentation/sphinx/kerneldoc.py class KernelDocDirective(Directive): · line 299

Quoted 5 lines

Documentation/sphinx/kerneldoc.py +4 -0

+            logger.warning("kernel-doc '%s' processing failed with: %s" %
+                           (cmd_str(cmd), str(e)))
 
+                logger.warning("kernel-doc '%s' processing failed with: %s" %
+                               (cmd_str(cmd), str(e)))

The prefix of the message is the same for different (semantically) places. Is it okay? (I would expect them to slightly differ, but I dunno if cmd here is the same, perhaps that's enough for distinguishing the two.)

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Mauro Carvalho Chehab

author

2025-05-20 14:50 UTC #5 lore raw

Re: [PATCH 3/4] docs: kerneldoc.py: add try/except blocks for kernel-doc class errors

Em Tue, 20 May 2025 17:22:27 +0300
Andy Shevchenko <[email protected]> escreveu:

Quoted 9 lines · Mauro Carvalho Chehab

Quoted 2 lines
Replicate the same behavior as what's done with kernel-doc.pl:
continue building docs even when there are exceptions.
...
Quoted 2 lines
(quoted diff) +2 -0
+            logger.warning("kernel-doc '%s' processing failed with: %s" %
+                           (cmd_str(cmd), str(e)))  

Quoted 6 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+                logger.warning("kernel-doc '%s' processing failed with: %s" %
+                               (cmd_str(cmd), str(e)))  
The prefix of the message is the same for different (semantically) places. Is it okay? (I would expect them to slightly differ, but I dunno if cmd here is the same, perhaps that's enough for distinguishing the two.)

I guess it should be OK, as the "%s" variables are the ones that will actually help to provide a hint about the issue. See, in practice, if one wants to check what crashed, the procedure would likely be to run the command line, given by "cmd_str(cmd)" and see what output was produced.

Regards,
Mauro

Mauro Carvalho Chehab

author

2025-05-20 15:40 UTC #6 lore raw

Re: [PATCH 3/4] docs: kerneldoc.py: add try/except blocks for kernel-doc class errors

Em Tue, 20 May 2025 16:50:24 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:

Quoted 15 lines

Em Tue, 20 May 2025 17:22:27 +0300
Andy Shevchenko <[email protected]> escreveu:
Quoted 12 lines · Mauro Carvalho Chehab
Quoted 2 lines
Replicate the same behavior as what's done with kernel-doc.pl:
continue building docs even when there are exceptions.
...
Quoted 5 lines
(quoted diff) +4 -0
+            logger.warning("kernel-doc '%s' processing failed with: %s" %
+                           (cmd_str(cmd), str(e)))  
 
+                logger.warning("kernel-doc '%s' processing failed with: %s" %
+                               (cmd_str(cmd), str(e)))  

Quoted 8 lines

Quoted 3 lines
The prefix of the message is the same for different (semantically) places. Is it okay? (I would expect them to slightly differ, but I dunno if cmd here is the same, perhaps that's enough for distinguishing the two.)
I guess it should be OK, as the "%s" variables are the ones that will actually help to provide a hint about the issue. See, in practice, if one wants to check what crashed, the procedure would likely be to run the command line, given by "cmd_str(cmd)" and see what output was produced.

On a second thought, the try/except logic there is too complex. We need just one to cover all cases. Also, "str(e)" is not the best, as it doesn't really show the error. "pformat(e)" works a lot better:

	$ make htmldocs
	Using alabaster theme
	Using Python kernel-doc
	Cannot find file ./drivers/gpio/gpiolib-acpi.c
	Cannot find file ./drivers/gpio/gpiolib-acpi.c
	WARNING: kernel-doc './scripts/kernel-doc.py -rst -enable-lineno -export ./drivers/gpio/gpiolib-acpi.c' processing failed with: KeyError('./drivers/gpio/gpiolib-acpi.c')
	Documentation/arch/powerpc/htm.rst: WARNING: document isn't included in any toctree

See enclosed patch (to be applied after this series).

Regards,
Mauro

---

[PATCH] docs: kerneldoc.py: simplify exception handling logic

Instead of having try/except everywhere, place them on a common place.

While here, get rid of some bogus logs.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Documentation/sphinx/kerneldoc.py +51 ~~-70~~

index 833cb0285afc..3f9754b84566 100644
@@ -40,6 +40,7 @@ from docutils.parsers.rst import directives, Directive
 import sphinx
 from sphinx.util.docutils import switch_source_input
 from sphinx.util import logging
+from pprint import pformat
 
 srctree = os.path.abspath(os.environ["srctree"])
 sys.path.insert(0, os.path.join(srctree, "scripts/lib/kdoc"))

@@ -49,7 +50,7 @@ from kdoc_output import RestFormat
 
 __version__  = '1.0'
 kfiles = None
-logger = logging.getLogger('kerneldoc')
+logger = logging.getLogger(__name__)
 
 def cmd_str(cmd):
     """

@@ -190,46 +191,35 @@ class KernelDocDirective(Directive):
 
         return cmd
 
-    def run_cmd(self):
+    def run_cmd(self, cmd):
         """
         Execute an external kernel-doc command.
         """
 
         env = self.state.document.settings.env
-        cmd = self.handle_args()
 
         if self.verbose >= 1:
-            print(cmd_str(cmd))
+            logger.info(cmd_str(cmd))
 
         node = nodes.section()
 
-        try:
-            logger.verbose("calling kernel-doc '%s'" % (" ".join(cmd)))
-
-            p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            out, err = p.communicate()
-
-            out, err = codecs.decode(out, 'utf-8'), codecs.decode(err, 'utf-8')
+        p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        out, err = p.communicate()
 
-            if p.returncode != 0:
-                sys.stderr.write(err)
+        out, err = codecs.decode(out, 'utf-8'), codecs.decode(err, 'utf-8')
 
-                logger.warning("kernel-doc '%s' failed with return code %d"
-                                    % (" ".join(cmd), p.returncode))
-                return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
-            elif env.config.kerneldoc_verbosity > 0:
-                sys.stderr.write(err)
+        if p.returncode != 0:
+            sys.stderr.write(err)
 
-        except Exception as e:  # pylint: disable=W0703
-            logger.warning("kernel-doc '%s' processing failed with: %s" %
-                                (" ".join(cmd), str(e)))
+            logger.warning("kernel-doc '%s' failed with return code %d"
+                                % (" ".join(cmd), p.returncode))
             return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
+        elif env.config.kerneldoc_verbosity > 0:
+            sys.stderr.write(err)
 
         filenames = self.parse_args["file_list"]
         for filename in filenames:
-            ret = self.parse_msg(filename, node, out, cmd)
-            if ret:
-                return ret
+            self.parse_msg(filename, node, out, cmd)
 
         return node.children

@@ -240,71 +230,62 @@ class KernelDocDirective(Directive):
 
         env = self.state.document.settings.env
 
-        try:
-            lines = statemachine.string2lines(out, self.tab_width,
-                                              convert_whitespace=True)
-            result = ViewList()
-
-            lineoffset = 0;
-            line_regex = re.compile(r"^\.\. LINENO ([0-9]+)$")
-            for line in lines:
-                match = line_regex.search(line)
-                if match:
-                    # sphinx counts lines from 0
-                    lineoffset = int(match.group(1)) - 1
-                    # we must eat our comments since the upset the markup
-                else:
-                    doc = str(env.srcdir) + "/" + env.docname + ":" + str(self.lineno)
-                    result.append(line, doc + ": " + filename, lineoffset)
-                    lineoffset += 1
-
-            self.do_parse(result, node)
-
-        except Exception as e:  # pylint: disable=W0703
-            logger.warning("kernel-doc '%s' processing failed with: %s" %
-                                (cmd_str(cmd), str(e)))
-            return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
+        lines = statemachine.string2lines(out, self.tab_width,
+                                            convert_whitespace=True)
+        result = ViewList()
+
+        lineoffset = 0;
+        line_regex = re.compile(r"^\.\. LINENO ([0-9]+)$")
+        for line in lines:
+            match = line_regex.search(line)
+            if match:
+                # sphinx counts lines from 0
+                lineoffset = int(match.group(1)) - 1
+                # we must eat our comments since the upset the markup
+            else:
+                doc = str(env.srcdir) + "/" + env.docname + ":" + str(self.lineno)
+                result.append(line, doc + ": " + filename, lineoffset)
+                lineoffset += 1
 
-        return None
+        self.do_parse(result, node)
 
-    def run_kdoc(self, kfiles):
+    def run_kdoc(self, cmd, kfiles):
         """
         Execute kernel-doc classes directly instead of running as a separate
         command.
         """
 
-        cmd = self.handle_args()
         env = self.state.document.settings.env
 
         node = nodes.section()
 
-        try:
-            kfiles.parse(**self.parse_args)
-            filenames = self.parse_args["file_list"]
-
-            msgs = kfiles.msg(**self.msg_args, filenames=filenames)
-            for filename, out in msgs:
-                if self.verbose >= 1:
-                    print(cmd_str(cmd))
+        kfiles.parse(**self.parse_args)
+        filenames = self.parse_args["file_list"]
 
-                ret = self.parse_msg(filename, node, out, cmd)
-                if ret:
-                    return ret
+        msgs = kfiles.msg(**self.msg_args, filenames=filenames)
+        for filename, out in msgs:
+            if self.verbose >= 1:
+                print(cmd_str(cmd))
 
-        except Exception as e:  # pylint: disable=W0703
-            logger.warning("kernel-doc '%s' processing failed with: %s" %
-                            (cmd_str(cmd), str(e)))
-            return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
+            self.parse_msg(filename, node, out, cmd)
 
         return node.children
 
     def run(self):
         global kfiles
 
-        if kfiles:
-            return self.run_kdoc(kfiles)
-        else:
-            return self.run_cmd()
+        cmd = self.handle_args()
+
+        try:
+            if kfiles:
+                return self.run_kdoc(cmd, kfiles)
+            else:
+                return self.run_cmd(cmd)
+
+        except Exception as e:  # pylint: disable=W0703
+            logger.warning("kernel-doc '%s' processing failed with: %s" %
+                           (cmd_str(cmd), pformat(e)))
+            return [nodes.error(None, nodes.paragraph(text = "kernel-doc missing"))]
 
     def do_parse(self, result, node):
         with switch_source_input(self.state, result):

Mauro Carvalho Chehab

author

2025-05-20 13:33 UTC #7 lore raw

PATCH 4/4 MAINTAINERS: update linux-doc entry to cover new Python scripts

In reply to Mauro Carvalho Chehab

Changes to ABI and kernel-doc need to be c/c linux-doc. Update the maintainer's entry to cover those files.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 MAINTAINERS | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

MAINTAINERS +4 -1

index 20e07e61a148..a668808769b6 100644
@@ -7173,7 +7173,10 @@ T:	git git://git.lwn.net/linux.git docs-next
 F:	Documentation/
 F:	scripts/check-variable-fonts.sh
 F:	scripts/documentation-file-ref-check
-F:	scripts/kernel-doc
+F:	scripts/get_abi.py
+F:	scripts/kernel-doc*
+F	scripts/lib/abi/*
+F	scripts/lib/kdoc/*
 F:	scripts/sphinx-pre-install
 X:	Documentation/ABI/
 X:	Documentation/admin-guide/media/

Signature · 1 line

2.49.0

Andy Shevchenko

2025-05-20 14:18 UTC #8 lore raw

In reply to Mauro Carvalho Chehab

Quoted 6 lines · Mauro Carvalho Chehab

Hi Jon,
Let me consolidate some patches on a single PR to make life simpler for you. Those should address Stephen and Akira's concerns with regards to KernelDoc class usage via sphinx kerneldoc.py extension.

Quoted 8 lines

Patch 1: don't let Sphinx suppress errors/warnings;
Patch 2: fix a KeyError when trying to acess data from non-existing files;
Patch 3: add try/except blocks to avoid crashes when handling bad
	kernel-doc markups;
Patch 4: makes Lore and kernel-doc ML receive patches related
to kernel-doc.py and get_abi.py.
Patches 1 to 3 were already submitted on separate series. Patch 4 is new.

Can we actually utilise CONFIG_WERROR to fail the build. If yes, the build will be failed. This is in align with the warnings in the C code.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Mauro Carvalho Chehab

author

2025-05-20 14:31 UTC #9 lore raw

Em Tue, 20 May 2025 17:17:56 +0300
Andy Shevchenko <[email protected]> escreveu:

Quoted 14 lines · Mauro Carvalho Chehab

Quoted 13 lines
Hi Jon,
Let me consolidate some patches on a single PR to make life simpler for you. Those should address Stephen and Akira's concerns with regards to KernelDoc class usage via sphinx kerneldoc.py extension.
Patch 1: don't let Sphinx suppress errors/warnings;
Patch 2: fix a KeyError when trying to acess data from non-existing files;
Patch 3: add try/except blocks to avoid crashes when handling bad
	kernel-doc markups;
Patch 4: makes Lore and kernel-doc ML receive patches related
to kernel-doc.py and get_abi.py.

Quoted 4 lines

Quoted 1 line
Patches 1 to 3 were already submitted on separate series. Patch 4 is new.
Can we actually utilise CONFIG_WERROR to fail the build. If yes, the build will be failed. This is in align with the warnings in the C code.

It makes sense to me - either to use CONFIG_WERROR or to have something like:

make SPHINX_WERROR=1 htmldocs

Btw, kernel-doc.pl (*) and kernel-doc.py have several command-line parameters to control warnings:

  $ ./scripts/kernel-doc --help
  ... 
  -Wreturn, --wreturn   Warns about the lack of a return markup on functions.
  -Wshort-desc, -Wshort-description, --wshort-desc
                        Warns if initial short description is missing
  -Wcontents-before-sections, --wcontents-before-sections

Warns if there are contents before sections (deprecated).

This option is kept just for backward-compatibility, but it does nothing, neither here nor at the original Perl script.
-Wall, --wall Enable all types of warnings
-Werror, --werror Treat warnings as errors.
...

(*) from the above, only -Werror is documented at the Perl version.

In the future, I'm planning to do some work to improve it - including removing the deprecated -Wcontents-before-sections.

But anyway this is out of the scope of this series, as we're aiming to be bug-compatible with kernel-doc.pl. The crashes were unintended.

Regards,
Mauro