On Tue, Aug 19, 2025 at 01:16:25PM +0100, Peter Maydell wrote:
> 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".
Ok, I'm not that fussed so to your patch
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
