Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 7 May 2025 21:37:11 GMT
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: b8e77c8b00c1 - main - umass: Move common code into umass_std_transform
Message-ID:  <202505072137.547LbBLS092118@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=b8e77c8b00c1190005142b1a2a70e3b5d36c6841

commit b8e77c8b00c1190005142b1a2a70e3b5d36c6841
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2025-05-07 16:07:31 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2025-05-07 21:36:54 +0000

    umass: Move common code into umass_std_transform
    
    Move the length checks, and byte copying into umass_std_transform. The
    copies are typically small and this simplifies the code a lot. Move
    zeroing the buffer now to only when we change TEST UNIT READY to START
    STOP.
    
    Sponsored by:           Netflix
    Differential Revision:  https://reviews.freebsd.org/D49471
---
 sys/dev/usb/storage/umass.c | 54 +++++++++------------------------------------
 1 file changed, 11 insertions(+), 43 deletions(-)

diff --git a/sys/dev/usb/storage/umass.c b/sys/dev/usb/storage/umass.c
index f7df9f49257e..344a42f718fc 100644
--- a/sys/dev/usb/storage/umass.c
+++ b/sys/dev/usb/storage/umass.c
@@ -2680,12 +2680,6 @@ static bool
 umass_scsi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
     uint8_t cmd_len)
 {
-	if ((cmd_len == 0) ||
-	    (cmd_len > sizeof(sc->sc_transfer.cmd_data))) {
-		DPRINTF(sc, UDMASS_SCSI, "Invalid command "
-		    "length: %d bytes\n", cmd_len);
-		return (false);		/* failure */
-	}
 	sc->sc_transfer.cmd_len = cmd_len;
 
 	switch (cmd_ptr[0]) {
@@ -2706,26 +2700,16 @@ umass_scsi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 		 * information.
 		 */
 		if (sc->sc_quirks & FORCE_SHORT_INQUIRY) {
-			memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
 			sc->sc_transfer.cmd_data[4] = SHORT_INQUIRY_LENGTH;
-			return (true);
 		}
 		break;
 	}
-
-	memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
 	return (true);
 }
 
 static bool
 umass_rbc_transform(struct umass_softc *sc, uint8_t *cmd_ptr, uint8_t cmd_len)
 {
-	if ((cmd_len == 0) ||
-	    (cmd_len > sizeof(sc->sc_transfer.cmd_data))) {
-		DPRINTF(sc, UDMASS_SCSI, "Invalid command "
-		    "length: %d bytes\n", cmd_len);
-		return (false);		/* failure */
-	}
 	switch (cmd_ptr[0]) {
 		/* these commands are defined in RBC: */
 	case READ_10:
@@ -2746,9 +2730,6 @@ umass_rbc_transform(struct umass_softc *sc, uint8_t *cmd_ptr, uint8_t cmd_len)
 		 */
 	case REQUEST_SENSE:
 	case PREVENT_ALLOW:
-
-		memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
-
 		if ((sc->sc_quirks & RBC_PAD_TO_12) && (cmd_len < 12)) {
 			memset(sc->sc_transfer.cmd_data + cmd_len,
 			    0, 12 - cmd_len);
@@ -2769,18 +2750,9 @@ static bool
 umass_ufi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
     uint8_t cmd_len)
 {
-	if ((cmd_len == 0) ||
-	    (cmd_len > sizeof(sc->sc_transfer.cmd_data))) {
-		DPRINTF(sc, UDMASS_SCSI, "Invalid command "
-		    "length: %d bytes\n", cmd_len);
-		return (false);		/* failure */
-	}
 	/* An UFI command is always 12 bytes in length */
 	sc->sc_transfer.cmd_len = UFI_COMMAND_LENGTH;
 
-	/* Zero the command data */
-	memset(sc->sc_transfer.cmd_data, 0, UFI_COMMAND_LENGTH);
-
 	switch (cmd_ptr[0]) {
 		/*
 		 * Commands of which the format has been verified. They
@@ -2796,6 +2768,8 @@ umass_ufi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 			DPRINTF(sc, UDMASS_UFI, "Converted TEST_UNIT_READY "
 			    "to START_UNIT\n");
 
+			/* Zero the command data */
+			memset(sc->sc_transfer.cmd_data, 0, UFI_COMMAND_LENGTH);
 			sc->sc_transfer.cmd_data[0] = START_STOP_UNIT;
 			sc->sc_transfer.cmd_data[4] = SSS_START;
 			return (1);
@@ -2835,7 +2809,6 @@ umass_ufi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 		return (false);		/* failure */
 	}
 
-	memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
 	return (true);			/* success */
 }
 
@@ -2846,18 +2819,9 @@ static bool
 umass_atapi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
     uint8_t cmd_len)
 {
-	if ((cmd_len == 0) ||
-	    (cmd_len > sizeof(sc->sc_transfer.cmd_data))) {
-		DPRINTF(sc, UDMASS_SCSI, "Invalid command "
-		    "length: %d bytes\n", cmd_len);
-		return (false);		/* failure */
-	}
 	/* An ATAPI command is always 12 bytes in length. */
 	sc->sc_transfer.cmd_len = ATAPI_COMMAND_LENGTH;
 
-	/* Zero the command data */
-	memset(sc->sc_transfer.cmd_data, 0, ATAPI_COMMAND_LENGTH);
-
 	switch (cmd_ptr[0]) {
 		/*
 		 * Commands of which the format has been verified. They
@@ -2867,13 +2831,10 @@ umass_atapi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 	case INQUIRY:
 		/*
 		 * some drives wedge when asked for full inquiry
-		 * information.
+		 * information, so adjust the transfer length
 		 */
 		if (sc->sc_quirks & FORCE_SHORT_INQUIRY) {
-			memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
-
 			sc->sc_transfer.cmd_data[4] = SHORT_INQUIRY_LENGTH;
-			return (true);
 		}
 		break;
 
@@ -2881,6 +2842,7 @@ umass_atapi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 		if (sc->sc_quirks & NO_TEST_UNIT_READY) {
 			DPRINTF(sc, UDMASS_SCSI, "Converted TEST_UNIT_READY "
 			    "to START_UNIT\n");
+			memset(sc->sc_transfer.cmd_data, 0, ATAPI_COMMAND_LENGTH);
 			sc->sc_transfer.cmd_data[0] = START_STOP_UNIT;
 			sc->sc_transfer.cmd_data[4] = SSS_START;
 			return (true);
@@ -2931,7 +2893,6 @@ umass_atapi_transform(struct umass_softc *sc, uint8_t *cmd_ptr,
 		break;
 	}
 
-	memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
 	return (true);			/* success */
 }
 
@@ -2948,6 +2909,13 @@ umass_std_transform(struct umass_softc *sc, union ccb *ccb,
 {
 	uint8_t retval;
 
+	if (cmd_len == 0 || cmd_len > sizeof(sc->sc_transfer.cmd_data)) {
+		DPRINTF(sc, UDMASS_SCSI, "Invalid command length: %d bytes\n",
+		    cmd_len);
+		return (false);		/* failure */
+	}
+
+	memcpy(sc->sc_transfer.cmd_data, cmd_ptr, cmd_len);
 	if (sc->sc_transform(sc, cmd, cmdlen))
 		return (true);	/* Execute command */
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505072137.547LbBLS092118>