All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] small cleanups
@ 2021-09-29  9:17 Damien Le Moal
  2021-09-29  9:17 ` [PATCH 1/2] scsi: simplify scsi_get_vpd_page() Damien Le Moal
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Damien Le Moal @ 2021-09-29  9:17 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen

Martin,

Here is a couple of patches ifor some light cleanup in scsi_lib.
No functional changes are introduced.

Damien Le Moal (2):
  scsi: simplify scsi_get_vpd_page()
  scsi: fix scsi_mode_select() interface

 drivers/scsi/scsi.c        | 31 ++++++++++++++-----------------
 drivers/scsi/scsi_lib.c    |  8 +++-----
 drivers/scsi/sd.c          |  2 +-
 include/scsi/scsi_device.h |  5 ++---
 4 files changed, 20 insertions(+), 26 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] scsi: simplify scsi_get_vpd_page()
  2021-09-29  9:17 [PATCH 0/2] small cleanups Damien Le Moal
@ 2021-09-29  9:17 ` Damien Le Moal
  2021-09-29 18:45   ` Himanshu Madhani
  2021-09-29  9:17 ` [PATCH 2/2] scsi: fix scsi_mode_select() interface Damien Le Moal
  2022-01-10 22:04 ` [PATCH 0/2] small cleanups Martin K. Petersen
  2 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2021-09-29  9:17 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen

Remove unnecessary gotos in scsi_get_vpd_page() to improve the code
readability and use memchr() instead of an open coded search loop.
Also update the outdated kernel doc comment for this function.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/scsi.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b241f9e3885c..6be68b3427a0 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -339,47 +339,44 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
  *
  * SCSI devices may optionally supply Vital Product Data.  Each 'page'
  * of VPD is defined in the appropriate SCSI document (eg SPC, SBC).
- * If the device supports this VPD page, this routine returns a pointer
- * to a buffer containing the data from that page.  The caller is
- * responsible for calling kfree() on this pointer when it is no longer
- * needed.  If we cannot retrieve the VPD page this routine returns %NULL.
+ * If the device supports this VPD page, this routine fills @buf
+ * with the data from that page and return 0. If the VPD page is not
+ * supported or its content cannot be retrieved, -EINVAL is returned.
  */
 int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf,
 		      int buf_len)
 {
-	int i, result;
+	int result, len;
 
 	if (sdev->skip_vpd_pages)
-		goto fail;
+		return -EINVAL;
 
 	/* Ask for all the pages supported by this device */
 	result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
 	if (result < 4)
-		goto fail;
+		return -EINVAL;
 
 	/* If the user actually wanted this page, we can skip the rest */
 	if (page == 0)
 		return 0;
 
-	for (i = 4; i < min(result, buf_len); i++)
-		if (buf[i] == page)
-			goto found;
+	len = min(result, buf_len);
+	if (len > 4 && memchr(&buf[4], page, len - 4))
+		goto found;
 
-	if (i < result && i >= buf_len)
-		/* ran off the end of the buffer, give us benefit of doubt */
+	/* If we ran off the end of the buffer, give us benefit of doubt */
+	if (result > buf_len)
 		goto found;
+
 	/* The device claims it doesn't support the requested page */
-	goto fail;
+	return -EINVAL;
 
  found:
 	result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
 	if (result < 0)
-		goto fail;
+		return -EINVAL;
 
 	return 0;
-
- fail:
-	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] scsi: fix scsi_mode_select() interface
  2021-09-29  9:17 [PATCH 0/2] small cleanups Damien Le Moal
  2021-09-29  9:17 ` [PATCH 1/2] scsi: simplify scsi_get_vpd_page() Damien Le Moal
@ 2021-09-29  9:17 ` Damien Le Moal
  2021-09-29 18:46   ` Himanshu Madhani
  2022-01-10 22:04 ` [PATCH 0/2] small cleanups Martin K. Petersen
  2 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2021-09-29  9:17 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen

The modepage argument is unused. Remove it.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/scsi_lib.c    | 8 +++-----
 drivers/scsi/sd.c          | 2 +-
 include/scsi/scsi_device.h | 5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index dcf105287a76..f1fe5803d7ec 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2001,7 +2001,6 @@ void scsi_exit_queue(void)
  *	@sdev:	SCSI device to be queried
  *	@pf:	Page format bit (1 == standard, 0 == vendor specific)
  *	@sp:	Save page bit (0 == don't save, 1 == save)
- *	@modepage: mode page being requested
  *	@buffer: request buffer (may not be smaller than eight bytes)
  *	@len:	length of request buffer.
  *	@timeout: command timeout
@@ -2014,10 +2013,9 @@ void scsi_exit_queue(void)
  *	status on error
  *
  */
-int
-scsi_mode_select(struct scsi_device *sdev, int pf, int sp, int modepage,
-		 unsigned char *buffer, int len, int timeout, int retries,
-		 struct scsi_mode_data *data, struct scsi_sense_hdr *sshdr)
+int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
+		     unsigned char *buffer, int len, int timeout, int retries,
+		     struct scsi_mode_data *data, struct scsi_sense_hdr *sshdr)
 {
 	unsigned char cmd[10];
 	unsigned char *real_buffer;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 71fa70b42c2b..89b5eea0ea0c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -209,7 +209,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr,
 	 */
 	data.device_specific = 0;
 
-	if (scsi_mode_select(sdp, 1, sp, 8, buffer_data, len, SD_TIMEOUT,
+	if (scsi_mode_select(sdp, 1, sp, buffer_data, len, SD_TIMEOUT,
 			     sdkp->max_retries, &data, &sshdr)) {
 		if (scsi_sense_valid(&sshdr))
 			sd_print_sense_hdr(sdkp, &sshdr);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 09a17f6e93a7..1a9d2fe6aa02 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -415,9 +415,8 @@ extern int scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
 			   int retries, struct scsi_mode_data *data,
 			   struct scsi_sense_hdr *);
 extern int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
-			    int modepage, unsigned char *buffer, int len,
-			    int timeout, int retries,
-			    struct scsi_mode_data *data,
+			    unsigned char *buffer, int len, int timeout,
+			    int retries, struct scsi_mode_data *data,
 			    struct scsi_sense_hdr *);
 extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
 				int retries, struct scsi_sense_hdr *sshdr);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] scsi: simplify scsi_get_vpd_page()
  2021-09-29  9:17 ` [PATCH 1/2] scsi: simplify scsi_get_vpd_page() Damien Le Moal
@ 2021-09-29 18:45   ` Himanshu Madhani
  0 siblings, 0 replies; 6+ messages in thread
From: Himanshu Madhani @ 2021-09-29 18:45 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-scsi, Martin Petersen



> On Sep 29, 2021, at 4:17 AM, Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
> 
> Remove unnecessary gotos in scsi_get_vpd_page() to improve the code
> readability and use memchr() instead of an open coded search loop.
> Also update the outdated kernel doc comment for this function.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> drivers/scsi/scsi.c | 31 ++++++++++++++-----------------
> 1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index b241f9e3885c..6be68b3427a0 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -339,47 +339,44 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
>  *
>  * SCSI devices may optionally supply Vital Product Data.  Each 'page'
>  * of VPD is defined in the appropriate SCSI document (eg SPC, SBC).
> - * If the device supports this VPD page, this routine returns a pointer
> - * to a buffer containing the data from that page.  The caller is
> - * responsible for calling kfree() on this pointer when it is no longer
> - * needed.  If we cannot retrieve the VPD page this routine returns %NULL.
> + * If the device supports this VPD page, this routine fills @buf
> + * with the data from that page and return 0. If the VPD page is not
> + * supported or its content cannot be retrieved, -EINVAL is returned.
>  */
> int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf,
> 		      int buf_len)
> {
> -	int i, result;
> +	int result, len;
> 
> 	if (sdev->skip_vpd_pages)
> -		goto fail;
> +		return -EINVAL;
> 
> 	/* Ask for all the pages supported by this device */
> 	result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
> 	if (result < 4)
> -		goto fail;
> +		return -EINVAL;
> 
> 	/* If the user actually wanted this page, we can skip the rest */
> 	if (page == 0)
> 		return 0;
> 
> -	for (i = 4; i < min(result, buf_len); i++)
> -		if (buf[i] == page)
> -			goto found;
> +	len = min(result, buf_len);
> +	if (len > 4 && memchr(&buf[4], page, len - 4))
> +		goto found;
> 
> -	if (i < result && i >= buf_len)
> -		/* ran off the end of the buffer, give us benefit of doubt */
> +	/* If we ran off the end of the buffer, give us benefit of doubt */
> +	if (result > buf_len)
> 		goto found;
> +
> 	/* The device claims it doesn't support the requested page */
> -	goto fail;
> +	return -EINVAL;
> 
>  found:
> 	result = scsi_vpd_inquiry(sdev, buf, page, buf_len);
> 	if (result < 0)
> -		goto fail;
> +		return -EINVAL;
> 
> 	return 0;
> -
> - fail:
> -	return -EINVAL;
> }
> EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
> 
> -- 
> 2.31.1
> 

Looks fine. 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] scsi: fix scsi_mode_select() interface
  2021-09-29  9:17 ` [PATCH 2/2] scsi: fix scsi_mode_select() interface Damien Le Moal
@ 2021-09-29 18:46   ` Himanshu Madhani
  0 siblings, 0 replies; 6+ messages in thread
From: Himanshu Madhani @ 2021-09-29 18:46 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-scsi, Martin Petersen



> On Sep 29, 2021, at 4:17 AM, Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
> 
> The modepage argument is unused. Remove it.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> drivers/scsi/scsi_lib.c    | 8 +++-----
> drivers/scsi/sd.c          | 2 +-
> include/scsi/scsi_device.h | 5 ++---
> 3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index dcf105287a76..f1fe5803d7ec 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2001,7 +2001,6 @@ void scsi_exit_queue(void)
>  *	@sdev:	SCSI device to be queried
>  *	@pf:	Page format bit (1 == standard, 0 == vendor specific)
>  *	@sp:	Save page bit (0 == don't save, 1 == save)
> - *	@modepage: mode page being requested
>  *	@buffer: request buffer (may not be smaller than eight bytes)
>  *	@len:	length of request buffer.
>  *	@timeout: command timeout
> @@ -2014,10 +2013,9 @@ void scsi_exit_queue(void)
>  *	status on error
>  *
>  */
> -int
> -scsi_mode_select(struct scsi_device *sdev, int pf, int sp, int modepage,
> -		 unsigned char *buffer, int len, int timeout, int retries,
> -		 struct scsi_mode_data *data, struct scsi_sense_hdr *sshdr)
> +int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
> +		     unsigned char *buffer, int len, int timeout, int retries,
> +		     struct scsi_mode_data *data, struct scsi_sense_hdr *sshdr)
> {
> 	unsigned char cmd[10];
> 	unsigned char *real_buffer;
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 71fa70b42c2b..89b5eea0ea0c 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -209,7 +209,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr,
> 	 */
> 	data.device_specific = 0;
> 
> -	if (scsi_mode_select(sdp, 1, sp, 8, buffer_data, len, SD_TIMEOUT,
> +	if (scsi_mode_select(sdp, 1, sp, buffer_data, len, SD_TIMEOUT,
> 			     sdkp->max_retries, &data, &sshdr)) {
> 		if (scsi_sense_valid(&sshdr))
> 			sd_print_sense_hdr(sdkp, &sshdr);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 09a17f6e93a7..1a9d2fe6aa02 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -415,9 +415,8 @@ extern int scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
> 			   int retries, struct scsi_mode_data *data,
> 			   struct scsi_sense_hdr *);
> extern int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
> -			    int modepage, unsigned char *buffer, int len,
> -			    int timeout, int retries,
> -			    struct scsi_mode_data *data,
> +			    unsigned char *buffer, int len, int timeout,
> +			    int retries, struct scsi_mode_data *data,
> 			    struct scsi_sense_hdr *);
> extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
> 				int retries, struct scsi_sense_hdr *sshdr);
> -- 
> 2.31.1
> 

Looks Good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] small cleanups
  2021-09-29  9:17 [PATCH 0/2] small cleanups Damien Le Moal
  2021-09-29  9:17 ` [PATCH 1/2] scsi: simplify scsi_get_vpd_page() Damien Le Moal
  2021-09-29  9:17 ` [PATCH 2/2] scsi: fix scsi_mode_select() interface Damien Le Moal
@ 2022-01-10 22:04 ` Martin K. Petersen
  2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2022-01-10 22:04 UTC (permalink / raw)
  To: linux-scsi, Damien Le Moal; +Cc: Martin K . Petersen

On Wed, 29 Sep 2021 18:17:42 +0900, Damien Le Moal wrote:

> Martin,
> 
> Here is a couple of patches ifor some light cleanup in scsi_lib.
> No functional changes are introduced.
> 
> Damien Le Moal (2):
>   scsi: simplify scsi_get_vpd_page()
>   scsi: fix scsi_mode_select() interface
> 
> [...]

Applied to 5.17/scsi-queue, thanks!

[2/2] scsi: fix scsi_mode_select() interface
      https://git.kernel.org/mkp/scsi/c/81d3f500ee98

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-10 22:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  9:17 [PATCH 0/2] small cleanups Damien Le Moal
2021-09-29  9:17 ` [PATCH 1/2] scsi: simplify scsi_get_vpd_page() Damien Le Moal
2021-09-29 18:45   ` Himanshu Madhani
2021-09-29  9:17 ` [PATCH 2/2] scsi: fix scsi_mode_select() interface Damien Le Moal
2021-09-29 18:46   ` Himanshu Madhani
2022-01-10 22:04 ` [PATCH 0/2] small cleanups Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.