2022-06-08 14:54:40

by Niklas Söderlund

[permalink] [raw]
Subject: [PATCH] scripts: kernel-doc: Always increment warnings counter

Some warnings did not increment the warnings counter making the behavior
of running kernel-doc with -Werror unlogical as some warnings would be
generated but not treated as errors.

Fix this by always incrementing the warnings counter every time a
warning related to the input documentation is generated. There is one
location in get_sphinx_version() where a warning is printed and the
counter is not touched as it concerns the execution environment of the
kernel-doc and not the documentation being processed.

Incrementing the counter only have effect when running kernel-doc in
either verbose mode (-v or environment variable KBUILD_VERBOSE) or when
treating warnings as errors (-Werror or environment variable
KDOC_WERROR). In both cases the number of warnings printed is printed to
stderr and for the later the exit code of kernel-doc is non-zero if
warnings where encountered.

Simple test case to demo one of the warnings,

$ cat test.c
/**
* foo() - Description
*/
int bar();

# Without this change
$ ./scripts/kernel-doc -Werror -none test.c
test.c:4: warning: expecting prototype for foo(). Prototype was for
bar() instead

# With this change
$ ./scripts/kernel-doc -Werror -none test.c
test.c:4: warning: expecting prototype for foo(). Prototype was for
bar() instead
1 warnings as Errors

Signed-off-by: Niklas Söderlund <[email protected]>
Signed-off-by: Simon Horman <[email protected]>
Signed-off-by: Louis Peens <[email protected]>
---
scripts/kernel-doc | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 7516949bb049e39f..0d1bde9e44f98d39 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1095,6 +1095,7 @@ sub dump_struct($$) {
if ($members) {
if ($identifier ne $declaration_name) {
print STDERR "${file}:$.: warning: expecting prototype for $decl_type $identifier. Prototype was for $decl_type $declaration_name instead\n";
+ ++$warnings;
return;
}

@@ -1302,6 +1303,7 @@ sub dump_enum($$) {
} else {
print STDERR "${file}:$.: warning: expecting prototype for enum $identifier. Prototype was for enum $declaration_name instead\n";
}
+ ++$warnings;
return;
}
$declaration_name = "(anonymous)" if ($declaration_name eq "");
@@ -1317,6 +1319,7 @@ sub dump_enum($$) {
$parameterdescs{$arg} = $undescribed;
if (show_warnings("enum", $declaration_name)) {
print STDERR "${file}:$.: warning: Enum value '$arg' not described in enum '$declaration_name'\n";
+ ++$warnings;
}
}
$_members{$arg} = 1;
@@ -1326,6 +1329,7 @@ sub dump_enum($$) {
if (!exists($_members{$k})) {
if (show_warnings("enum", $declaration_name)) {
print STDERR "${file}:$.: warning: Excess enum value '$k' description in '$declaration_name'\n";
+ ++$warnings;
}
}
}
@@ -1368,6 +1372,7 @@ sub dump_typedef($$) {

if ($identifier ne $declaration_name) {
print STDERR "${file}:$.: warning: expecting prototype for typedef $identifier. Prototype was for typedef $declaration_name instead\n";
+ ++$warnings;
return;
}

@@ -1399,6 +1404,7 @@ sub dump_typedef($$) {

if ($identifier ne $declaration_name) {
print STDERR "${file}:$.: warning: expecting prototype for typedef $identifier. Prototype was for typedef $declaration_name instead\n";
+ ++$warnings;
return;
}

@@ -1715,11 +1721,13 @@ sub dump_function($$) {
create_parameterlist($args, ',', $file, $declaration_name);
} else {
print STDERR "${file}:$.: warning: cannot understand function prototype: '$prototype'\n";
+ ++$warnings;
return;
}

if ($identifier ne $declaration_name) {
print STDERR "${file}:$.: warning: expecting prototype for $identifier(). Prototype was for $declaration_name() instead\n";
+ ++$warnings;
return;
}

@@ -1803,6 +1811,7 @@ sub tracepoint_munge($) {
if (($tracepointname eq 0) || ($tracepointargs eq 0)) {
print STDERR "${file}:$.: warning: Unrecognized tracepoint format: \n".
"$prototype\n";
+ ++$warnings;
} else {
$prototype = "static inline void trace_$tracepointname($tracepointargs)";
$identifier = "trace_$identifier";
@@ -2325,6 +2334,7 @@ sub process_file($) {
else {
print STDERR "${file}:1: warning: no structured comments found\n";
}
+ ++$warnings;
}
close IN_FILE;
}
--
2.36.0


2022-06-08 15:16:12

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] scripts: kernel-doc: Always increment warnings counter



On 6/8/22 07:26, Niklas Söderlund wrote:
> Some warnings did not increment the warnings counter making the behavior
> of running kernel-doc with -Werror unlogical as some warnings would be
> generated but not treated as errors.
>
> Fix this by always incrementing the warnings counter every time a
> warning related to the input documentation is generated. There is one
> location in get_sphinx_version() where a warning is printed and the
> counter is not touched as it concerns the execution environment of the
> kernel-doc and not the documentation being processed.
>
> Incrementing the counter only have effect when running kernel-doc in
> either verbose mode (-v or environment variable KBUILD_VERBOSE) or when
> treating warnings as errors (-Werror or environment variable
> KDOC_WERROR). In both cases the number of warnings printed is printed to
> stderr and for the later the exit code of kernel-doc is non-zero if
> warnings where encountered.
>
> Simple test case to demo one of the warnings,
>
> $ cat test.c
> /**
> * foo() - Description
> */
> int bar();
>
> # Without this change
> $ ./scripts/kernel-doc -Werror -none test.c
> test.c:4: warning: expecting prototype for foo(). Prototype was for
> bar() instead
>
> # With this change
> $ ./scripts/kernel-doc -Werror -none test.c
> test.c:4: warning: expecting prototype for foo(). Prototype was for
> bar() instead
> 1 warnings as Errors
>
> Signed-off-by: Niklas Söderlund <[email protected]>
> Signed-off-by: Simon Horman <[email protected]>
> Signed-off-by: Louis Peens <[email protected]>
> ---
> scripts/kernel-doc | 10 ++++++++++
> 1 file changed, 10 insertions(+)

LGTM. Thanks.

Reviewed-by: Randy Dunlap <[email protected]>

--
~Randy

2022-06-09 17:07:32

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] scripts: kernel-doc: Always increment warnings counter

Niklas Söderlund <[email protected]> writes:

> Some warnings did not increment the warnings counter making the behavior
> of running kernel-doc with -Werror unlogical as some warnings would be
> generated but not treated as errors.
>
> Fix this by always incrementing the warnings counter every time a
> warning related to the input documentation is generated. There is one
> location in get_sphinx_version() where a warning is printed and the
> counter is not touched as it concerns the execution environment of the
> kernel-doc and not the documentation being processed.

So this seems like an improvement, but I have to ask: wouldn't it be far
better to just add a function to emit a warning and use that rather than
all these print/++$warnings pairings? The current way seems repetitive
and error-prone.

I also have to ask...

> Incrementing the counter only have effect when running kernel-doc in
> either verbose mode (-v or environment variable KBUILD_VERBOSE) or when
> treating warnings as errors (-Werror or environment variable
> KDOC_WERROR). In both cases the number of warnings printed is printed to
> stderr and for the later the exit code of kernel-doc is non-zero if
> warnings where encountered.
>
> Simple test case to demo one of the warnings,
>
> $ cat test.c
> /**
> * foo() - Description
> */
> int bar();
>
> # Without this change
> $ ./scripts/kernel-doc -Werror -none test.c
> test.c:4: warning: expecting prototype for foo(). Prototype was for
> bar() instead
>
> # With this change
> $ ./scripts/kernel-doc -Werror -none test.c
> test.c:4: warning: expecting prototype for foo(). Prototype was for
> bar() instead
> 1 warnings as Errors
>
> Signed-off-by: Niklas Söderlund <[email protected]>
> Signed-off-by: Simon Horman <[email protected]>
> Signed-off-by: Louis Peens <[email protected]>

What does this signoff chain mean? If it really took three people to
make this patch, then we need Co-developed-by tags to reflect that.

Thanks,

jon