qla2xxx: Reduce the number of Coverity warnings

Remove dead code and simplify a pointer computation.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------

1 file changed, 1 insertion(+), 8 deletions(-)

drivers/scsi/qla2xxx/qla_bsg.c +1 -8

index 39719f8..7221521 100644
@@ -1282,14 +1282,7 @@ qla24xx_iidma(struct fc_bsg_job *bsg_job)
 		return -EINVAL;
 	}
 
-	port_param = (struct qla_port_param *)((char *)bsg_job->request +
-		sizeof(struct fc_bsg_request));
-	if (!port_param) {
-		ql_log(ql_log_warn, vha, 0x7047,
-		    "port_param header not provided.\n");
-		return -EINVAL;
-	}
-
+	port_param = (void *)bsg_job->request + sizeof(struct fc_bsg_request);
 	if (port_param->fc_scsi_addr.dest_type != EXT_DEF_TYPE_WWPN) {
 		ql_log(ql_log_warn, vha, 0x7048,
 		    "Invalid destination type.\n");

Signature · 1 line

1.7.10.4

Hi Bart,
Missing msg ids needs to be updated in qla_dbg.c file. Here is an updated patch

Quoted 4 lines

Hi Bart,
Missing msg ids needs to be updated in qla_dbg.c file. Here is an updated
patch

OK, so since you modified the first patch in the series, you need to be the one who sends it all to me (with proper signoffs).

Thanks,

James

Remove dead code, simplify a pointer computation and move the ql84_mgmt assignment to just before its first use.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------

1 file changed, 1 insertion(+), 8 deletions(-)

drivers/scsi/qla2xxx/qla_bsg.c +1 -8

index 7221521..cf07491 100644
@@ -1084,14 +1084,6 @@ qla84xx_mgmt_cmd(struct fc_bsg_job *bsg_job)
 		return -EINVAL;
 	}
 
-	ql84_mgmt = (struct qla_bsg_a84_mgmt *)((char *)bsg_job->request +
-		sizeof(struct fc_bsg_request));
-	if (!ql84_mgmt) {
-		ql_log(ql_log_warn, vha, 0x703b,
-		    "MGMT header not provided, exiting.\n");
-		return -EINVAL;
-	}
-
 	mn = dma_pool_alloc(ha->s_dma_pool, GFP_KERNEL, &mn_dma);
 	if (!mn) {
 		ql_log(ql_log_warn, vha, 0x703c,

@@ -1103,6 +1095,7 @@ qla84xx_mgmt_cmd(struct fc_bsg_job *bsg_job)
 	mn->entry_type = ACCESS_CHIP_IOCB_TYPE;
 	mn->entry_count = 1;
 
+	ql84_mgmt = (void *)bsg_job->request + sizeof(struct fc_bsg_request);
 	switch (ql84_mgmt->mgmt.cmd) {
 	case QLA84_MGMT_READ_MEM:
 	case QLA84_MGMT_GET_INFO:

Signature · 6 lines

1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

At the end of qla2x00_configure_hba() we know that rval == QLA_SUCCESS. Hence remove the code that depends on rval != QLA_SUCCESS.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_init.c | 8 +-------

1 file changed, 1 insertion(+), 7 deletions(-)

drivers/scsi/qla2xxx/qla_init.c +1 -7

index 3565dfd..c68bb01 100644
@@ -2309,13 +2309,7 @@ qla2x00_configure_hba(scsi_qla_host_t *vha)
 		    "Topology - %s, Host Loop address 0x%x.\n",
 		    connect_type, vha->loop_id);
 
-	if (rval) {
-		ql_log(ql_log_warn, vha, 0x2011,
-		    "%s FAILED\n", __func__);
-	} else {
-		ql_dbg(ql_dbg_disc, vha, 0x2012,
-		    "%s success\n", __func__);
-	}
+	ql_dbg(ql_dbg_disc, vha, 0x2012, "%s success\n", __func__);
 
 	return(rval);
 }

Signature · 1 line

1.7.10.4

Since ha->model_desc is an array comparing it against NULL is superfluous. Hence remove these tests.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---

drivers/scsi/qla2xxx/qla_gs.c |    3 +--
drivers/scsi/qla2xxx/qla_os.c |    3 +--
2 files changed, 2 insertions(+), 4 deletions(-)

drivers/scsi/qla2xxx/qla_gs.c +1 -2

index d0ea8b9..f26442a 100644
@@ -1370,8 +1370,7 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
 	/* Model description. */
 	eiter = (struct ct_fdmi_hba_attr *) (entries + size);
 	eiter->type = __constant_cpu_to_be16(FDMI_HBA_MODEL_DESCRIPTION);
-	if (ha->model_desc)
-		strncpy(eiter->a.model_desc, ha->model_desc, 80);
+	strncpy(eiter->a.model_desc, ha->model_desc, 80);
 	alen = strlen(eiter->a.model_desc);
 	alen += (alen & 3) ? (4 - (alen & 3)) : 4;
 	eiter->len = cpu_to_be16(4 + alen);

drivers/scsi/qla2xxx/qla_os.c +1 -2

index ad72c1d..1d97626 100644
@@ -2840,8 +2840,7 @@ skip_dpc:
 	qla2x00_dfs_setup(base_vha);
 
 	ql_log(ql_log_info, base_vha, 0x00fb,
-	    "QLogic %s - %s.\n",
-	    ha->model_number, ha->model_desc ? ha->model_desc : "");
+	    "QLogic %s - %s.\n", ha->model_number, ha->model_desc);
 	ql_log(ql_log_info, base_vha, 0x00fc,
 	    "ISP%04X: %s @ %s hdma%c host#=%ld fw=%s.\n",
 	    pdev->device, ha->isp_ops->pci_info_str(base_vha, pci_info),

Signature · 1 line

1.7.10.4

Since the value of cur_seg is not used and since scsi_prot_sglist() has no side effects it is safe to remove the statement "cur_seg = scsi_prot_sglist(cmd)". Detected by Coverity.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_iocb.c | 2 --

1 file changed, 2 deletions(-)

drivers/scsi/qla2xxx/qla_iocb.c +0 -1

index 15e4080..b589d24 100644
@@ -1189,7 +1189,6 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct
 cmd_type_crc_2 *cmd_pkt,
 	uint32_t		*cur_dsd, *fcp_dl;
 	scsi_qla_host_t		*vha;
 	struct scsi_cmnd	*cmd;
-	struct scatterlist	*cur_seg;
 	int			sgc;
 	uint32_t		total_bytes = 0;

	uint32_t		data_bytes;

drivers/scsi/qla2xxx/qla_iocb.c +0 -1

@@ -1396,7 +1395,6 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct
 cmd_type_crc_2 *cmd_pkt,
 
 	if (bundling && tot_prot_dsds) {
 		/* Walks dif segments */
-		cur_seg = scsi_prot_sglist(cmd);
 		cmd_pkt->control_flags |=
 			__constant_cpu_to_le16(CF_DIF_SEG_DESCR_ENABLE);

		cur_dsd = (uint32_t *) &crc_ctx_pkt->u.bundling.dif_address;

Signature · 1 line

1.7.10.4

The value of the pointer called "nxt" is not used after the "nxt = qla24xx_copy_eft(ha, nxt)" statement. Hence keep the function call but remove the assignment.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_dbg.c | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)

drivers/scsi/qla2xxx/qla_dbg.c +1 -1

index cfa2a20..c612785 100644
@@ -1468,7 +1468,7 @@ qla25xx_fw_dump(scsi_qla_host_t *vha, int
 hardware_locked)
 
 	nxt = qla2xxx_copy_queues(ha, nxt);
 
-	nxt = qla24xx_copy_eft(ha, nxt);
+	qla24xx_copy_eft(ha, nxt);
 
 	/* Chain entries -- started with MQ. */

	nxt_chain = qla25xx_copy_fce(ha, nxt_chain, &last_chain);

drivers/scsi/qla2xxx/qla_dbg.c +1 -1

@@ -1787,7 +1787,7 @@ qla81xx_fw_dump(scsi_qla_host_t *vha, int
 hardware_locked)
 
 	nxt = qla2xxx_copy_queues(ha, nxt);
 
-	nxt = qla24xx_copy_eft(ha, nxt);
+	qla24xx_copy_eft(ha, nxt);
 
 	/* Chain entries -- started with MQ. */

	nxt_chain = qla25xx_copy_fce(ha, nxt_chain, &last_chain);

drivers/scsi/qla2xxx/qla_dbg.c +0 -0

@@ -2289,7 +2289,7 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int
 hardware_locked)

copy_queue:

	nxt = qla2xxx_copy_queues(ha, nxt);

drivers/scsi/qla2xxx/qla_dbg.c +1 -1

-	nxt = qla24xx_copy_eft(ha, nxt);
+	qla24xx_copy_eft(ha, nxt);

	/* Chain entries -- started with MQ. */
	nxt_chain = qla25xx_copy_fce(ha, nxt_chain, &last_chain);

Signature · 1 line

1.7.10.4

Coverity reports "Overrunning struct type ct_sns_req of 1228 bytes by passing it to a function which accesses it at byte offset 8207" for each qla2x00_prep_ct_req(), qla2x00_prep_ct_fdmi_req() and qla24xx_prep_ct_fm_req() call. Help Coverity to recognize that these calls do not trigger a buffer overflow by making it explicit that these three functions initializes both the request and reply structures. This patch does not change any functionality.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_gs.c | 86
++++++++++++++++++-----------------------

1 file changed, 38 insertions(+), 48 deletions(-)

drivers/scsi/qla2xxx/qla_gs.c +0 -0

index f26442a..1ad361b 100644
@@ -99,17 +99,17 @@ qla24xx_prep_ms_iocb(scsi_qla_host_t *vha, uint32_t
 req_size, uint32_t rsp_size)
 * Returns a pointer to the intitialized @ct_req.
 */

static inline struct ct_sns_req * -qla2x00_prep_ct_req(struct ct_sns_req *ct_req, uint16_t cmd, uint16_t rsp_size)
+qla2x00_prep_ct_req(struct ct_sns_pkt *p, uint16_t cmd, uint16_t rsp_size)
{

drivers/scsi/qla2xxx/qla_gs.c +7 -7

-	memset(ct_req, 0, sizeof(struct ct_sns_pkt));
+	memset(p, 0, sizeof(struct ct_sns_pkt));
 
-	ct_req->header.revision = 0x01;
-	ct_req->header.gs_type = 0xFC;
-	ct_req->header.gs_subtype = 0x02;
-	ct_req->command = cpu_to_be16(cmd);
-	ct_req->max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
+	p->p.req.header.revision = 0x01;
+	p->p.req.header.gs_type = 0xFC;
+	p->p.req.header.gs_subtype = 0x02;
+	p->p.req.command = cpu_to_be16(cmd);
+	p->p.req.max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
 
-	return (ct_req);
+	return &p->p.req;

}

static int

drivers/scsi/qla2xxx/qla_gs.c +1 -2

@@ -188,8 +188,7 @@ qla2x00_ga_nxt(scsi_qla_host_t *vha, fc_port_t
 *fcport)
 	    GA_NXT_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GA_NXT_CMD,
-	    GA_NXT_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, GA_NXT_CMD, GA_NXT_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 

	/* Prepare CT arguments -- port_id */

drivers/scsi/qla2xxx/qla_gs.c +7 -11

@@ -284,8 +283,7 @@ qla2x00_gid_pt(scsi_qla_host_t *vha, sw_info_t *list)
 	    gid_pt_rsp_size);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GID_PT_CMD,
-	    gid_pt_rsp_size);
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, GID_PT_CMD, gid_pt_rsp_size);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare CT arguments -- port_type */

@@ -359,7 +357,7 @@ qla2x00_gpn_id(scsi_qla_host_t *vha, sw_info_t *list)
 		    GPN_ID_RSP_SIZE);
 
 		/* Prepare CT request */
-		ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GPN_ID_CMD,
+		ct_req = qla2x00_prep_ct_req(ha->ct_sns, GPN_ID_CMD,
 		    GPN_ID_RSP_SIZE);
 		ct_rsp = &ha->ct_sns->p.rsp;
 

@@ -421,7 +419,7 @@ qla2x00_gnn_id(scsi_qla_host_t *vha, sw_info_t *list)
 		    GNN_ID_RSP_SIZE);
 
 		/* Prepare CT request */
-		ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GNN_ID_CMD,
+		ct_req = qla2x00_prep_ct_req(ha->ct_sns, GNN_ID_CMD,
 		    GNN_ID_RSP_SIZE);
 		ct_rsp = &ha->ct_sns->p.rsp;
 

@@ -495,7 +493,7 @@ qla2x00_rft_id(scsi_qla_host_t *vha)
 	    RFT_ID_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, RFT_ID_CMD,
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, RFT_ID_CMD,
 	    RFT_ID_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 

@@ -551,8 +549,7 @@ qla2x00_rff_id(scsi_qla_host_t *vha)
 	    RFF_ID_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, RFF_ID_CMD,
-	    RFF_ID_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, RFF_ID_CMD, RFF_ID_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare CT arguments -- port_id, FC-4 feature, FC-4 type */

@@ -606,8 +603,7 @@ qla2x00_rnn_id(scsi_qla_host_t *vha)
 	    RNN_ID_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, RNN_ID_CMD,
-	    RNN_ID_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, RNN_ID_CMD, RNN_ID_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare CT arguments -- port_id, node_name */

@@ -676,8 +672,7 @@ qla2x00_rsnn_nn(scsi_qla_host_t *vha)
 	ms_pkt = ha->isp_ops->prep_ms_iocb(vha, 0, RSNN_NN_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, RSNN_NN_CMD,
-	    RSNN_NN_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_req(ha->ct_sns, RSNN_NN_CMD, RSNN_NN_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare CT arguments -- node_name, symbolic node_name, size */

@@ -1262,18 +1257,17 @@ qla2x00_update_ms_fdmi_iocb(scsi_qla_host_t *vha,
 uint32_t req_size)
 * Returns a pointer to the intitialized @ct_req.
 */

static inline struct ct_sns_req *

drivers/scsi/qla2xxx/qla_gs.c +1 -2

-qla2x00_prep_ct_fdmi_req(struct ct_sns_req *ct_req, uint16_t cmd,
-    uint16_t rsp_size)
+qla2x00_prep_ct_fdmi_req(struct ct_sns_pkt *p, uint16_t cmd, uint16_t

rsp_size)
{

drivers/scsi/qla2xxx/qla_gs.c +7 -7

-	memset(ct_req, 0, sizeof(struct ct_sns_pkt));
+	memset(p, 0, sizeof(struct ct_sns_pkt));
 
-	ct_req->header.revision = 0x01;
-	ct_req->header.gs_type = 0xFA;
-	ct_req->header.gs_subtype = 0x10;
-	ct_req->command = cpu_to_be16(cmd);
-	ct_req->max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
+	p->p.req.header.revision = 0x01;
+	p->p.req.header.gs_type = 0xFA;
+	p->p.req.header.gs_subtype = 0x10;
+	p->p.req.command = cpu_to_be16(cmd);
+	p->p.req.max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
 
-	return ct_req;
+	return &p->p.req;

}

/**

drivers/scsi/qla2xxx/qla_gs.c +4 -7

@@ -1301,8 +1295,7 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
 	ms_pkt = ha->isp_ops->prep_ms_fdmi_iocb(vha, 0, RHBA_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_fdmi_req(&ha->ct_sns->p.req, RHBA_CMD,
-	    RHBA_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_fdmi_req(ha->ct_sns, RHBA_CMD, RHBA_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare FDMI command arguments -- attribute block, attributes. */

@@ -1490,8 +1483,7 @@ qla2x00_fdmi_dhba(scsi_qla_host_t *vha)
 	    DHBA_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_fdmi_req(&ha->ct_sns->p.req, DHBA_CMD,
-	    DHBA_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_fdmi_req(ha->ct_sns, DHBA_CMD, DHBA_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare FDMI command arguments -- portname. */

@@ -1547,8 +1539,7 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
 	ms_pkt = ha->isp_ops->prep_ms_fdmi_iocb(vha, 0, RPA_RSP_SIZE);
 
 	/* Prepare CT request */
-	ct_req = qla2x00_prep_ct_fdmi_req(&ha->ct_sns->p.req, RPA_CMD,
-	    RPA_RSP_SIZE);
+	ct_req = qla2x00_prep_ct_fdmi_req(ha->ct_sns, RPA_CMD, RPA_RSP_SIZE);
 	ct_rsp = &ha->ct_sns->p.rsp;
 
 	/* Prepare FDMI command arguments -- attribute block, attributes. */

@@ -1775,7 +1766,7 @@ qla2x00_gfpn_id(scsi_qla_host_t *vha, sw_info_t
 *list)
 		    GFPN_ID_RSP_SIZE);
 
 		/* Prepare CT request */
-		ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GFPN_ID_CMD,
+		ct_req = qla2x00_prep_ct_req(ha->ct_sns, GFPN_ID_CMD,
 		    GFPN_ID_RSP_SIZE);
 		ct_rsp = &ha->ct_sns->p.rsp;

@@ -1842,18 +1833,17 @@ qla24xx_prep_ms_fm_iocb(scsi_qla_host_t *vha,
 uint32_t req_size,
 
 

static inline struct ct_sns_req *

drivers/scsi/qla2xxx/qla_gs.c +1 -2

-qla24xx_prep_ct_fm_req(struct ct_sns_req *ct_req, uint16_t cmd,
-    uint16_t rsp_size)
+qla24xx_prep_ct_fm_req(struct ct_sns_pkt *p, uint16_t cmd, uint16_t

rsp_size)
{

drivers/scsi/qla2xxx/qla_gs.c +7 -7

-	memset(ct_req, 0, sizeof(struct ct_sns_pkt));
+	memset(p, 0, sizeof(struct ct_sns_pkt));
 
-	ct_req->header.revision = 0x01;
-	ct_req->header.gs_type = 0xFA;
-	ct_req->header.gs_subtype = 0x01;
-	ct_req->command = cpu_to_be16(cmd);
-	ct_req->max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
+	p->p.req.header.revision = 0x01;
+	p->p.req.header.gs_type = 0xFA;
+	p->p.req.header.gs_subtype = 0x01;
+	p->p.req.command = cpu_to_be16(cmd);
+	p->p.req.max_rsp_size = cpu_to_be16((rsp_size - 16) / 4);
 
-	return ct_req;
+	return &p->p.req;

}

/**

drivers/scsi/qla2xxx/qla_gs.c +2 -2

@@ -1889,7 +1879,7 @@ qla2x00_gpsc(scsi_qla_host_t *vha, sw_info_t *list)
 		    GPSC_RSP_SIZE);
 
 		/* Prepare CT request */
-		ct_req = qla24xx_prep_ct_fm_req(&ha->ct_sns->p.req,
+		ct_req = qla24xx_prep_ct_fm_req(ha->ct_sns,
 		    GPSC_CMD, GPSC_RSP_SIZE);
 		ct_rsp = &ha->ct_sns->p.rsp;
 

@@ -2000,7 +1990,7 @@ qla2x00_gff_id(scsi_qla_host_t *vha, sw_info_t
 *list)
 		    GFF_ID_RSP_SIZE);
 
 		/* Prepare CT request */
-		ct_req = qla2x00_prep_ct_req(&ha->ct_sns->p.req, GFF_ID_CMD,
+		ct_req = qla2x00_prep_ct_req(ha->ct_sns, GFF_ID_CMD,
 		    GFF_ID_RSP_SIZE);
 		ct_rsp = &ha->ct_sns->p.rsp;

Signature · 2 lines

-- 
1.7.10.4

Change the 'rval' variable from QLA_FUNCTION_TIMEOUT into QLA_SUCCESS before starting a loop that is only executed if rval is initialized to QLA_SUCCESS. Coverity reported that loop as "dead code".

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_isr.c | 1 +

1 file changed, 1 insertion(+)

drivers/scsi/qla2xxx/qla_isr.c +1 -0

index 259d920..bd0e2fa 100644
@@ -2495,6 +2495,7 @@ qla2xxx_check_risc_status(scsi_qla_host_t *vha)
 	if (rval == QLA_SUCCESS)
 		goto next_test;
 
+	rval = QLA_SUCCESS;
 	WRT_REG_DWORD(&reg->iobase_window, 0x0003);
 	for (cnt = 100; (RD_REG_DWORD(&reg->iobase_window) & BIT_0) == 0 &&
 	    rval == QLA_SUCCESS; cnt--) {

Signature · 1 line

1.7.10.4

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_os.c | 4 +---

1 file changed, 1 insertion(+), 3 deletions(-)

drivers/scsi/qla2xxx/qla_os.c +1 -3

index 1d97626..2176cc9 100644
@@ -2980,14 +2980,12 @@ qla2x00_remove_one(struct pci_dev *pdev)
 	set_bit(UNLOADING, &base_vha->dpc_flags);
 	mutex_lock(&ha->vport_lock);
 	while (ha->cur_vport_count) {
-		struct Scsi_Host *scsi_host;
-
 		spin_lock_irqsave(&ha->vport_slock, flags);
 
 		BUG_ON(base_vha->list.next == &ha->vp_list);
 		/* This assumes first entry in ha->vp_list is always base vha */
 		vha = list_first_entry(&base_vha->list, scsi_qla_host_t, list);
-		scsi_host = scsi_host_get(vha->host);
+		scsi_host_get(vha->host);
 
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
 		mutex_unlock(&ha->vport_lock);

Signature · 1 line

1.7.10.4

Avoid that the fcport structure gets leaked if bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN, the fcport allocation succeeds and the !vha->flags.online branch is taken. This was detected by Coverity. However, Coverity does not recognize that all qla2x00_process_els() callers specify either FC_BSG_RPT_ELS or FC_BSG_HST_ELS_NOLOGIN in the field bsg_job->request->msgcode and that the value of that field is not modified inside that function. This results in a false positive report about a possible memory leak in an error path for bsg_job->request->msgcode values other than the two mentioned values. Make it easy for Coverity (and for humans) to recognize that there is no fcport leak in the error path by changing the bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN test into bsg_job->request->msgcode != FC_BSG_RPT_ELS.

Signed-off-by Bart Van Assche <[email protected]>

Cc Chad Dupuis <[email protected]>

Cc Saurav Kashyap <[email protected]>

---
drivers/scsi/qla2xxx/qla_bsg.c | 14 +++++++-------

1 file changed, 7 insertions(+), 7 deletions(-)

drivers/scsi/qla2xxx/qla_bsg.c +6 -6

index cf07491..f8a2634 100644
@@ -255,6 +255,12 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
 	int rval =  (DRIVER_ERROR << 16);
 	uint16_t nextlid = 0;
 
+	if (!vha->flags.online) {
+		ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
+		rval = -EIO;
+		goto done;
+	}
+
 	if (bsg_job->request->msgcode == FC_BSG_RPT_ELS) {
 		rport = bsg_job->rport;
 		fcport = *(fc_port_t **) rport->dd_data;

@@ -326,12 +332,6 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
 			NPH_FABRIC_CONTROLLER : NPH_F_PORT;
 	}
 
-	if (!vha->flags.online) {
-		ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
-		rval = -EIO;
-		goto done;
-	}
-
 	req_sg_cnt =
 		dma_map_sg(&ha->pdev->dev, bsg_job->request_payload.sg_list,
 		bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);

@@ -399,7 +399,7 @@ done_unmap_sg:
 	goto done_free_fcport;
 

done_free_fcport:

drivers/scsi/qla2xxx/qla_bsg.c +1 -1

-	if (bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN)
+	if (bsg_job->request->msgcode != FC_BSG_RPT_ELS)

		kfree(fcport);
done:
	return rval;

Signature · 1 line

1.7.10.4

The current implementation of the qla2xxx Fibre Channel initiator driver triggers multiple warnings when analyzed with the Coverity static analyzer. This patch series addresses a significant number of these warnings.

Note: patch 10/10 of this series is a reworked version of a patch that had been posted a few weeks ago on the linux-scsi mailing list.

Thanks for the patches. Please share the warnings reported by Coverity, so that its easy for us to review the patches.

Quoted 3 lines

Thanks for the patches. Please share the warnings reported by Coverity, so
that its easy for us to review the patches.

Hello Saurav,

The Coverity warnings that led me to developing this patch series are as follows:
- For patches 1, 2, 3, 8: Logically dead code - execution cannot reach this statement.
- For patch 4: Array compared against 0 (NO_EFFECT) array_null:
Comparing an array to null is not useful: "ha->model_desc".
- For patch 5, 6 and 9: Unused pointer value.
- For patch 7: Overrunning struct type ct_sns_req of 1228 bytes by passing it to a function which accesses it at byte offset 8207.
- For patch 10: Resource leak (RESOURCE_LEAK) leaked_storage: Variable "fcport" going out of scope leaks the storage it points to.

Hope this helps,

Bart.