2023-10-30 08:59:05

by Yujie Liu

[permalink] [raw]
Subject: [PATCH] scripts/kernel-doc: Fix the regex for matching -Werror flag

Swarup reported a "make htmldocs" warning:

Variable length lookbehind is experimental in regex;
marked by <-- HERE in m/(?<=^|\s)-Werror(?=$|\s)
<-- HERE / at ./scripts/kernel-doc line 188.

Akira managed to reproduce it by perl v5.34.0.

On second thought, it is not necessary to have the complicated
"lookahead and lookbehind" things, and the regex can be simplified.

Generally, the kernel-doc warnings should be considered as errors only
when "-Werror" flag is set in KCFLAGS, but not when
"-Werror=<diagnostic-type>" is set, which means there needs to be a
space or start of string before "-Werror", and a space or end of string
after "-Werror".

The following cases have been tested to work as expected:

* kernel-doc warnings are considered as errors:

$ KCFLAGS="-Werror" make W=1
$ KCFLAGS="-Wcomment -Werror" make W=1
$ KCFLAGS="-Werror -Wundef" make W=1
$ KCFLAGS="-Wcomment -Werror -Wundef" make W=1

* kernel-doc warnings remain as warnings:

$ KCFLAGS="-Werror=return-type" make W=1
$ KCFLAGS="-Wcomment -Werror=return-type" make W=1
$ KCFLAGS="-Werror=return-type -Wundef" make W=1
$ KCFLAGS="-Wcomment -Werror=return-type -Wundef" make W=1

The "Variable length lookbehind is experimental in regex" warning is
also resolved by this patch.

Fixes: 91f950e8b9d8 ("scripts/kernel-doc: match -Werror flag strictly")
Reported-by: Swarup Laxman Kotiaklapudi <[email protected]>
Cc: Akira Yokosawa <[email protected]>
Signed-off-by: Yujie Liu <[email protected]>
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index d660e1f4b483..08a3e603db19 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -185,7 +185,7 @@ if (defined($ENV{'KBUILD_VERBOSE'}) && $ENV{'KBUILD_VERBOSE'} =~ '1') {
if (defined($ENV{'KCFLAGS'})) {
my $kcflags = "$ENV{'KCFLAGS'}";

- if ($kcflags =~ /(?<=^|\s)-Werror(?=$|\s)/) {
+ if ($kcflags =~ /(\s|^)-Werror(\s|$)/) {
$Werror = 1;
}
}
--
2.34.1


2023-10-30 09:55:10

by Akira Yokosawa

[permalink] [raw]
Subject: Re: [PATCH] scripts/kernel-doc: Fix the regex for matching -Werror flag

Hi,

On 2023/10/30 17:54, Yujie Liu wrote:
> Swarup reported a "make htmldocs" warning:
>
> Variable length lookbehind is experimental in regex;
> marked by <-- HERE in m/(?<=^|\s)-Werror(?=$|\s)
> <-- HERE / at ./scripts/kernel-doc line 188.
>
> Akira managed to reproduce it by perl v5.34.0.
>
> On second thought, it is not necessary to have the complicated
> "lookahead and lookbehind" things, and the regex can be simplified.

Nice!

As a matter of fact, that experimental "Variable length lookbehind"
support was new to perl v5.30. So your previous change didn't work on
systems such as Debian buster, RHEL 8, and the like.

Thank you for the quick fix.

Let me add a couple of tags.

>
> Generally, the kernel-doc warnings should be considered as errors only
> when "-Werror" flag is set in KCFLAGS, but not when
> "-Werror=<diagnostic-type>" is set, which means there needs to be a
> space or start of string before "-Werror", and a space or end of string
> after "-Werror".
>
> The following cases have been tested to work as expected:
>
> * kernel-doc warnings are considered as errors:
>
> $ KCFLAGS="-Werror" make W=1
> $ KCFLAGS="-Wcomment -Werror" make W=1
> $ KCFLAGS="-Werror -Wundef" make W=1
> $ KCFLAGS="-Wcomment -Werror -Wundef" make W=1
>
> * kernel-doc warnings remain as warnings:
>
> $ KCFLAGS="-Werror=return-type" make W=1
> $ KCFLAGS="-Wcomment -Werror=return-type" make W=1
> $ KCFLAGS="-Werror=return-type -Wundef" make W=1
> $ KCFLAGS="-Wcomment -Werror=return-type -Wundef" make W=1
>
> The "Variable length lookbehind is experimental in regex" warning is
> also resolved by this patch.
>
> Fixes: 91f950e8b9d8 ("scripts/kernel-doc: match -Werror flag strictly")
> Reported-by: Swarup Laxman Kotiaklapudi <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/

> Cc: Akira Yokosawa <[email protected]>
Reviewed-by: Akira Yokosawa <[email protected]>

> Signed-off-by: Yujie Liu <[email protected]>
> ---
> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index d660e1f4b483..08a3e603db19 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -185,7 +185,7 @@ if (defined($ENV{'KBUILD_VERBOSE'}) && $ENV{'KBUILD_VERBOSE'} =~ '1') {
> if (defined($ENV{'KCFLAGS'})) {
> my $kcflags = "$ENV{'KCFLAGS'}";
>
> - if ($kcflags =~ /(?<=^|\s)-Werror(?=$|\s)/) {
> + if ($kcflags =~ /(\s|^)-Werror(\s|$)/) {

Simpler is better. I can read this ;-)

Thanks, Akira

> $Werror = 1;
> }
> }

2023-10-30 16:53:30

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] scripts/kernel-doc: Fix the regex for matching -Werror flag

Yujie Liu <[email protected]> writes:

> Swarup reported a "make htmldocs" warning:
>
> Variable length lookbehind is experimental in regex;
> marked by <-- HERE in m/(?<=^|\s)-Werror(?=$|\s)
> <-- HERE / at ./scripts/kernel-doc line 188.
>
> Akira managed to reproduce it by perl v5.34.0.
>
> On second thought, it is not necessary to have the complicated
> "lookahead and lookbehind" things, and the regex can be simplified.
>
> Generally, the kernel-doc warnings should be considered as errors only
> when "-Werror" flag is set in KCFLAGS, but not when
> "-Werror=<diagnostic-type>" is set, which means there needs to be a
> space or start of string before "-Werror", and a space or end of string
> after "-Werror".
>
> The following cases have been tested to work as expected:
>
> * kernel-doc warnings are considered as errors:
>
> $ KCFLAGS="-Werror" make W=1
> $ KCFLAGS="-Wcomment -Werror" make W=1
> $ KCFLAGS="-Werror -Wundef" make W=1
> $ KCFLAGS="-Wcomment -Werror -Wundef" make W=1
>
> * kernel-doc warnings remain as warnings:
>
> $ KCFLAGS="-Werror=return-type" make W=1
> $ KCFLAGS="-Wcomment -Werror=return-type" make W=1
> $ KCFLAGS="-Werror=return-type -Wundef" make W=1
> $ KCFLAGS="-Wcomment -Werror=return-type -Wundef" make W=1
>
> The "Variable length lookbehind is experimental in regex" warning is
> also resolved by this patch.
>
> Fixes: 91f950e8b9d8 ("scripts/kernel-doc: match -Werror flag strictly")
> Reported-by: Swarup Laxman Kotiaklapudi <[email protected]>
> Cc: Akira Yokosawa <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
> ---
> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index d660e1f4b483..08a3e603db19 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -185,7 +185,7 @@ if (defined($ENV{'KBUILD_VERBOSE'}) && $ENV{'KBUILD_VERBOSE'} =~ '1') {
> if (defined($ENV{'KCFLAGS'})) {
> my $kcflags = "$ENV{'KCFLAGS'}";
>
> - if ($kcflags =~ /(?<=^|\s)-Werror(?=$|\s)/) {
> + if ($kcflags =~ /(\s|^)-Werror(\s|$)/) {
> $Werror = 1;

OK, I've applied this one and will sneak it into the 6.7 pull request,
thanks.

jon