Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>,
	James Bottomley <james.bottomley@hansenpartnership.com>,
	Bart van Assche <bvanassche@acm.org>,
	linux-scsi@vger.kernel.org, Hannes Reinecke <hare@suse.de>
Subject: [PATCH 03/40] scsi: Fixup calling convention for scsi_mode_sense()
Date: Tue, 27 Apr 2021 10:30:09 +0200
Message-ID: <20210427083046.31620-4-hare@suse.de> (raw)
In-Reply-To: <20210427083046.31620-1-hare@suse.de>

The description for scsi_mode_sense() claims to return the number
of valid bytes on success, which is not what the code does.
Additionally there is no gain in returning the scsi status, as
everything the callers do is to check against scsi_result_is_good(),
which is what scsi_mode_sense() does already.
So change the calling convention to return a standard error code
on failure, and 0 on success, and adapt the description and all
callers.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c           | 10 ++++++----
 drivers/scsi/scsi_transport_sas.c |  9 ++++-----
 drivers/scsi/sd.c                 | 12 ++++++------
 drivers/scsi/sr.c                 |  2 +-
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d7c0d5a5f263..ca9dbfa42cb7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2135,9 +2135,7 @@ EXPORT_SYMBOL_GPL(scsi_mode_select);
  *	@sshdr: place to put sense data (or NULL if no sense to be collected).
  *		must be SCSI_SENSE_BUFFERSIZE big.
  *
- *	Returns zero if unsuccessful, or the header offset (either 4
- *	or 8 depending on whether a six or ten byte command was
- *	issued) if successful.
+ *	Returns zero if successful, or a negative error number on failure
  */
 int
 scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
@@ -2184,6 +2182,8 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
 
 	result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, len,
 				  sshdr, timeout, retries, NULL);
+	if (result < 0)
+		return result;
 
 	/* This code looks awful: what it's doing is making sure an
 	 * ILLEGAL REQUEST sense return identifies the actual command
@@ -2228,13 +2228,15 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
 			data->block_descriptor_length = buffer[3];
 		}
 		data->header_length = header_length;
+		result = 0;
 	} else if ((status_byte(result) == CHECK_CONDITION) &&
 		   scsi_sense_valid(sshdr) &&
 		   sshdr->sense_key == UNIT_ATTENTION && retry_count) {
 		retry_count--;
 		goto retry;
 	}
-
+	if (result > 0)
+		result = -EIO;
 	return result;
 }
 EXPORT_SYMBOL(scsi_mode_sense);
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index c9abed8429c9..4a96fb05731d 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1229,16 +1229,15 @@ int sas_read_port_mode_page(struct scsi_device *sdev)
 	char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata;
 	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
 	struct scsi_mode_data mode_data;
-	int res, error;
+	int error;
 
 	if (!buffer)
 		return -ENOMEM;
 
-	res = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3,
-			      &mode_data, NULL);
+	error = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3,
+				&mode_data, NULL);
 
-	error = -EINVAL;
-	if (!scsi_status_is_good(res))
+	if (error)
 		goto out;
 
 	msdata = buffer +  mode_data.header_length +
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index cb3c37d1e009..2ef2954375f4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2670,18 +2670,18 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 		 * 5: Illegal Request, Sense Code 24: Invalid field in
 		 * CDB.
 		 */
-		if (!scsi_status_is_good(res))
+		if (res < 0)
 			res = sd_do_mode_sense(sdkp, 0, 0, buffer, 4, &data, NULL);
 
 		/*
 		 * Third attempt: ask 255 bytes, as we did earlier.
 		 */
-		if (!scsi_status_is_good(res))
+		if (res < 0)
 			res = sd_do_mode_sense(sdkp, 0, 0x3F, buffer, 255,
 					       &data, NULL);
 	}
 
-	if (!scsi_status_is_good(res)) {
+	if (res < 0) {
 		sd_first_printk(KERN_WARNING, sdkp,
 			  "Test WP failed, assume Write Enabled\n");
 	} else {
@@ -2742,7 +2742,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
 	res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, first_len,
 			&data, &sshdr);
 
-	if (!scsi_status_is_good(res))
+	if (res < 0)
 		goto bad_sense;
 
 	if (!data.header_length) {
@@ -2774,7 +2774,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
 		res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, len,
 				&data, &sshdr);
 
-	if (scsi_status_is_good(res)) {
+	if (!res) {
 		int offset = data.header_length + data.block_descriptor_length;
 
 		while (offset < len) {
@@ -2892,7 +2892,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
 	res = scsi_mode_sense(sdp, 1, 0x0a, buffer, 36, SD_TIMEOUT,
 			      sdkp->max_retries, &data, &sshdr);
 
-	if (!scsi_status_is_good(res) || !data.header_length ||
+	if (res < 0 || !data.header_length ||
 	    data.length < 6) {
 		sd_first_printk(KERN_WARNING, sdkp,
 			  "getting Control mode page failed, assume no ATO\n");
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index e4633b84c556..9b2ccf0c9a8c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -911,7 +911,7 @@ static void get_capabilities(struct scsi_cd *cd)
 	rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, ms_len,
 			     SR_TIMEOUT, 3, &data, NULL);
 
-	if (!scsi_status_is_good(rc) || data.length > ms_len ||
+	if (rc < 0 || data.length > ms_len ||
 	    data.header_length + data.block_descriptor_length > data.length) {
 		/* failed, drive doesn't have capabilities mode page */
 		cd->cdi.speed = 1;
-- 
2.29.2


  parent reply index

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-27  8:30 [PATCHv3 00/40] SCSI result cleanup, part 2 Hannes Reinecke
2021-04-27  8:30 ` [PATCH 01/40] st: return error code in st_scsi_execute() Hannes Reinecke
2021-04-27 12:11   ` "Kai Mäkisara (Kolumbus)"
2021-04-27  8:30 ` [PATCH 02/40] scsi_ioctl: return error code when blk_rq_map_kern() fails Hannes Reinecke
2021-04-27  8:30 ` Hannes Reinecke [this message]
2021-04-27  8:30 ` [PATCH 04/40] scsi: reshuffle response handling in scsi_mode_sense() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 05/40] scsi_dh_alua: check for negative result value Hannes Reinecke
2021-04-27  8:30 ` [PATCH 06/40] scsi: stop using DRIVER_ERROR Hannes Reinecke
2021-04-27  8:30 ` [PATCH 07/40] scsi: introduce scsi_build_sense() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 08/40] scsi: introduce scsi_status_is_check_condition() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 09/40] scsi: Kill DRIVER_SENSE Hannes Reinecke
2021-04-27  8:30 ` [PATCH 10/40] scsi: do not use DRIVER_INVALID Hannes Reinecke
2021-04-27  8:30 ` [PATCH 11/40] scsi_error: use DID_TIME_OUT instead of DRIVER_TIMEOUT Hannes Reinecke
2021-04-27  8:30 ` [PATCH 12/40] xen-scsiback: use DID_ERROR instead of DRIVER_ERROR Hannes Reinecke
2021-04-27  8:30 ` [PATCH 13/40] xen-scsifront: compability status handling Hannes Reinecke
2021-04-27  8:30 ` [PATCH 14/40] scsi: Drop the now obsolete driver_byte definitions Hannes Reinecke
2021-04-27  8:30 ` [PATCH 15/40] NCR5380: Fold SCSI message ABORT onto DID_ABORT Hannes Reinecke
2021-04-27  8:30 ` [PATCH 16/40] scsi: add get_{status,host}_byte() accessor function Hannes Reinecke
2021-04-27  8:30 ` [PATCH 17/40] scsi: add scsi_msg_to_host_byte() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 18/40] dc395: use standard macros to set SCSI result Hannes Reinecke
2021-04-27  8:30 ` [PATCH 19/40] dc395: translate message bytes Hannes Reinecke
2021-04-27  8:30 ` [PATCH 20/40] qlogicfas408: make ql_pcmd() a void function Hannes Reinecke
2021-04-27  8:30 ` [PATCH 21/40] qlogicfas408: whitespace cleanup Hannes Reinecke
2021-04-27  8:30 ` [PATCH 22/40] qlogicfas408: translate message to host byte status Hannes Reinecke
2021-04-27  8:30 ` [PATCH 23/40] nsp32: whitespace cleanup Hannes Reinecke
2021-04-27  8:30 ` [PATCH 24/40] nsp32: do not set message byte Hannes Reinecke
2021-04-27  8:30 ` [PATCH 25/40] wd33c93: translate message byte to host byte Hannes Reinecke
2021-04-27  8:30 ` [PATCH 26/40] mesh: translate message to host byte status Hannes Reinecke
2021-04-27  8:30 ` [PATCH 27/40] acornscsi: remove acornscsi_reportstatus() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 28/40] acornscsi: translate message byte to host byte Hannes Reinecke
2021-04-27  8:30 ` [PATCH 29/40] aha152x: modify done() to use separate status bytes Hannes Reinecke
2021-04-27  8:30 ` [PATCH 30/40] aha152x: do not set message byte when calling scsi_done() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 31/40] advansys: do not set message byte in SCSI status Hannes Reinecke
2021-04-27  8:30 ` [PATCH 32/40] fas216: translate message to host byte status Hannes Reinecke
2021-04-27  8:30 ` [PATCH 33/40] fas216: Use get_status_byte() to avoid using linux-specific status codes Hannes Reinecke
2021-04-27  8:30 ` [PATCH 34/40] FlashPoint: Use standard SCSI definitions Hannes Reinecke
2021-04-27  8:30 ` [PATCH 35/40] fdomain: drop last argument to fdomain_finish_cmd() Hannes Reinecke
2021-04-27  8:30 ` [PATCH 36/40] fdomain: translate message to host byte status Hannes Reinecke
2021-04-27  8:30 ` [PATCH 37/40] scsi: drop message byte helper Hannes Reinecke
2021-04-27  8:30 ` [PATCH 38/40] scsi: kill message byte Hannes Reinecke
2021-04-27  8:30 ` [PATCH 39/40] target: use standard SAM status types Hannes Reinecke
2021-04-27  8:30 ` [PATCH 40/40] scsi: drop obsolete linux-specific SCSI status codes Hannes Reinecke
2021-04-29  6:48   ` Christoph Hellwig
2021-04-29  7:29     ` Hannes Reinecke
2021-04-29 15:52     ` Douglas Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210427083046.31620-4-hare@suse.de \
    --to=hare@suse.de \
    --cc=bvanassche@acm.org \
    --cc=hch@lst.de \
    --cc=james.bottomley@hansenpartnership.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git