linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block: t10-pi: fix -Wswitch warning
@ 2019-09-19 13:57 Arnd Bergmann
  2019-09-19 16:21 ` Nick Desaulniers
  2019-09-20  6:05 ` Nathan Chancellor
  0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2019-09-19 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Arnd Bergmann, Christoph Hellwig, Martin K. Petersen,
	Max Gurtovoy, Chaitanya Kulkarni, linux-block, linux-kernel,
	clang-built-linux

Changing the switch() statement to symbolic constants made
the compiler (at least clang-9, did not check gcc) notice that
there is one enum value that is not handled here:

block/t10-pi.c:62:11: error: enumeration value 'T10_PI_TYPE0_PROTECTION' not handled in switch [-Werror,-Wswitch]

Add another case for the missing value and do nothing there
based on the assumption that the code was working correctly
already.

Fixes: 9b2061b1a262 ("block: use symbolic constants for t10_pi type")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 block/t10-pi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/t10-pi.c b/block/t10-pi.c
index 0c0120a672f9..055fac923946 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -60,6 +60,8 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
 		__be16 csum;
 
 		switch (type) {
+		case T10_PI_TYPE0_PROTECTION:
+			break;
 		case T10_PI_TYPE1_PROTECTION:
 		case T10_PI_TYPE2_PROTECTION:
 			if (pi->app_tag == T10_PI_APP_ESCAPE)
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] block: t10-pi: fix -Wswitch warning
  2019-09-19 13:57 [PATCH] block: t10-pi: fix -Wswitch warning Arnd Bergmann
@ 2019-09-19 16:21 ` Nick Desaulniers
  2019-09-20  6:05 ` Nathan Chancellor
  1 sibling, 0 replies; 4+ messages in thread
From: Nick Desaulniers @ 2019-09-19 16:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jens Axboe, Christoph Hellwig, Martin K. Petersen, Max Gurtovoy,
	Chaitanya Kulkarni, linux-block, LKML, clang-built-linux,
	Nathan Chancellor, kbuild-all

On Thu, Sep 19, 2019 at 6:57 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Changing the switch() statement to symbolic constants made
> the compiler (at least clang-9, did not check gcc) notice that
> there is one enum value that is not handled here:
>
> block/t10-pi.c:62:11: error: enumeration value 'T10_PI_TYPE0_PROTECTION' not handled in switch [-Werror,-Wswitch]
>
> Add another case for the missing value and do nothing there
> based on the assumption that the code was working correctly
> already.
>
> Fixes: 9b2061b1a262 ("block: use symbolic constants for t10_pi type")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reported-by: kbuild test robot <lkp@intel.com>
https://groups.google.com/forum/#!topic/clang-built-linux/awgY7hmSCCM
Hard to say what's the right thing to do here, there's not a lot of
other switches on this variable.  That enum value barely even shows up
in the kernel.  Since this is no functional change:
Acked-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for sending the patch.

> ---
>  block/t10-pi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/block/t10-pi.c b/block/t10-pi.c
> index 0c0120a672f9..055fac923946 100644
> --- a/block/t10-pi.c
> +++ b/block/t10-pi.c
> @@ -60,6 +60,8 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
>                 __be16 csum;
>
>                 switch (type) {
> +               case T10_PI_TYPE0_PROTECTION:
> +                       break;
>                 case T10_PI_TYPE1_PROTECTION:
>                 case T10_PI_TYPE2_PROTECTION:
>                         if (pi->app_tag == T10_PI_APP_ESCAPE)
> --
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] block: t10-pi: fix -Wswitch warning
  2019-09-19 13:57 [PATCH] block: t10-pi: fix -Wswitch warning Arnd Bergmann
  2019-09-19 16:21 ` Nick Desaulniers
@ 2019-09-20  6:05 ` Nathan Chancellor
  2019-09-21 22:05   ` Max Gurtovoy
  1 sibling, 1 reply; 4+ messages in thread
From: Nathan Chancellor @ 2019-09-20  6:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jens Axboe, Christoph Hellwig, Martin K. Petersen, Max Gurtovoy,
	Chaitanya Kulkarni, linux-block, linux-kernel, clang-built-linux

On Thu, Sep 19, 2019 at 03:57:19PM +0200, Arnd Bergmann wrote:
> Changing the switch() statement to symbolic constants made
> the compiler (at least clang-9, did not check gcc) notice that
> there is one enum value that is not handled here:
> 
> block/t10-pi.c:62:11: error: enumeration value 'T10_PI_TYPE0_PROTECTION' not handled in switch [-Werror,-Wswitch]
> 
> Add another case for the missing value and do nothing there
> based on the assumption that the code was working correctly
> already.
> 
> Fixes: 9b2061b1a262 ("block: use symbolic constants for t10_pi type")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  block/t10-pi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/t10-pi.c b/block/t10-pi.c
> index 0c0120a672f9..055fac923946 100644
> --- a/block/t10-pi.c
> +++ b/block/t10-pi.c
> @@ -60,6 +60,8 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
>  		__be16 csum;
>  
>  		switch (type) {
> +		case T10_PI_TYPE0_PROTECTION:
> +			break;
>  		case T10_PI_TYPE1_PROTECTION:
>  		case T10_PI_TYPE2_PROTECTION:
>  			if (pi->app_tag == T10_PI_APP_ESCAPE)
> -- 
> 2.20.0

I didn't have the break in my local patch but I think this is more
correct based on the description of the enums. Like Nick pointed out,
there is no functional change because this value is not used in this
file.

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] block: t10-pi: fix -Wswitch warning
  2019-09-20  6:05 ` Nathan Chancellor
@ 2019-09-21 22:05   ` Max Gurtovoy
  0 siblings, 0 replies; 4+ messages in thread
From: Max Gurtovoy @ 2019-09-21 22:05 UTC (permalink / raw)
  To: Nathan Chancellor, Arnd Bergmann
  Cc: Jens Axboe, Christoph Hellwig, Martin K. Petersen,
	Chaitanya Kulkarni, linux-block, linux-kernel, clang-built-linux


On 9/20/2019 9:05 AM, Nathan Chancellor wrote:
> On Thu, Sep 19, 2019 at 03:57:19PM +0200, Arnd Bergmann wrote:
>> Changing the switch() statement to symbolic constants made
>> the compiler (at least clang-9, did not check gcc) notice that
>> there is one enum value that is not handled here:
>>
>> block/t10-pi.c:62:11: error: enumeration value 'T10_PI_TYPE0_PROTECTION' not handled in switch [-Werror,-Wswitch]
>>
>> Add another case for the missing value and do nothing there
>> based on the assumption that the code was working correctly
>> already.
>>
>> Fixes: 9b2061b1a262 ("block: use symbolic constants for t10_pi type")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   block/t10-pi.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/block/t10-pi.c b/block/t10-pi.c
>> index 0c0120a672f9..055fac923946 100644
>> --- a/block/t10-pi.c
>> +++ b/block/t10-pi.c
>> @@ -60,6 +60,8 @@ static blk_status_t t10_pi_verify(struct blk_integrity_iter *iter,
>>   		__be16 csum;
>>   
>>   		switch (type) {
>> +		case T10_PI_TYPE0_PROTECTION:
>> +			break;
>>   		case T10_PI_TYPE1_PROTECTION:
>>   		case T10_PI_TYPE2_PROTECTION:
>>   			if (pi->app_tag == T10_PI_APP_ESCAPE)
>> -- 
>> 2.20.0
> I didn't have the break in my local patch but I think this is more
> correct based on the description of the enums. Like Nick pointed out,
> there is no functional change because this value is not used in this
> file.
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

Hi,

we had a thread with Martin regarding this issue and we decided to set a 
default clause and add a BUG() in case we get there.

I've sent a patch with this fix.

-Max.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-09-21 22:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 13:57 [PATCH] block: t10-pi: fix -Wswitch warning Arnd Bergmann
2019-09-19 16:21 ` Nick Desaulniers
2019-09-20  6:05 ` Nathan Chancellor
2019-09-21 22:05   ` Max Gurtovoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).