linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Linux Documentation <linux-doc@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: about make mandocs warnings
Date: Sun, 26 Oct 2025 08:59:17 -0300	[thread overview]
Message-ID: <20251026085906.2d7e1d70@sal.lan> (raw)
In-Reply-To: <efbccba7-7377-409d-9d0a-4e99b464e2ab@infradead.org>

Em Sat, 25 Oct 2025 16:49:21 -0700
Randy Dunlap <rdunlap@infradead.org> escreveu:

> Hi Mauro,
> 
> 
> 'make mandocs' on the full kernel tree generates a little over 10,000
> lines of Warning:s.  Then it says:
>   Warning: kernel-doc returned 138 warnings
> 
> What does that number mean?

Basically, at kdoc_files, we have:

    def warning(self, msg):
        """Ancillary routine to output a warning and increment error count"""

        self.config.log.warning(msg)
        self.errors += 1

    def error(self, msg):
        """Ancillary routine to output an error and increment error count"""

        self.config.log.error(msg)
        self.errors += 1

And kernel-doc itself exits with:

	error_count = kfiles.errors
	sys.exit(error_count)

I guess the problem here is that POSIX exit codes are 8 bits only, so
it ends reporting a wrong number of errors. Not sure what would be the
best way to fix it.

> kernel-doc is doing a very good job at finding issues in kernel-doc
> notation, but there are a few cases of erroneous reporting. These are not
> numerous and may not be worth much effort to fix them, although
> some of them should not take much effort (I think). But I am just
> reporting these in case someone wants to fix them.
> 
> 
> Examples:
> 
> Warning: drivers/misc/vmw_balloon.c:259 struct member '5' not described in 'vmballoon_batch_entry'
> "5" is part of an expression "PAGE_SHIFT - 5" for number of bits in a bitfield:
> struct vmballoon_batch_entry {
> 	u64 status : 5;
> 	u64 reserved : PAGE_SHIFT - 5;
> 	u64 pfn : 52;
> } __packed;

Fixing this one could be tricky, if we want to allow math expressions 
at the regex, but sounds doable.

> 
> Warning: net/netfilter/nft_set_pipapo.h:102 union member '32' not described in 'nft_pipapo_map_bucket'
> "32" is part of a static_assert() expression. Probably just
> ignore the entire static_assert() [any number of lines].

Yeah.

> Warning: include/linux/hrtimer_types.h:47 Invalid param: enum hrtimer_restart            (*__private function)(struct hrtimer *)
> Warning: include/linux/hrtimer_types.h:47 struct member 'enum hrtimer_restart            (*__private function' not described in 'hrtimer'
> "__private" is an attribute from <linux/compiler_types.h> and the
> struct member here should be "function", which is described.
> 
> 
> Warning: include/linux/rethook.h:38 Invalid param: void (__rcu *handler) (struct rethook_node *, void *, unsigned long, struct pt_regs *)
> Warning: include/linux/rethook.h:38 struct member 'void (__rcu *handler' not described in 'rethook'
> "__rcu" is an attribute from <linux/compiler_types.h> and the
> struct member here is "handler", which is described.
> 
> Warning: security/ipe/hooks.c:54 function parameter '__always_unused' not described in 'ipe_mmap_file'
> Warning: security/ipe/hooks.c:82 function parameter '__always_unused' not described in 'ipe_file_mprotect'
> "__always_unused" is an attribute from <linux/compiler_attributes.h>.
> 

Probably all we need for the above is to add one line for each,
to ignore the above macros.

> 
> Warning: security/landlock/fs.c:765 Invalid param: layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS]
> Warning: security/landlock/fs.c:765 function parameter 'layer_mask_t (*const layer_masks_parent1' not described in 'is_access_to_paths_allowed'
> Warning: security/landlock/fs.c:765 Invalid param: layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS]
> Warning: security/landlock/fs.c:765 function parameter 'layer_mask_t (*const layer_masks_parent2' not described in 'is_access_to_paths_allowed'
> @layer_masks_parent1 and @layer_masks_parent2 are described in kernel-doc.
>
> Warning: security/landlock/fs.c:1142 Invalid param: layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS]
> Warning: security/landlock/fs.c:1142 function parameter 'layer_mask_t (*const layer_masks_dom' not described in 'collect_domain_accesses'
> @layer_masks_dom is described in kernel-doc.
>
> 
> Warning: security/landlock/ruleset.c:210 Invalid param: const struct landlock_layer (*const layers)[]
> Warning: security/landlock/ruleset.c:210 function parameter 'const struct landlock_layer (*const layers' not described in 'insert_rule'
> @layers is described in kernel-doc.
> 
> Warning: security/landlock/ruleset.c:693 Invalid param: layer_mask_t (*const layer_masks)[]
> Warning: security/landlock/ruleset.c:693 function parameter 'layer_mask_t (*const layer_masks' not described in 'landlock_init_layer_masks'
> @layer_masks is described in kernel-doc.

These would require some extra logic - or a fix at an existing one - to 
better handle parenthesis on arguments.

> Note: hundreds (probably thousands) of the mandocs warnings would disappear
> if kernel-doc accepted '-' in addition to ':' as a function parameter
> or struct/union/enum member separator (like it does for
> function/struct/union/enum short description).

This is easy to fix, and QEMU has a patch mentioning what is needed
at:
	9cbe72b868b7 ("scripts/kernel-doc: tweak for QEMU coding standards")

on its description: basically two regexes from Perl code would need changes:

        -       if (/\s*([\w\s]+?)(\(\))?\s*-/) {
        +       if (/\s*([\w\s]+?)(\s*-|:)/) {

and:
        -       if (/-(.*)/) {
        +       if (/[-:](.*)/) {

If I'm not mistaken, I got rid of the second regex during rewrite,
but I might be wrong. If I recall correctly, with Python code, the
change would be aon a single place at scripts/lib/kdoc/kdoc_parser.py:

	doc_sect = doc_com + \
-	    KernRe(r'\s*(@[.\w]+|@\.\.\.|' + known_section_names + r')\s*:([^:].*)?$',
+	    KernRe(r'\s*(@[.\w]+|@\.\.\.|' + known_section_names + r')\s*[:-]([^:].*)?$',
	           flags=re.I, cache=False)

btw, the [^:] pattern there seems to be trying to avoid catching
"::". With the new proposed regex, and if "::" is something that we
need to avoid, if one uses "-:", it would miss the description. 
I guess that's ok.

From my side, I'm OK with the new regex, but one has to verify if
this won't cause unwanted side-effects.

Regards,
Mauro

  reply	other threads:[~2025-10-26 11:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-25 23:49 about make mandocs warnings Randy Dunlap
2025-10-26 11:59 ` Mauro Carvalho Chehab [this message]
2025-10-26 16:43   ` Randy Dunlap
2025-11-07 10:15     ` Mauro Carvalho Chehab
2025-11-07 21:33       ` about make mandocs warnings (1) Randy Dunlap
2025-11-07 22:54         ` Randy Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251026085906.2d7e1d70@sal.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).