On 9/22/19 3:38 AM, Max Gurtovoy wrote: > > On 9/22/2019 2:29 AM, Jens Axboe wrote: >> On 9/21/19 4:54 PM, Martin K. Petersen wrote: >>> Jens, >>> >>>>> block/t10-pi.c: In function 't10_pi_verify': >>>>> block/t10-pi.c:62:3: warning: enumeration value 'T10_PI_TYPE0_PROTECTION' >>>>> not handled in switch [-Wswitch] >>>>> switch (type) { >>>>> ^~~~~~ >>>> This commit message is woefully lacking. It doesn't explain >>>> anything...? Why aren't we just flagging this as an error? Seems a >>>> lot saner than adding a BUG(). >>> The fundamental issue is that T10_PI_TYPE0_PROTECTION means "no attached >>> protection information". So it's a block layer bug if we ever end up in >>> this function and the protection type is 0. >>> >>> My main beef with all this is that I don't particularly like introducing >>> a nonsensical switch case to quiesce a compiler warning. We never call >>> t10_pi_verify() with a type of 0 and there are lots of safeguards >>> further up the stack preventing that from ever happening. Adding a Type >>> 0 here gives the reader the false impression that it's valid input to >>> the function. Which it really isn't. >>> >>> Arnd: Any ideas how to handle this? >> Why not just add the default catch, that logs, and returns the error? >> Would seem like the cleanest way to handle it to me. Since the >> compiler knows the type, it'll complain if we have missing cases. > > what about removing the switch/case and do the following change:
It's effectively the same thing, I really don't think we need (or should have) a BUG/BUG_ON for this condition. Just return an error? Just include a T10_PI_TYPE0_PROTECTION case in the switch, have it log and return an error. Add a comment on how it's impossible, if need be. I don't think it has to be more complicated than that. -- Jens Axboe