| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250801080744.14f83626@foz.lan>
Date: Fri, 1 Aug 2025 08:07:44 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Jonathan Corbet <corbet@....net>
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, Akira Yokosawa
<akiyks@...il.com>
Subject: Re: [PATCH 10/12] docs: kdoc: further rewrite_struct_members()
cleanup
Em Thu, 31 Jul 2025 18:13:24 -0600
Jonathan Corbet <corbet@....net> escreveu:
> Get rid of some single-use variables and redundant checks, and generally
> tighten up the code; no logical change.
>
> Signed-off-by: Jonathan Corbet <corbet@....net>
> ---
> scripts/lib/kdoc/kdoc_parser.py | 89 ++++++++++++++++-----------------
> 1 file changed, 42 insertions(+), 47 deletions(-)
>
> diff --git a/scripts/lib/kdoc/kdoc_parser.py b/scripts/lib/kdoc/kdoc_parser.py
> index 20e0a2abe13b..2b7d7e646367 100644
> --- a/scripts/lib/kdoc/kdoc_parser.py
> +++ b/scripts/lib/kdoc/kdoc_parser.py
> @@ -673,73 +673,68 @@ class KernelDoc:
> while tuples:
> for t in tuples:
> newmember = ""
> - maintype = t[0]
> - s_ids = t[5]
> - content = t[3]
The reason I opted for this particular approach...
> -
> - oldmember = "".join(t)
> -
> - for s_id in s_ids.split(','):
> + oldmember = "".join(t) # Reconstruct the original formatting
> + #
> + # Pass through each field name, normalizing the form and formatting.
> + #
> + for s_id in t[5].split(','):
... is that it is easier to understand and to maintain:
for s_id in s_ids.split(','):
than when magic numbers like this are used:
for s_id in t[5].split(','):
> s_id = s_id.strip()
>
> - newmember += f"{maintype} {s_id}; "
> + newmember += f"{t[0]} {s_id}; "
> + #
> + # Remove bitfield/array/pointer info, getting the bare name.
> + #
> s_id = KernRe(r'[:[].*').sub('', s_id)
> s_id = KernRe(r'^\s*\**(\S+)\s*').sub(r'\1', s_id)
> -
> - for arg in content.split(';'):
> + #
> + # Pass through the members of this inner structure/union.
> + #
> + for arg in t[3].split(';'):
Here, for example, we're far away from the tuple definition... I can't
recall anymore what "3" magic number means ;-)
> arg = arg.strip()
> -
> - if not arg:
> - continue
> -
> + #
> + # Look for (type)(*name)(args) - pointer to function
> + #
> r = KernRe(r'^([^(]+\(\*?\s*)([\w.]*)(\s*\).*)')
> if r.match(arg):
> # Pointer-to-function
> - dtype = r.group(1)
> - name = r.group(2)
> - extra = r.group(3)
Same applies here. Having a named var makes easier to understand/maintain
rest of the code. If you're willing to do something like that, better to
use named capture groups, like:
r = KernRe(r'^(?P<dtype>[^(]+\(\*?\s*)'
r'(?P<name>[\w.]*)'
r'(?P<extra>\s*\).*)')
together with a syntax using match.group(group_name)
I'm not a particular fan of named groups, as it adds a lot more stuff
at regexes. They're already hard enough to understand without ?P<name>,
but at least match.group('dtype'), match.group('name'), match.group('extra')
inside the next calls would be easier to maintain than when using magic
numbers.
Same comments apply to other changes below.
> -
> - if not name:
> - continue
> -
> if not s_id:
> # Anonymous struct/union
> - newmember += f"{dtype}{name}{extra}; "
> + newmember += f"{r.group(1)}{r.group(2)}{r.group(3)}; "
> else:
> - newmember += f"{dtype}{s_id}.{name}{extra}; "
> -
> + newmember += f"{r.group(1)}{s_id}.{r.group(2)}{r.group(3)}; "
> + #
> + # Otherwise a non-function member.
> + #
> else:
> - # Handle bitmaps
> + #
> + # Remove bitmap and array portions and spaces around commas
> + #
> arg = KernRe(r':\s*\d+\s*').sub('', arg)
> -
> - # Handle arrays
> arg = KernRe(r'\[.*\]').sub('', arg)
> -
> - # Handle multiple IDs
> arg = KernRe(r'\s*,\s*').sub(',', arg)
> -
> + #
> + # Look for a normal decl - "type name[,name...]"
> + #
> r = KernRe(r'(.*)\s+([\S+,]+)')
> -
> if r.search(arg):
> - dtype = r.group(1)
> - names = r.group(2)
> + for name in r.group(2).split(','):
> + name = KernRe(r'^\s*\**(\S+)\s*').sub(r'\1', name)
> + if not s_id:
> + # Anonymous struct/union
> + newmember += f"{r.group(1)} {name}; "
> + else:
> + newmember += f"{r.group(1)} {s_id}.{name}; "
> else:
> newmember += f"{arg}; "
> - continue
> -
> - for name in names.split(','):
> - name = KernRe(r'^\s*\**(\S+)\s*').sub(r'\1', name).strip()
> -
> - if not name:
> - continue
> -
> - if not s_id:
> - # Anonymous struct/union
> - newmember += f"{dtype} {name}; "
> - else:
> - newmember += f"{dtype} {s_id}.{name}; "
> -
> + #
> + # At the end of the s_id loop, replace the original declaration with
> + # the munged version.
> + #
> members = members.replace(oldmember, newmember)
> + #
> + # End of the tuple loop - search again and see if there are outer members
> + # that now turn up.
> + #
> tuples = struct_members.findall(members)
> return members
>
Thanks,
Mauro
Powered by blists - more mailing lists