[PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[Nathan Chancellor]

Clang + -Wimplicit-fallthrough warns:

drivers/scsi/st.c:3831:2: warning: unannotated fall-through between
switch labels [-Wimplicit-fallthrough]
        default:
        ^
drivers/scsi/st.c:3831:2: note: insert 'break;' to avoid fall-through
        default:
        ^
        break;
1 warning generated.

Clang's -Wimplicit-fallthrough is a little bit more pedantic than GCC's,
requiring every case block to end in break, return, or fallthrough,
rather than allowing implicit fallthroughs to cases that just contain
break or return. Add a break so that there is no more warning, as has
been done all over the tree already.

Fixes: 2e27f576abc6 ("scsi: scsi_ioctl: Call scsi_cmd_ioctl() from scsi_ioctl()")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/scsi/st.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 2d1b0594af69..0e36a36ed24d 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 	case CDROM_SEND_PACKET:
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
+		break;
 	default:
 		break;
 	}

base-commit: 58dd8f6e1cf8c47e81fbec9f47099772ab75278b
-- 
2.33.0

Re: [PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[Gustavo A. R. Silva]

On 8/17/21 18:55, Nathan Chancellor wrote:
> Clang + -Wimplicit-fallthrough warns:
> 
> drivers/scsi/st.c:3831:2: warning: unannotated fall-through between
> switch labels [-Wimplicit-fallthrough]
>         default:
>         ^
> drivers/scsi/st.c:3831:2: note: insert 'break;' to avoid fall-through
>         default:
>         ^
>         break;
> 1 warning generated.
> 
> Clang's -Wimplicit-fallthrough is a little bit more pedantic than GCC's,
> requiring every case block to end in break, return, or fallthrough,
> rather than allowing implicit fallthroughs to cases that just contain
> break or return. Add a break so that there is no more warning, as has
> been done all over the tree already.
> 
> Fixes: 2e27f576abc6 ("scsi: scsi_ioctl: Call scsi_cmd_ioctl() from scsi_ioctl()")

I don't think this tag is needed for these patches.

> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

I also got the warnings in staging and ntfs3, I have the fixes for those in my
local tree and I will commit them to my tree, soon.

Thanks
--
Gustavo

> ---
>  drivers/scsi/st.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 2d1b0594af69..0e36a36ed24d 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  	case CDROM_SEND_PACKET:
>  		if (!capable(CAP_SYS_RAWIO))
>  			return -EPERM;
> +		break;
>  	default:
>  		break;
>  	}
> 
> base-commit: 58dd8f6e1cf8c47e81fbec9f47099772ab75278b
> 

Re: [PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[Finn Thain]

On Tue, 17 Aug 2021, Nathan Chancellor wrote:

> Clang + -Wimplicit-fallthrough warns:
> 
> drivers/scsi/st.c:3831:2: warning: unannotated fall-through between
> switch labels [-Wimplicit-fallthrough]
>         default:
>         ^
> drivers/scsi/st.c:3831:2: note: insert 'break;' to avoid fall-through
>         default:
>         ^
>         break;
> 1 warning generated.
> 
> Clang's -Wimplicit-fallthrough is a little bit more pedantic than GCC's,
> requiring every case block to end in break, return, or fallthrough,
> rather than allowing implicit fallthroughs to cases that just contain
> break or return. Add a break so that there is no more warning, as has
> been done all over the tree already.
> 
> Fixes: 2e27f576abc6 ("scsi: scsi_ioctl: Call scsi_cmd_ioctl() from scsi_ioctl()")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/scsi/st.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 2d1b0594af69..0e36a36ed24d 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>  	case CDROM_SEND_PACKET:
>  		if (!capable(CAP_SYS_RAWIO))
>  			return -EPERM;
> +		break;
>  	default:
>  		break;
>  	}
> 
> base-commit: 58dd8f6e1cf8c47e81fbec9f47099772ab75278b
> 

Well, that sure is ugly.

Do you think the following change would cause any static checkers to spit 
their dummys and throw their toys out of the pram?

@@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 	case CDROM_SEND_PACKET:
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
+		break;
-	default:
-		break;
 	}
 	

Re: [PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[Nathan Chancellor]

On 8/17/2021 5:54 PM, Finn Thain wrote:
> On Tue, 17 Aug 2021, Nathan Chancellor wrote:
> 
>> Clang + -Wimplicit-fallthrough warns:
>>
>> drivers/scsi/st.c:3831:2: warning: unannotated fall-through between
>> switch labels [-Wimplicit-fallthrough]
>>          default:
>>          ^
>> drivers/scsi/st.c:3831:2: note: insert 'break;' to avoid fall-through
>>          default:
>>          ^
>>          break;
>> 1 warning generated.
>>
>> Clang's -Wimplicit-fallthrough is a little bit more pedantic than GCC's,
>> requiring every case block to end in break, return, or fallthrough,
>> rather than allowing implicit fallthroughs to cases that just contain
>> break or return. Add a break so that there is no more warning, as has
>> been done all over the tree already.
>>
>> Fixes: 2e27f576abc6 ("scsi: scsi_ioctl: Call scsi_cmd_ioctl() from scsi_ioctl()")
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> ---
>>   drivers/scsi/st.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
>> index 2d1b0594af69..0e36a36ed24d 100644
>> --- a/drivers/scsi/st.c
>> +++ b/drivers/scsi/st.c
>> @@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>>   	case CDROM_SEND_PACKET:
>>   		if (!capable(CAP_SYS_RAWIO))
>>   			return -EPERM;
>> +		break;
>>   	default:
>>   		break;
>>   	}
>>
>> base-commit: 58dd8f6e1cf8c47e81fbec9f47099772ab75278b
>>
> 
> Well, that sure is ugly.
> 
> Do you think the following change would cause any static checkers to spit
> their dummys and throw their toys out of the pram?
> 
> @@ -3828,6 +3828,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>   	case CDROM_SEND_PACKET:
>   		if (!capable(CAP_SYS_RAWIO))
>   			return -EPERM;
> +		break;
> -	default:
> -		break;
>   	}
>   	

I cannot speak for other static checkers but clang does not complain in 
this instance. cmd_in is the switch value, which is unsigned int; as far 
as I am aware, clang will only complain about a switch not handling all 
values when switching on an enumerated type.

Gustavo, if you are already handling all of the other warnings in -next, 
do you want to take this one too?

Cheers,
Nathan

Re: [PATCH] scsi: st: Add missing break in switch statement in st_ioctl()

[Martin K. Petersen]

On Tue, 17 Aug 2021 16:55:31 -0700, Nathan Chancellor wrote:

> Clang + -Wimplicit-fallthrough warns:
> 
> drivers/scsi/st.c:3831:2: warning: unannotated fall-through between
> switch labels [-Wimplicit-fallthrough]
>         default:
>         ^
> drivers/scsi/st.c:3831:2: note: insert 'break;' to avoid fall-through
>         default:
>         ^
>         break;
> 1 warning generated.
> 
> [...]

Applied to 5.15/scsi-fixes, thanks!

[1/1] scsi: st: Add missing break in switch statement in st_ioctl()
      https://git.kernel.org/mkp/scsi/c/6a2ea0d34af1

-- 
Martin K. Petersen	Oracle Linux Engineering