If coccicheck finds errors, it should return an error code
distinct from zero. Current code instead of exiting with an
error code of coccinelle returns error code of
'echo "coccicheck failed"' which is almost always equals to zero,
thus failing original intention of alerting about errors.
This patch fixes the problem.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Denis Efremov <[email protected]>
---
scripts/coccicheck | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/coccicheck b/scripts/coccicheck
index 9fedca611b7f..e04d328210ac 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -128,9 +128,10 @@ run_cmd_parmap() {
fi
echo $@ >>$DEBUG_FILE
$@ 2>>$DEBUG_FILE
- if [[ $? -ne 0 ]]; then
+ err=$?
+ if [[ $err -ne 0 ]]; then
echo "coccicheck failed"
- exit $?
+ exit $err
fi
}
--
2.17.1
On Fri, 10 Aug 2018, [email protected] wrote:
> If coccicheck finds errors,
What do you mean by finds errors? Do you mean that there is an error in
the behavior of coccicheck or that coccicheck finds an error in the source
code?
To put it another way, can you give an example of the kind of error you
are concerned about?
thanks,
julia
> it should return an error code
> distinct from zero. Current code instead of exiting with an
> error code of coccinelle returns error code of
> 'echo "coccicheck failed"' which is almost always equals to zero,
> thus failing original intention of alerting about errors.
> This patch fixes the problem.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> scripts/coccicheck | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 9fedca611b7f..e04d328210ac 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -128,9 +128,10 @@ run_cmd_parmap() {
> fi
> echo $@ >>$DEBUG_FILE
> $@ 2>>$DEBUG_FILE
> - if [[ $? -ne 0 ]]; then
> + err=$?
> + if [[ $err -ne 0 ]]; then
> echo "coccicheck failed"
> - exit $?
> + exit $err
> fi
> }
>
> --
> 2.17.1
>
>
On Fri, 10 Aug 2018, Denis Efremov wrote:
> > Do you mean that there is an error in the behavior of coccicheck or that coccicheck finds an error in the source code?
>
> An error in the source code.
>
> Here is an example of how the patch changes the behavior of 'make
> coccicheck' (my comments after the ###):
> Current behavior:
> $ make M=mymodule coccicheck
> mymodule/file1.c:97:4-14: ERROR: Assignment of bool to non-0/1 constant
> mymodule/file2.c:104:4-19: ERROR: Assignment of bool to non-0/1 constant
> mymodule/file2.c:577:1-15: code aligned with following code on line 583
> mymodule/file3.c:439:5-10: Unneeded variable: "error". Return "0" on line 449
> mymodule/file4.c:451:5-7: Unneeded variable: "rc". Return "0" on line 455
> mymodule/file5.c:433:5-8: Unneeded variable: "ret". Return "0" on line 607
> mymodule/file6.c:433:5-10: Unneeded variable: "error". Return "0" on line 440
> mymodule/file7.c:774:2-3: Unneeded semicolon
> coccicheck failed ### <-- Check failed
Are you sure that this coccicheck failed has any connection to the various
messages printed above it? Normally Coccinelle has no idea about the
semantics of messages printed using python script code. I'm not sure what
would cause it to return an error code because a particular script was
activated.
julia
> $ echo $?
> 0 ### <-- But error code signals that everthing is OK
>
> After this patch:
> $ make M=mymodule coccicheck
> ...
> coccicheck failed
> make: *** [Makefile:1636: coccicheck] Error 2
> $ echo $?
> 2 ### <-- The patch changes error code
>
> Why does this matter?
> 1) Because it's clear from the source code that the original intention
> was to return an error code of checking command, not the "echo
> 'coccicheck failed'" command.
> 2) Automated testing systems (CI, for example) rely on the return code.
>
My mistake. Initially, I thought that this line signals about errors
in the code, but now I see that this is about the tool's internal
error. However, this doesn't change the fact that coccicheck returns
the improper error code.
I will reformulate the commit message and send the v2 patch with the
same diff. Thank you for clarifying things.
> Do you mean that there is an error in the behavior of coccicheck or that coccicheck finds an error in the source code?
An error in the source code.
Here is an example of how the patch changes the behavior of 'make
coccicheck' (my comments after the ###):
Current behavior:
$ make M=mymodule coccicheck
mymodule/file1.c:97:4-14: ERROR: Assignment of bool to non-0/1 constant
mymodule/file2.c:104:4-19: ERROR: Assignment of bool to non-0/1 constant
mymodule/file2.c:577:1-15: code aligned with following code on line 583
mymodule/file3.c:439:5-10: Unneeded variable: "error". Return "0" on line 449
mymodule/file4.c:451:5-7: Unneeded variable: "rc". Return "0" on line 455
mymodule/file5.c:433:5-8: Unneeded variable: "ret". Return "0" on line 607
mymodule/file6.c:433:5-10: Unneeded variable: "error". Return "0" on line 440
mymodule/file7.c:774:2-3: Unneeded semicolon
coccicheck failed ### <-- Check failed
$ echo $?
0 ### <-- But error code signals that everthing is OK
After this patch:
$ make M=mymodule coccicheck
...
coccicheck failed
make: *** [Makefile:1636: coccicheck] Error 2
$ echo $?
2 ### <-- The patch changes error code
Why does this matter?
1) Because it's clear from the source code that the original intention
was to return an error code of checking command, not the "echo
'coccicheck failed'" command.
2) Automated testing systems (CI, for example) rely on the return code.
On Fri, Aug 10, 2018 at 05:45:46PM +0300, Denis Efremov wrote:
> My mistake. Initially, I thought that this line signals about errors
> in the code, but now I see that this is about the tool's internal
> error. However, this doesn't change the fact that coccicheck returns
> the improper error code.
>
> I will reformulate the commit message and send the v2 patch with the
> same diff. Thank you for clarifying things.
I would also request to use the latest source from
https://github.com/coccinelle/coccinelle
Because some distribution supplied coccinelle packages are
obsolete and likely more prone to be disfunctional.
For instance: https://systeme.lip6.fr/pipermail/cocci/2017-December/004799.html
Thanks.
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
From: Denis Efremov <[email protected]>
If coccicheck fails, it should return an error code distinct from zero
to signal about an internal problem. Current code instead of exiting with
the tool's error code returns the error code of 'echo "coccicheck failed"'
which is almost always equals to zero, thus failing the original intention
of alerting about a problem. This patch fixes the code.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Denis Efremov <[email protected]>
---
scripts/coccicheck | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/coccicheck b/scripts/coccicheck
index 9fedca611b7f..e04d328210ac 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -128,9 +128,10 @@ run_cmd_parmap() {
fi
echo $@ >>$DEBUG_FILE
$@ 2>>$DEBUG_FILE
- if [[ $? -ne 0 ]]; then
+ err=$?
+ if [[ $err -ne 0 ]]; then
echo "coccicheck failed"
- exit $?
+ exit $err
fi
}
--
2.17.1
On Fri, 10 Aug 2018, [email protected] wrote:
> From: Denis Efremov <[email protected]>
>
> If coccicheck fails, it should return an error code distinct from zero
> to signal about an internal problem. Current code instead of exiting with
> the tool's error code returns the error code of 'echo "coccicheck failed"'
> which is almost always equals to zero, thus failing the original intention
> of alerting about a problem. This patch fixes the code.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Denis Efremov <[email protected]>
OK, I get it now. Thanks.
Acked-by: Julia Lawall <[email protected]>
> ---
> scripts/coccicheck | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 9fedca611b7f..e04d328210ac 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -128,9 +128,10 @@ run_cmd_parmap() {
> fi
> echo $@ >>$DEBUG_FILE
> $@ 2>>$DEBUG_FILE
> - if [[ $? -ne 0 ]]; then
> + err=$?
> + if [[ $err -ne 0 ]]; then
> echo "coccicheck failed"
> - exit $?
> + exit $err
> fi
> }
>
> --
> 2.17.1
>
>
2018-08-11 5:31 GMT+09:00 Julia Lawall <[email protected]>:
>
>
> On Fri, 10 Aug 2018, [email protected] wrote:
>
>> From: Denis Efremov <[email protected]>
>>
>> If coccicheck fails, it should return an error code distinct from zero
>> to signal about an internal problem. Current code instead of exiting with
>> the tool's error code returns the error code of 'echo "coccicheck failed"'
>> which is almost always equals to zero, thus failing the original intention
>> of alerting about a problem. This patch fixes the code.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Denis Efremov <[email protected]>
>
> OK, I get it now. Thanks.
>
> Acked-by: Julia Lawall <[email protected]>
>
Applied to linux-kbuild. Thanks!
>> ---
>> scripts/coccicheck | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>> index 9fedca611b7f..e04d328210ac 100755
>> --- a/scripts/coccicheck
>> +++ b/scripts/coccicheck
>> @@ -128,9 +128,10 @@ run_cmd_parmap() {
>> fi
>> echo $@ >>$DEBUG_FILE
>> $@ 2>>$DEBUG_FILE
>> - if [[ $? -ne 0 ]]; then
>> + err=$?
>> + if [[ $err -ne 0 ]]; then
>> echo "coccicheck failed"
>> - exit $?
>> + exit $err
>> fi
>> }
>>
>> --
>> 2.17.1
>>
>>
--
Best Regards
Masahiro Yamada