Linux Documentation
 help / color / mirror / Atom feed
From: Randy Dunlap <rdunlap@infradead.org>
To: Shuicheng Lin <shuicheng.lin@intel.com>, linux-doc@vger.kernel.org
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	linux-kernel@vger.kernel.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH v4] scripts/kernel-doc: Detect mismatched inline member documentation tags
Date: Wed, 6 May 2026 22:41:37 -0700	[thread overview]
Message-ID: <21c8813c-e3af-4764-b6f7-3d4cb1fe1c65@infradead.org> (raw)
In-Reply-To: <20260507023232.4108680-1-shuicheng.lin@intel.com>



On 5/6/26 7:32 PM, Shuicheng Lin wrote:
> Add validation in check_sections() to verify that inline member
> documentation tags (/** @member: description */) match actual struct/union
> member names. Previously, kernel-doc only validated section headers against
> the parameter list, but inline doc tags stored in parameterdescs were never
> cross-checked, allowing stale or mistyped member names to go undetected.
> 
> The new check iterates over parameterdescs keys and warns about any that
> don't appear in the parameter list, catching issues like renamed struct
> members where the documentation tag was not updated to match.
> 
> This catches real issues such as:
>   - xe_bo_types.h: @atomic_access (missing struct prefix, should be
>     @attr.atomic_access)
>   - xe_device_types.h: @usm.asid (member is actually asid_to_vm)
> 
> While at it, fix two long-standing issues with named variadic parameters
> (macros like ``#define foo(fmt, args...)``) that the new check exposed:
> 
>   1. A description provided via the ``@args...:`` doc form was stored
>      in parameterdescs under the unstripped key ``args...``, while
>      push_parameter() stripped the trailing ``...`` and only added
>      ``args`` to parameterlist.  As a result the user-supplied
>      description was orphaned, parameterdescs[``args``] was auto-
>      populated with the generic "variable arguments" text, and the
>      user's actual description was silently discarded by the output
>      stage.  Migrate the description from the unstripped to the
>      stripped key inside push_parameter() so the user's text reaches
>      the output and the new check does not flag the orphaned key.
> 
>   2. push_parameter() always auto-populated parameterdescs[param] with
>      "variable arguments" for variadic parameters, which bypassed the
>      existing "parameter not described" warning at line 549.  As a
>      consequence, a named variadic with no matching ``@<name>:`` doc
>      tag (or a mistyped one such as ``@args:`` for a parameter named
>      ``arg``) went undetected.  Emit the standard "not described"
>      warning for named variadics before applying the auto-fill, so
>      missing or mistyped variadic docs are reported just like missing
>      docs for any other parameter.  The bare ``@...:`` form is
>      unaffected because it has no natural name for the user to
>      document.
> 
> This second hunk surfaces one real pre-existing documentation gap in
> include/linux/hashtable.h: hash_for_each_possible_rcu()'s ``cond...``
> parameter has no matching ``@cond:`` doc entry.  No false positives were
> observed across include/linux, kernel/, or drivers/gpu/drm.
> 
> v2: Skip variadic parameters whose documented key ends with ``...`` and
>     whose stripped name is in parameterlist, to avoid false-positive
>     "Excess function parameter 'args...'" warnings on macros like
>     ``#define foo(fmt, args...)`` documented with ``@args...:``.
> 
> v3: The v2 special case in check_sections() only suppressed the warning
>     while still letting the user's description be silently dropped from
>     the generated output.  Replace it with a fix in push_parameter() that
>     migrates the description from ``args...`` to ``args`` when the name
>     is stripped, so the user's text is preserved end-to-end and the
>     new excess-parameter check naturally finds nothing to flag.
> 
> v4: Also emit the standard "parameter not described" warning for named
>     variadics that have no matching ``@<name>:`` doc tag.  Previously
>     push_parameter()'s unconditional auto-fill bypassed that warning,
>     so a missing or mistyped variadic doc went undetected. (Randy)
> 
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>

Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks. This is good stuff.

> ---
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: intel-xe@lists.freedesktop.org
> ---
>  tools/lib/python/kdoc/kdoc_parser.py | 54 +++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/python/kdoc/kdoc_parser.py b/tools/lib/python/kdoc/kdoc_parser.py
> index ca00695b47b3..2bc49c3ece14 100644
> --- a/tools/lib/python/kdoc/kdoc_parser.py
> +++ b/tools/lib/python/kdoc/kdoc_parser.py
> @@ -512,9 +512,36 @@ class KernelDoc:
>          #
>          if dtype == '':
>              if param.endswith("..."):
> -                if len(param) > 3: # there is a name provided, use that
> +                named_variadic = len(param) > 3
> +                if named_variadic: # there is a name provided, use that
> +                    #
> +                    # If the user documented the parameter using the
> +                    # ``@name...:`` form, the description is stored in
> +                    # parameterdescs under the unstripped key.  Migrate
> +                    # it to the stripped key so the user's text is not
> +                    # silently dropped during output, and so the new
> +                    # excess-parameter check in check_sections() does
> +                    # not flag the unstripped key as orphaned.
> +                    #
> +                    orig = self.entry.parameterdescs.pop(param, None)
>                      param = param[:-3]
> +                    if orig is not None and \
> +                       not self.entry.parameterdescs.get(param):
> +                        self.entry.parameterdescs[param] = orig
>                  if not self.entry.parameterdescs.get(param):
> +                    #
> +                    # For a named variadic (e.g. ``args...``), emit the
> +                    # standard "not described" warning before auto-filling
> +                    # so a missing or mistyped ``@<name>:`` doc tag does
> +                    # not go undetected.  The bare ``...`` form has no
> +                    # natural name for the user to document and so always
> +                    # gets the auto-generated text.
> +                    #
> +                    if named_variadic and decl_type == 'function':
> +                        self.emit_msg(ln,
> +                                      f"function parameter '{param}' "
> +                                      f"not described in "
> +                                      f"'{declaration_name}'")
>                      self.entry.parameterdescs[param] = "variable arguments"
>  
>              elif (not param) or param == "void":
> @@ -673,6 +700,31 @@ class KernelDoc:
>                  self.emit_msg(ln,
>                                f"Excess {dname} '{section}' description in '{decl_name}'")
>  
> +        #
> +        # Check that documented parameter names (from doc comments, including
> +        # inline ``/** @member: */`` tags) actually match real members in
> +        # the declaration.  This catches mismatched or stale kernel-doc
> +        # member tags that don't correspond to any actual struct/union
> +        # member or function parameter.
> +        #
> +        for param_name, desc in self.entry.parameterdescs.items():
> +            # Skip auto-generated entries from push_parameter()
> +            if desc == self.undescribed:
> +                continue
> +            if desc in ("no arguments", "anonymous\n", "variable arguments"):
> +                continue
> +            if param_name.startswith("{unnamed_"):
> +                continue
> +            if param_name in self.entry.parameterlist:
> +                continue
> +
> +            if decl_type == 'function':
> +                dname = f"{decl_type} parameter"
> +            else:
> +                dname = f"{decl_type} member"
> +            self.emit_msg(ln,
> +                          f"Excess {dname} '{param_name}' description in '{decl_name}'")
> +
>      def check_return_section(self, ln, declaration_name, return_type):
>          """
>          If the function doesn't return void, warns about the lack of a

-- 
~Randy

  reply	other threads:[~2026-05-07  5:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07  2:32 [PATCH v4] scripts/kernel-doc: Detect mismatched inline member documentation tags Shuicheng Lin
2026-05-07  5:41 ` Randy Dunlap [this message]
2026-05-15 14:42 ` Jonathan Corbet

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=21c8813c-e3af-4764-b6f7-3d4cb1fe1c65@infradead.org \
    --to=rdunlap@infradead.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shuicheng.lin@intel.com \
    /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