diff mbox series

scsi: core: Remove unused host error code strings

Message ID 20241015183948.86394-1-himanshu.madhani@oracle.com (mailing list archive)
State Changes Requested
Headers show
Series scsi: core: Remove unused host error code strings | expand

Commit Message

Himanshu Madhani Oct. 15, 2024, 6:39 p.m. UTC 
From: Himanshu Madhani <himanshu.madhani@oracle.com>

commit 68a3a9102a689 removed unused host code but
forgot to remove them from the corresponding
hostbyte_table[].

Fix it by removing unused hostcode strings and add
missing DID_TRANSPORT_MARGINAL string.

Fixes: 68a3a9102a689 ("scsi: core: Remove useless host error codes")
Signed-off-by: Himanshu Madhani <himanshu.madhani@oracle.com>
---
 drivers/scsi/constants.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Bart Van Assche Oct. 15, 2024, 7:15 p.m. UTC | #1 
On 10/15/24 11:39 AM, himanshu.madhani@oracle.com wrote:
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 340785536998..b74c3f505300 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -403,8 +403,8 @@ static const char * const hostbyte_table[]={
>   "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET",
>   "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
>   "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
> -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", "DID_TARGET_FAILURE",
> -"DID_NEXUS_FAILURE", "DID_ALLOC_FAILURE", "DID_MEDIUM_ERROR" };
> +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST",
> +"DID_TRANSPORT_MARGINAL" };

That doesn't look right. "DID_TRANSPORT_MARGINAL" occurs at the wrong
position in the array. Please use designated initializers instead of a
traditional array initialization list.

Thanks,

Bart.
Himanshu Madhani Oct. 15, 2024, 7:57 p.m. UTC | #2 
On 10/15/24 12:15, Bart Van Assche wrote:
> On 10/15/24 11:39 AM, himanshu.madhani@oracle.com wrote:
>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>> index 340785536998..b74c3f505300 100644
>> --- a/drivers/scsi/constants.c
>> +++ b/drivers/scsi/constants.c
>> @@ -403,8 +403,8 @@ static const char * const hostbyte_table[]={
>>   "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", 
>> "DID_BAD_TARGET",
>>   "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
>>   "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
>> -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", 
>> "DID_TARGET_FAILURE",
>> -"DID_NEXUS_FAILURE", "DID_ALLOC_FAILURE", "DID_MEDIUM_ERROR" };
>> +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST",
>> +"DID_TRANSPORT_MARGINAL" };
> 
> That doesn't look right. "DID_TRANSPORT_MARGINAL" occurs at the wrong
> position in the array. Please use designated initializers instead of a
> traditional array initialization list.
>

Can you elaborate? From what I see, enum scsi_host_status{ } maps to 
these strings. So, accordingly, "DID_TRANSPORT_MARGINAL" is placed after 
"DID_TRANSPORT_FAILFAST" in this array.

Am I missing something obvious?

> Thanks,
> 
> Bart.
>
Bart Van Assche Oct. 15, 2024, 8:38 p.m. UTC | #3 
On 10/15/24 12:57 PM, Himanshu Madhani wrote:
> 
> 
> On 10/15/24 12:15, Bart Van Assche wrote:
>> On 10/15/24 11:39 AM, himanshu.madhani@oracle.com wrote:
>>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>>> index 340785536998..b74c3f505300 100644
>>> --- a/drivers/scsi/constants.c
>>> +++ b/drivers/scsi/constants.c
>>> @@ -403,8 +403,8 @@ static const char * const hostbyte_table[]={
>>>   "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", 
>>> "DID_BAD_TARGET",
>>>   "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
>>>   "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
>>> -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", 
>>> "DID_TARGET_FAILURE",
>>> -"DID_NEXUS_FAILURE", "DID_ALLOC_FAILURE", "DID_MEDIUM_ERROR" };
>>> +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST",
>>> +"DID_TRANSPORT_MARGINAL" };
>>
>> That doesn't look right. "DID_TRANSPORT_MARGINAL" occurs at the wrong
>> position in the array. Please use designated initializers instead of a
>> traditional array initialization list.
>>
> 
> Can you elaborate? From what I see, enum scsi_host_status{ } maps to 
> these strings. So, accordingly, "DID_TRANSPORT_MARGINAL" is placed after 
> "DID_TRANSPORT_FAILFAST" in this array.
> 
> Am I missing something obvious?

The above patch places the "DID_TRANSPORT_MARGINAL" string at index 16.
I think that should be index 20 (0x14) instead. From scsi_status.h:

	DID_TRANSPORT_MARGINAL = 0x14, /* Transport marginal errors */

Thanks,

Bart.
Himanshu Madhani Oct. 16, 2024, 3:44 p.m. UTC | #4 
On 10/15/24 13:38, Bart Van Assche wrote:
> On 10/15/24 12:57 PM, Himanshu Madhani wrote:
>>
>>
>> On 10/15/24 12:15, Bart Van Assche wrote:
>>> On 10/15/24 11:39 AM, himanshu.madhani@oracle.com wrote:
>>>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>>>> index 340785536998..b74c3f505300 100644
>>>> --- a/drivers/scsi/constants.c
>>>> +++ b/drivers/scsi/constants.c
>>>> @@ -403,8 +403,8 @@ static const char * const hostbyte_table[]={
>>>>   "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", 
>>>> "DID_BAD_TARGET",
>>>>   "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
>>>>   "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
>>>> -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", 
>>>> "DID_TARGET_FAILURE",
>>>> -"DID_NEXUS_FAILURE", "DID_ALLOC_FAILURE", "DID_MEDIUM_ERROR" };
>>>> +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST",
>>>> +"DID_TRANSPORT_MARGINAL" };
>>>
>>> That doesn't look right. "DID_TRANSPORT_MARGINAL" occurs at the wrong
>>> position in the array. Please use designated initializers instead of a
>>> traditional array initialization list.
>>>
>>
>> Can you elaborate? From what I see, enum scsi_host_status{ } maps to 
>> these strings. So, accordingly, "DID_TRANSPORT_MARGINAL" is placed 
>> after "DID_TRANSPORT_FAILFAST" in this array.
>>
>> Am I missing something obvious?
> 
> The above patch places the "DID_TRANSPORT_MARGINAL" string at index 16.
> I think that should be index 20 (0x14) instead. From scsi_status.h:
> 
>      DID_TRANSPORT_MARGINAL = 0x14, /* Transport marginal errors */
> 

Okay. In that case, I'll drop this patch for now and will figure out 
cleanup in that area for reorganizing this table at a later time.

> Thanks,
> 
> Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 340785536998..b74c3f505300 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -403,8 +403,8 @@  static const char * const hostbyte_table[]={
 "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET",
 "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
 "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
-"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", "DID_TARGET_FAILURE",
-"DID_NEXUS_FAILURE", "DID_ALLOC_FAILURE", "DID_MEDIUM_ERROR" };
+"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST",
+"DID_TRANSPORT_MARGINAL" };
 
 const char *scsi_hostbyte_string(int result)
 {