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...
> }
Yes, the intention there is to negate the regular expression match, e.g.
either ~(expr) or $param !~ regex would equally work.
> 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".
True, but, on the other hand, this will get replaced at the next merge
window, so, it won't do much difference betwenn picking the smalllest
change or the if/then/else variant.
The Python version uses if/then/else:
if dtype == "" and param.endswith("..."):
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 = "..."
So, either way:
Reviewed-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
>
> thanks
> -- PMM
--
Thanks,
Mauro
