Skip to contentSkip to conversation

be2iscsi: cleanup a min_t() call

PATCH In review Dan Carpenter 3 participants 4 messages Sep 26, 2011 – Nov 29, 2011

Subsystems (3)
SCSI Kernel janitors Emulex 10Gbps iSCSI - OneConnect driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 be2iscsi: cleanup a min_t() call

"sense_len" was declared as int type but actually it only stores a u16 value that comes from hardware. The cast to u16 in min_t() confuses static analysis because it truncates the int to u16 so I've fixed the declaration to reflect that "sense_len" is just a u16.

Also there was a call to cpu_to_be16() which I've changed to be16_to_cpu(). The functions are equivalent, but obviously the hardware is big endian and we're doing the min_t() comparison on CPU endian values.

This whole patch is just a cleanup and doesn't affect how the code works.

Signed-off-by Dan Carpenter <[email protected]>

---
Compile tested only. Sorry.

drivers/scsi/be2iscsi/be_main.c +3 -2
index 57fea38..344af8d 100644
@@ -1104,7 +1104,6 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	struct be_status_bhs *sts_bhs  				(struct be_status_bhs *)io_task->cmd_bhs;
 	struct iscsi_conn *conn = beiscsi_conn->conn;
-	unsigned int sense_len;
 	unsigned char *sense;
 	u32 resid = 0, exp_cmdsn, max_cmdsn;
 	u8 rsp, status, flags;

@@ -1147,9 +1146,11 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	}
 
 	if (status = SAM_STAT_CHECK_CONDITION) {
+		u16 sense_len;
 		unsigned short *slen = (unsigned short *)sts_bhs->sense_info;
+
 		sense = sts_bhs->sense_info + sizeof(unsigned short);
-		sense_len =  cpu_to_be16(*slen);
+		sense_len = be16_to_cpu(*slen);
 		memcpy(task->sc->sense_buffer, sense,
 		       min_t(u16, sense_len, SCSI_SENSE_BUFFERSIZE));
 	}
author
#1 lore raw
Quoted 10 lines · Dan Carpenter

"sense_len" was declared as int type but actually it only stores a u16 value that comes from hardware. The cast to u16 in min_t() confuses static analysis because it truncates the int to u16 so I've fixed the declaration to reflect that "sense_len" is just a u16.

Also there was a call to cpu_to_be16() which I've changed to be16_to_cpu(). The functions are equivalent, but obviously the hardware is big endian and we're doing the min_t() comparison on CPU endian values.

Quoted 5 lines

This whole patch is just a cleanup and doesn't affect how the code works.

Signed-off-by Dan Carpenter <[email protected]>

---

Ping.

regards,
dan carpenter

Reviewed-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter
Quoted 30 lines · Dan Carpenter

"sense_len" was declared as int type but actually it only stores a u16 value that comes from hardware. The cast to u16 in min_t() confuses static analysis because it truncates the int to u16 so I've fixed the declaration to reflect that "sense_len" is just a u16.

Also there was a call to cpu_to_be16() which I've changed to be16_to_cpu(). The functions are equivalent, but obviously the hardware is big endian and we're doing the min_t() comparison on CPU endian values.

This whole patch is just a cleanup and doesn't affect how the code works.

Signed-off-by Dan Carpenter <[email protected]>

---
Compile tested only. Sorry.

drivers/scsi/be2iscsi/be_main.c +0 -1
index 57fea38..344af8d 100644
@@ -1104,7 +1104,6 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	struct be_status_bhs *sts_bhs >  				(struct be_status_bhs *)io_task->cmd_bhs;
 	struct iscsi_conn *conn = beiscsi_conn->conn;
-	unsigned int sense_len;
 	unsigned char *sense;
 	u32 resid = 0, exp_cmdsn, max_cmdsn;
 	u8 rsp, status, flags;

@@ -1147,9 +1146,11 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	}

drivers/scsi/be2iscsi/be_main.c be_complete_io(struct beiscsi_conn *beiscsi_conn, · line 1155

Quoted 10 lines
drivers/scsi/be2iscsi/be_main.c +3 -1
 
 	if (status = SAM_STAT_CHECK_CONDITION) {
+		u16 sense_len;
 		unsigned short *slen = (unsigned short *)sts_bhs->sense_info;
+
 		sense = sts_bhs->sense_info + sizeof(unsigned short);
-		sense_len =  cpu_to_be16(*slen);
+		sense_len = be16_to_cpu(*slen);
 		memcpy(task->sc->sense_buffer, sense,
 		       min_t(u16, sense_len, SCSI_SENSE_BUFFERSIZE));

Looks ok to me.

Reviewed-by Mike Christie <[email protected]>
Acked-by Reviewed-by Mike Christie
#3 lore raw

Looks good.

Acked-by Jayamohan Kallickal <[email protected]>

Thanks
Jay

-----Original Message-----
From: Mike Christie [mailto:[email protected]]
Sent: Friday, November 18, 2011 11:28 AM
To: Dan Carpenter
Cc: Kallickal, Jayamohan; James E.J. Bottomley; [email protected]; [email protected]
Subject: Re: [patch] [SCSI] be2iscsi: cleanup a min_t() call

Quoted 30 lines · Dan Carpenter

"sense_len" was declared as int type but actually it only stores a u16 value that comes from hardware. The cast to u16 in min_t() confuses static analysis because it truncates the int to u16 so I've fixed the declaration to reflect that "sense_len" is just a u16.

Also there was a call to cpu_to_be16() which I've changed to be16_to_cpu(). The functions are equivalent, but obviously the hardware is big endian and we're doing the min_t() comparison on CPU endian values.

This whole patch is just a cleanup and doesn't affect how the code works.

Signed-off-by Dan Carpenter <[email protected]>

---
Compile tested only. Sorry.

drivers/scsi/be2iscsi/be_main.c +0 -1
index 57fea38..344af8d 100644
@@ -1104,7 +1104,6 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	struct be_status_bhs *sts_bhs >  				(struct be_status_bhs *)io_task->cmd_bhs;
 	struct iscsi_conn *conn = beiscsi_conn->conn;
-	unsigned int sense_len;
 	unsigned char *sense;
 	u32 resid = 0, exp_cmdsn, max_cmdsn;
 	u8 rsp, status, flags;

@@ -1147,9 +1146,11 @@ be_complete_io(struct beiscsi_conn *beiscsi_conn,
 	}

drivers/scsi/be2iscsi/be_main.c be_complete_io(struct beiscsi_conn *beiscsi_conn, · line 1155

Quoted 10 lines
drivers/scsi/be2iscsi/be_main.c +3 -1
 
 	if (status = SAM_STAT_CHECK_CONDITION) {
+		u16 sense_len;
 		unsigned short *slen = (unsigned short *)sts_bhs->sense_info;
+
 		sense = sts_bhs->sense_info + sizeof(unsigned short);
-		sense_len =  cpu_to_be16(*slen);
+		sense_len = be16_to_cpu(*slen);
 		memcpy(task->sc->sense_buffer, sense,
 		       min_t(u16, sense_len, SCSI_SENSE_BUFFERSIZE));

Looks ok to me.

Reviewed-by Mike Christie <[email protected]>