Re: [PATCH 2/3] Introduce enums for the SAM, message, host and driver status codes

[Bart Van Assche <bvanassche@acm.org>]

On 5/17/21 3:38 AM, John Garry wrote:
> On 15/05/2021 00:23, Bart Van Assche wrote:
>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>> index 84d73f57292b..d8774998ec6d 100644
>> --- a/drivers/scsi/constants.c
>> +++ b/drivers/scsi/constants.c
>> @@ -413,7 +413,7 @@ static const char * const driverbyte_table[]={
>>   const char *scsi_hostbyte_string(int result)
>>   {
>>       const char *hb_string = NULL;
>> -    int hb = host_byte(result);
>> +    enum host_status hb = host_byte(result);
>>   
> nit: I figure that this code had been consciously written to use
> reverse-Christmas tree style, so maybe we can maintain it

Ah, that's something I was not aware of. I will fix this.

>> --- a/drivers/target/target_core_pscsi.c
>> +++ b/drivers/target/target_core_pscsi.c
>> @@ -1044,7 +1044,7 @@ static void pscsi_req_done(struct request *req,
>> blk_status_t status)
>>       struct se_cmd *cmd = req->end_io_data;
>>       struct pscsi_plugin_task *pt = cmd->priv;
>>       int result = scsi_req(req)->result;
>> -    u8 scsi_status = status_byte(result) << 1;
>> +    enum sam_status scsi_status = status_byte(result) << 1;
> 
> Is someone going to be fixing up drivers elsewhere to use these enums?

I plan to repost the patch series that fixes up the SCSI LLDs after this
patch series has been accepted.

>> +/* Host byte codes. */
>> +enum host_status {
> 
> Just wondered is it intentional that we don't prefix "scsi_" to the enum
> name? Would it be because none of the symbols, below, don't?

I will add the prefix "scsi_" to these enumeration type names.

Thanks,

Bart.