[PATCH] cmd: env: fix unreachable statements
C's switch statement takes an integer value for switching. As efi_status_t is defined as unsigned long and each error code has the top bit set, all "cases" cannot be reachable.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reported-by: Coverity (CID 300335) --- cmd/nvedit_efi.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c index 837e39e02179..84cba0c7324b 100644 --- a/cmd/nvedit_efi.c +++ b/cmd/nvedit_efi.c @@ -597,26 +597,18 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else { const char *msg;
- switch (ret) { - case EFI_NOT_FOUND: + if (ret == EFI_NOT_FOUND) msg = " (not found)"; - break; - case EFI_WRITE_PROTECTED: + else if (ret == EFI_WRITE_PROTECTED) msg = " (read only)"; - break; - case EFI_INVALID_PARAMETER: + else if (ret == EFI_INVALID_PARAMETER) msg = " (invalid parameter)"; - break; - case EFI_SECURITY_VIOLATION: + else if (ret == EFI_SECURITY_VIOLATION) msg = " (validation failed)"; - break; - case EFI_OUT_OF_RESOURCES: + else if (ret == EFI_OUT_OF_RESOURCES) msg = " (out of memory)"; - break; - default: + else msg = ""; - break; - } printf("## Failed to set EFI variable%s\n", msg); ret = CMD_RET_FAILURE; }
On 08.05.20 07:51, AKASHI Takahiro wrote:
C's switch statement takes an integer value for switching. As efi_status_t is defined as unsigned long and each error code has the top bit set, all "cases" cannot be reachable.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reported-by: Coverity (CID 300335)
The requirement of C 1999 specification is: "The controlling expression of a switch statement shall have integer type." The requirement is not that the controlling expression should be int.
GCC works fine with uint64_t as argument of a switch statement.
To me this is a false positive of Coverity.
Best regards
Heinrich
cmd/nvedit_efi.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c index 837e39e02179..84cba0c7324b 100644 --- a/cmd/nvedit_efi.c +++ b/cmd/nvedit_efi.c @@ -597,26 +597,18 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else { const char *msg;
switch (ret) {case EFI_NOT_FOUND:
if (ret == EFI_NOT_FOUND) msg = " (not found)";
break;case EFI_WRITE_PROTECTED:
else if (ret == EFI_WRITE_PROTECTED) msg = " (read only)";
break;case EFI_INVALID_PARAMETER:
else if (ret == EFI_INVALID_PARAMETER) msg = " (invalid parameter)";
break;case EFI_SECURITY_VIOLATION:
else if (ret == EFI_SECURITY_VIOLATION) msg = " (validation failed)";
break;case EFI_OUT_OF_RESOURCES:
else if (ret == EFI_OUT_OF_RESOURCES) msg = " (out of memory)";
break;default:
else msg = "";
break;}
On Fri, May 08, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
On 08.05.20 07:51, AKASHI Takahiro wrote:
C's switch statement takes an integer value for switching. As efi_status_t is defined as unsigned long and each error code has the top bit set, all "cases" cannot be reachable.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reported-by: Coverity (CID 300335)
The requirement of C 1999 specification is: "The controlling expression of a switch statement shall have integer type." The requirement is not that the controlling expression should be int.
GCC works fine with uint64_t as argument of a switch statement.
OK, but for the record what about C11 with GNU extensions?
To me this is a false positive of Coverity.
I can go mark it as such after the above is answered, thanks!
On 5/8/20 6:19 PM, Tom Rini wrote:
On Fri, May 08, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
On 08.05.20 07:51, AKASHI Takahiro wrote:
C's switch statement takes an integer value for switching. As efi_status_t is defined as unsigned long and each error code has the top bit set, all "cases" cannot be reachable.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reported-by: Coverity (CID 300335)
The requirement of C 1999 specification is: "The controlling expression of a switch statement shall have integer type." The requirement is not that the controlling expression should be int.
GCC works fine with uint64_t as argument of a switch statement.
OK, but for the record what about C11 with GNU extensions?
C11: "The controlling expression of a switchstatement shall have integer type." (https://web.cs.dal.ca/~vlado/pl/C_Standard_2011-n1570.pdf)
Why did you expect anything that is incompatible to C99?
Best regards
Heinrich
To me this is a false positive of Coverity.
I can go mark it as such after the above is answered, thanks!
On Fri, May 08, 2020 at 07:27:05PM +0200, Heinrich Schuchardt wrote:
On 5/8/20 6:19 PM, Tom Rini wrote:
On Fri, May 08, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
On 08.05.20 07:51, AKASHI Takahiro wrote:
C's switch statement takes an integer value for switching. As efi_status_t is defined as unsigned long and each error code has the top bit set, all "cases" cannot be reachable.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reported-by: Coverity (CID 300335)
The requirement of C 1999 specification is: "The controlling expression of a switch statement shall have integer type." The requirement is not that the controlling expression should be int.
GCC works fine with uint64_t as argument of a switch statement.
OK, but for the record what about C11 with GNU extensions?
C11: "The controlling expression of a switchstatement shall have integer type." (https://web.cs.dal.ca/~vlado/pl/C_Standard_2011-n1570.pdf)
Why did you expect anything that is incompatible to C99?
I didn't really, I just wanted a reference to C11. It's odd I think Coverity hit this as a false positive, but it's just a tool. Thanks!
participants (3)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Tom Rini