Skip to contentSkip to conversation

coccicheck: return proper error code on check fail

PATCH Superseded [email protected] 4 participants 9 messages Aug 10, 2018 – Aug 13, 2018 Coccinelle/Semantic Patches (SmPL)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 coccicheck: return proper error code on check fail

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).

---
 scripts/coccicheck | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/coccicheck +3 -2
index 9fedca611b7f..e04d328210ac 100755
@@ -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
 }
 
Signature · 1 line
2.17.1
#1 lore raw
Quoted 1 line · 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

Quoted 35 lines

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).

---
 scripts/coccicheck | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/coccicheck +3 -3
index 9fedca611b7f..e04d328210ac 100755
@@ -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

#2 lore raw
Quoted 1 line

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.

#3 lore raw
Quoted 7 lines · Denis Efremov
Quoted 1 line

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:

Quoted 10 lines
$ 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

Quoted 17 lines
$ 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.

#4 lore raw

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.

#5 lore raw
Quoted 7 lines · Denis Efremov

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.

Signature · 4 lines
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
#6 lore raw

PATCH v2 1/1 coccicheck: return proper error code on fail

In reply to [email protected]

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).

---
 scripts/coccicheck | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/coccicheck +3 -2
index 9fedca611b7f..e04d328210ac 100755
@@ -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
 }
 
Signature · 1 line
2.17.1
Acked-by on v2 1/1
#7 lore raw

Re: [PATCH v2] coccicheck: return proper error code on fail

Quoted 11 lines · 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).

OK, I get it now. Thanks.

Quoted 25 lines
---
 scripts/coccicheck | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/coccicheck +3 -3
index 9fedca611b7f..e04d328210ac 100755
@@ -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

#8 lore raw

Re: [PATCH v2] coccicheck: return proper error code on fail

2018-08-11 5:31 GMT+09:00 Julia Lawall <[email protected]>:

Quoted 12 lines · On Fri, 10 Aug 2018, [email protected] wrote:
Quoted 9 lines

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.

Quoted 8 lines
Quoted 3 lines

Found by Linux Driver Verification project (linuxtesting.org).

OK, I get it now. Thanks.

Applied to linux-kbuild. Thanks!

Quoted 25 lines
Quoted 25 lines
---
 scripts/coccicheck | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/coccicheck +3 -3
index 9fedca611b7f..e04d328210ac 100755
@@ -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

Signature · 2 lines
Best Regards
Masahiro Yamada