Re: [PATCH v3 03/19] scsi: Move sd_pr_type to header to share

Bart Van Assche
2022-11-03T18:14:31Z

On 11/2/22 19:13, Mike Christie wrote:
> On 11/2/22 5:47 PM, Bart Van Assche wrote:
>> On 10/26/22 16:19, Mike Christie wrote:
>>> +static inline enum scsi_pr_type block_pr_type_to_scsi(enum pr_type type)
>>> +{
>>> +    switch (type) {
>>> +    case PR_WRITE_EXCLUSIVE:
>>> +        return SCSI_PR_WRITE_EXCLUSIVE;
>>> +    case PR_EXCLUSIVE_ACCESS:
>>> +        return SCSI_PR_EXCLUSIVE_ACCESS;
>>> +    case PR_WRITE_EXCLUSIVE_REG_ONLY:
>>> +        return SCSI_PR_WRITE_EXCLUSIVE_REG_ONLY;
>>> +    case PR_EXCLUSIVE_ACCESS_REG_ONLY:
>>> +        return SCSI_PR_EXCLUSIVE_ACCESS_REG_ONLY;
>>> +    case PR_WRITE_EXCLUSIVE_ALL_REGS:
>>> +        return SCSI_PR_WRITE_EXCLUSIVE_ALL_REGS;
>>> +    case PR_EXCLUSIVE_ACCESS_ALL_REGS:
>>> +        return SCSI_PR_EXCLUSIVE_ACCESS_ALL_REGS;
>>> +    default:
>>> +        return 0;
>>> +    }
>>> +};
>>
>> Please leave out "default: return 0;" from the switch statement and add "return 0;" after the switch statement. That will make the compiler emit a warning if a value is added in enum pr_type but not in the above function.
>
> Did you want that compiler warning functionality in the future code
> or are you asking me to do the above only if we go the switch based
> route?
>
> Chaitanya requested me to make this array based in nvme/scsi. For this
> approach, I can add a WARN or printk warning if the pr_type passed in
> does not match a value in the array. I don't think I can do a compiler
> warning though.
>
> I didn't care, but I think the compiler warning route might be better
> though.

Hi Mike,

Although I do not have a strong opinion about this, keeping the switch
statement and moving the return statement out of the switch statement
has the advantage that the compiler will warn if the switch statement is
incomplete. I don't think that a similar warning would be emitted if the
switch statement would be converted into an array lookup.

Thanks,

Bart.