On 9/22/19 3:21 PM, Max Gurtovoy wrote: > > On 9/22/2019 8:31 PM, Martin K. Petersen wrote: >> Jens, >> >>> 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. >> The additional case statement is inside an iterator loop which would >> bomb for Type 0 since there is no protection buffer to iterate >> over. We'd presumably never reach that default: case before >> dereferencing something bad. >> >> t10_pi_verify() is a static function exclusively called by helpers that >> pass in either 1 or 3 as argument. It seems kind of silly that we have >> to jump through hoops to silence a compiler warning for this. I would >> prefer a BUILD_BUG_ON(type == T10_PI_TYPE0_PROTECTION) at the top of the >> function but that does not satisfy the -Wswitch logic either. >> >> Anyway. Enough energy wasted on this. I'm OK with either the default: >> case or Max' if statement approach. My objection is purely >> wrt. introducing semantically incorrect and/or unreachable code to >> silence compiler warnings. Seems backwards. > > I agree that enough energy wasted here :) Agree ;-) > Attached some proposal to fix this warning. > > Let me know if you want me to send it to the mailing list OK, fine with me, I've queued it up. -- Jens Axboe
