On Tue, 19 Aug 2025 at 13:11, Daniel P. Berrangé <berra...@redhat.com> 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: qemu-sta...@nongnu.org
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > ---
> > 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

Reply via email to