[PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code

Qianfeng Rong posted 6 patches 1 month, 2 weeks ago
[PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code
Posted by Qianfeng Rong 1 month, 2 weeks ago
Use min()/min_t() to reduce the code in complete_scsi_command() and
hpsa_vpd_page_supported(), and improve readability.

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
 drivers/scsi/hpsa.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c73a71ac3c29..95dfcbac997f 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2662,10 +2662,8 @@ static void complete_scsi_command(struct CommandList *cp)
 	case CMD_TARGET_STATUS:
 		cmd->result |= ei->ScsiStatus;
 		/* copy the sense data */
-		if (SCSI_SENSE_BUFFERSIZE < sizeof(ei->SenseInfo))
-			sense_data_size = SCSI_SENSE_BUFFERSIZE;
-		else
-			sense_data_size = sizeof(ei->SenseInfo);
+		sense_data_size = min_t(unsigned long, SCSI_SENSE_BUFFERSIZE,
+					sizeof(ei->SenseInfo));
 		if (ei->SenseLen < sense_data_size)
 			sense_data_size = ei->SenseLen;
 		memcpy(cmd->sense_buffer, ei->SenseInfo, sense_data_size);
@@ -3628,10 +3626,7 @@ static bool hpsa_vpd_page_supported(struct ctlr_info *h,
 	if (rc != 0)
 		goto exit_unsupported;
 	pages = buf[3];
-	if ((pages + HPSA_VPD_HEADER_SZ) <= 255)
-		bufsize = pages + HPSA_VPD_HEADER_SZ;
-	else
-		bufsize = 255;
+	bufsize = min(pages + HPSA_VPD_HEADER_SZ, 255);
 
 	/* Get the whole VPD page list */
 	rc = hpsa_scsi_do_inquiry(h, scsi3addr,
-- 
2.34.1
Re: [PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code
Posted by David Laight 1 month, 2 weeks ago
On Fri, 15 Aug 2025 20:16:04 +0800
Qianfeng Rong <rongqianfeng@vivo.com> wrote:

> Use min()/min_t() to reduce the code in complete_scsi_command() and
> hpsa_vpd_page_supported(), and improve readability.
> 
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> ---
>  drivers/scsi/hpsa.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index c73a71ac3c29..95dfcbac997f 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -2662,10 +2662,8 @@ static void complete_scsi_command(struct CommandList *cp)
>  	case CMD_TARGET_STATUS:
>  		cmd->result |= ei->ScsiStatus;
>  		/* copy the sense data */
> -		if (SCSI_SENSE_BUFFERSIZE < sizeof(ei->SenseInfo))
> -			sense_data_size = SCSI_SENSE_BUFFERSIZE;
> -		else
> -			sense_data_size = sizeof(ei->SenseInfo);
> +		sense_data_size = min_t(unsigned long, SCSI_SENSE_BUFFERSIZE,
> +					sizeof(ei->SenseInfo));

Why min_t() ?
A plain min() should be fine.
If it isn't you should really need to justify why the type of one parameter
can't be changes before using min_t().

	David

>  		if (ei->SenseLen < sense_data_size)
>  			sense_data_size = ei->SenseLen;
>  		memcpy(cmd->sense_buffer, ei->SenseInfo, sense_data_size);
> @@ -3628,10 +3626,7 @@ static bool hpsa_vpd_page_supported(struct ctlr_info *h,
>  	if (rc != 0)
>  		goto exit_unsupported;
>  	pages = buf[3];
> -	if ((pages + HPSA_VPD_HEADER_SZ) <= 255)
> -		bufsize = pages + HPSA_VPD_HEADER_SZ;
> -	else
> -		bufsize = 255;
> +	bufsize = min(pages + HPSA_VPD_HEADER_SZ, 255);
>  
>  	/* Get the whole VPD page list */
>  	rc = hpsa_scsi_do_inquiry(h, scsi3addr,
Re: [PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code
Posted by Qianfeng Rong 1 month, 2 weeks ago
在 2025/8/20 20:02, David Laight 写道:
> [You don't often get email from david.laight.linux@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Fri, 15 Aug 2025 20:16:04 +0800
> Qianfeng Rong <rongqianfeng@vivo.com> wrote:
>
>> Use min()/min_t() to reduce the code in complete_scsi_command() and
>> hpsa_vpd_page_supported(), and improve readability.
>>
>> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
>> ---
>>   drivers/scsi/hpsa.c | 11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index c73a71ac3c29..95dfcbac997f 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -2662,10 +2662,8 @@ static void complete_scsi_command(struct CommandList *cp)
>>        case CMD_TARGET_STATUS:
>>                cmd->result |= ei->ScsiStatus;
>>                /* copy the sense data */
>> -             if (SCSI_SENSE_BUFFERSIZE < sizeof(ei->SenseInfo))
>> -                     sense_data_size = SCSI_SENSE_BUFFERSIZE;
>> -             else
>> -                     sense_data_size = sizeof(ei->SenseInfo);
>> +             sense_data_size = min_t(unsigned long, SCSI_SENSE_BUFFERSIZE,
>> +                                     sizeof(ei->SenseInfo));
> Why min_t() ?
> A plain min() should be fine.
> If it isn't you should really need to justify why the type of one parameter
> can't be changes before using min_t().
SCSI_SENSE_BUFFERSIZE is a macro definition and is generally of type int.
The return type of sizeof(ei->SenseInfo) is size_t, so I used min_t()
here.  However, as you mentioned, min() can also be used.  Do I need to
send v2?

Best regards,
Qianfeng
Re: [PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code
Posted by David Laight 1 month, 2 weeks ago
On Wed, 20 Aug 2025 20:50:08 +0800
Qianfeng Rong <rongqianfeng@vivo.com> wrote:

> 在 2025/8/20 20:02, David Laight 写道:
> > [You don't often get email from david.laight.linux@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Fri, 15 Aug 2025 20:16:04 +0800
> > Qianfeng Rong <rongqianfeng@vivo.com> wrote:
> >  
> >> Use min()/min_t() to reduce the code in complete_scsi_command() and
> >> hpsa_vpd_page_supported(), and improve readability.
> >>
> >> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> >> ---
> >>   drivers/scsi/hpsa.c | 11 +++--------
> >>   1 file changed, 3 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> >> index c73a71ac3c29..95dfcbac997f 100644
> >> --- a/drivers/scsi/hpsa.c
> >> +++ b/drivers/scsi/hpsa.c
> >> @@ -2662,10 +2662,8 @@ static void complete_scsi_command(struct CommandList *cp)
> >>        case CMD_TARGET_STATUS:
> >>                cmd->result |= ei->ScsiStatus;
> >>                /* copy the sense data */
> >> -             if (SCSI_SENSE_BUFFERSIZE < sizeof(ei->SenseInfo))
> >> -                     sense_data_size = SCSI_SENSE_BUFFERSIZE;
> >> -             else
> >> -                     sense_data_size = sizeof(ei->SenseInfo);
> >> +             sense_data_size = min_t(unsigned long, SCSI_SENSE_BUFFERSIZE,
> >> +                                     sizeof(ei->SenseInfo));  
> > Why min_t() ?
> > A plain min() should be fine.
> > If it isn't you should really need to justify why the type of one parameter
> > can't be changes before using min_t().

> SCSI_SENSE_BUFFERSIZE is a macro definition and is generally of type int.
> The return type of sizeof(ei->SenseInfo) is size_t, so I used min_t()
> here.  However, as you mentioned, min() can also be used.  Do I need to
> send v2?

The thing to remember is that min_t(type, a, b) is just min((type)a, (type)b))
and you really would never write the latter.

	David

> 
> Best regards,
> Qianfeng
Re: [PATCH 2/6] scsi: hpsa: use min()/min_t() to improve code
Posted by Martin K. Petersen 1 month, 2 weeks ago
Qianfeng,

> Use min()/min_t() to reduce the code in complete_scsi_command() and
> hpsa_vpd_page_supported(), and improve readability.

Applied to 6.18/scsi-staging, thanks!

-- 
Martin K. Petersen