On Tue, 19 Aug 2025 at 13:11, Daniel P. Berrangé <[email protected]> wrote:
>
> On Tue, Aug 19, 2025 at 12:56:48PM +0100, Peter Maydell wrote:
> > Newer versions of Perl (5.41.x and up) emit a warning for code in
> > kernel-doc:
> > Possible precedence problem between ! and pattern match (m//) at
> > /scripts/kernel-doc line 1597.
> >
> > This is because the code does:
> > if (!$param =~ /\w\.\.\.$/) {
> >
> > In Perl, the ! operator has higher precedence than the =~
> > pattern-match binding, so the effect of this condition is to first
> > logically-negate the string $param into a true-or-false value and
> > then try to pattern match it against the regex, which in this case
> > will always fail. This is almost certainly not what the author
> > intended.
> >
> > In the new Python version of kernel-doc in the Linux kernel,
> > the equivalent code is written:
> >
> > if KernRe(r'\w\.\.\.$').search(param):
> > # For named variable parameters of the form `x...`,
> > # remove the dots
> > param = param[:-3]
> > else:
> > # Handles unnamed variable parameters
> > param = "..."
> >
> > which is a more sensible way of writing the behaviour you would
> > get if you put in brackets to make the regex match first and
> > then negate the result.
> >
> > Take this as the intended behaviour, and update the Perl to match.
> >
> > For QEMU, this produces no change in output, presumably because we
> > never used the "unnamed variable parameters" syntax.
> >
> > Cc: [email protected]
> > Signed-off-by: Peter Maydell <[email protected]>
> > ---
> > This obviously will clash with the "import the python script"
> > patchseries, but I figured it was worth providing the minimal
> > fix for the benefit of stable backports.
> >
> > The kernel's copy of kernel-doc.pl still has this bug.
> > ---
> > scripts/kernel-doc | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> > index fec83f53eda..117ec8fcd1f 100755
> > --- a/scripts/kernel-doc
> > +++ b/scripts/kernel-doc
> > @@ -1594,13 +1594,12 @@ sub push_parameter($$$$$) {
> >
> > if ($type eq "" && $param =~ /\.\.\.$/)
> > {
> > - if (!$param =~ /\w\.\.\.$/) {
>
> I think it would be possible to change this only line to
> collapse the ! and =~ into the !~ operator:
>
> if ($param !~ /\w\.\.\.$/) {
Yes, it would, but then the code would be:
if ($param !~ /\w\.\.\.$/) {
stuff...
} elsif (param =~ /\w\.\.\.$/){
other stuff...
}
where we check the condition twice rather than using "else".
So I favoured:
* "if C then {A} else {B}" is faster and more obvious as a way to say
"do A or B depending on C" than "if !C {B} elsif C {A}".
* match the logic that the Python script uses
over "smallest possible change to resolve the warning".
thanks
-- PMM