| Message ID | 20200508055145.7621-1-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | cmd: env: fix unreachable statements | expand |
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 at 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; > - } > printf("## Failed to set EFI variable%s\n", msg); > ret = CMD_RET_FAILURE; > } >
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 at 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 at 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 at 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!
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; }
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 at linaro.org> Reported-by: Coverity (CID 300335) --- cmd/nvedit_efi.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-)