* [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
@ 2013-06-05 13:02 ` Bart Van Assche
2013-06-12 8:05 ` Saurav Kashyap
2013-06-05 13:03 ` [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd() Bart Van Assche
` (9 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:02 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
Remove dead code and simplify a pointer computation.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 39719f8..7221521 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -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");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma()
2013-06-05 13:02 ` [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma() Bart Van Assche
@ 2013-06-12 8:05 ` Saurav Kashyap
2013-06-12 18:59 ` James Bottomley
0 siblings, 1 reply; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:05 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]
Hi Bart,
Missing msg ids needs to be updated in qla_dbg.c file. Here is an updated
patch
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 89dd050..25ded57 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -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");
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c
b/drivers/scsi/qla2xxx/qla_dbg.c
index 94f3a25..980dcbc 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -35,7 +35,8 @@
* | | | 0x70a5,0x70a6, |
* | | | 0x70a8,0x70ab, |
* | | | 0x70ad-0x70ae, |
- * | | | 0x70d1-0x70da |
+ * | | | 0x70d1-0x70da, |
+ * | | | 0x7047
|
* | Task Management | 0x803c | 0x8025-0x8026 |
* | | | 0x800b,0x8039 |
* | AER/EEH | 0x9011 | |
Thanks,
~Saurav
>Remove dead code and simplify a pointer computation.
>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
>b/drivers/scsi/qla2xxx/qla_bsg.c
>index 39719f8..7221521 100644
>--- a/drivers/scsi/qla2xxx/qla_bsg.c
>+++ b/drivers/scsi/qla2xxx/qla_bsg.c
>@@ -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");
>--
>1.7.10.4
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4636 bytes --]
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma()
2013-06-12 8:05 ` Saurav Kashyap
@ 2013-06-12 18:59 ` James Bottomley
2013-06-13 5:20 ` Saurav Kashyap
0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2013-06-12 18:59 UTC (permalink / raw)
To: Saurav Kashyap; +Cc: Bart Van Assche, linux-scsi, Chad Dupuis
On Wed, 2013-06-12 at 08:05 +0000, Saurav Kashyap wrote:
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma()
2013-06-12 18:59 ` James Bottomley
@ 2013-06-13 5:20 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-13 5:20 UTC (permalink / raw)
To: James Bottomley; +Cc: Bart Van Assche, linux-scsi, Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 430 bytes --]
>On Wed, 2013-06-12 at 08:05 +0000, Saurav Kashyap wrote:
>> 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).
<SK> Sure, I will send all the patches in next scsi-misc submission.
Thanks,
~Saurav
>
>Thanks,
>
>James
>
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 3800 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
2013-06-05 13:02 ` [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma() Bart Van Assche
@ 2013-06-05 13:03 ` Bart Van Assche
2013-06-12 8:06 ` Saurav Kashyap
2013-06-05 13:04 ` [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba() Bart Van Assche
` (8 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:03 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 7221521..cf07491 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -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:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd()
2013-06-05 13:03 ` [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd() Bart Van Assche
@ 2013-06-12 8:06 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:06 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 3645 bytes --]
HI Bart,
Missing msg ids need to be updated on qla_dbg.c file. Here is an updated
patch.
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 25ded57..9520b1f 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -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,
@@ -1102,7 +1094,7 @@ qla84xx_mgmt_cmd(struct fc_bsg_job *bsg_job)
memset(mn, 0, sizeof(struct access_chip_84xx));
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:
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c
b/drivers/scsi/qla2xxx/qla_dbg.c
index 980dcbc..1c1972b 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -36,7 +36,7 @@
* | | | 0x70a8,0x70ab, |
* | | | 0x70ad-0x70ae, |
* | | | 0x70d1-0x70da, |
- * | | | 0x7047
|
+ * | | | 0x7047,0x703b |
* | Task Management | 0x803c | 0x8025-0x8026 |
* | | | 0x800b,0x8039 |
* | AER/EEH | 0x9011 | |
Thanks,
~Saurav
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_bsg.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
>b/drivers/scsi/qla2xxx/qla_bsg.c
>index 7221521..cf07491 100644
>--- a/drivers/scsi/qla2xxx/qla_bsg.c
>+++ b/drivers/scsi/qla2xxx/qla_bsg.c
>@@ -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:
>--
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 5047 bytes --]
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
2013-06-05 13:02 ` [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma() Bart Van Assche
2013-06-05 13:03 ` [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd() Bart Van Assche
@ 2013-06-05 13:04 ` Bart Van Assche
2013-06-12 8:08 ` Saurav Kashyap
2013-06-05 13:05 ` [PATCH 04/10] qla2xxx: Remove two superfluous tests Bart Van Assche
` (7 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:04 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_init.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 3565dfd..c68bb01 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -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);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba()
2013-06-05 13:04 ` [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba() Bart Van Assche
@ 2013-06-12 8:08 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:08 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]
Hi Bart,
I removed the success message also and updated the qla_dbg.c file.
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c
b/drivers/scsi/qla2xxx/qla_dbg.c
index 1c1972b..dc227fa 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -15,6 +15,7 @@
* | Mailbox commands | 0x117a | 0x111a-0x111b |
* | | | 0x1155-0x1158 |
* | Device Discovery | 0x2095 | 0x2020-0x2022, |
+ * | | | 0x2011-0x2012, |
* | | | 0x2016 |
* | Queue Command and IO tracing | 0x3058 | 0x3006-0x300b |
* | | | 0x3027-0x3028 |
diff --git a/drivers/scsi/qla2xxx/qla_init.c
b/drivers/scsi/qla2xxx/qla_init.c
index 8c7a123..c4fc8a2 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2320,14 +2320,6 @@ 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__);
- }
-
return(rval);
}
Thanks,
~Saurav
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_init.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_init.c
>b/drivers/scsi/qla2xxx/qla_init.c
>index 3565dfd..c68bb01 100644
>--- a/drivers/scsi/qla2xxx/qla_init.c
>+++ b/drivers/scsi/qla2xxx/qla_init.c
>@@ -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);
> }
>--
>1.7.10.4
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4683 bytes --]
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 04/10] qla2xxx: Remove two superfluous tests
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (2 preceding siblings ...)
2013-06-05 13:04 ` [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba() Bart Van Assche
@ 2013-06-05 13:05 ` Bart Van Assche
2013-06-12 8:08 ` Saurav Kashyap
2013-06-05 13:06 ` [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs() Bart Van Assche
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:05 UTC (permalink / raw)
Cc: linux-scsi, Chad Dupuis, Saurav Kashyap
Since ha->model_desc is an array comparing it against NULL is
superfluous. Hence remove these tests.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_gs.c | 3 +--
drivers/scsi/qla2xxx/qla_os.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index d0ea8b9..f26442a 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -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);
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index ad72c1d..1d97626 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -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),
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 04/10] qla2xxx: Remove two superfluous tests
2013-06-05 13:05 ` [PATCH 04/10] qla2xxx: Remove two superfluous tests Bart Van Assche
@ 2013-06-12 8:08 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:08 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-scsi, Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>Since ha->model_desc is an array comparing it against NULL is
>superfluous. Hence remove these tests.
>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_gs.c | 3 +--
> drivers/scsi/qla2xxx/qla_os.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
>index d0ea8b9..f26442a 100644
>--- a/drivers/scsi/qla2xxx/qla_gs.c
>+++ b/drivers/scsi/qla2xxx/qla_gs.c
>@@ -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);
>diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>index ad72c1d..1d97626 100644
>--- a/drivers/scsi/qla2xxx/qla_os.c
>+++ b/drivers/scsi/qla2xxx/qla_os.c
>@@ -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),
>--
>1.7.10.4
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4428 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (3 preceding siblings ...)
2013-06-05 13:05 ` [PATCH 04/10] qla2xxx: Remove two superfluous tests Bart Van Assche
@ 2013-06-05 13:06 ` Bart Van Assche
2013-06-12 8:08 ` Saurav Kashyap
2013-06-05 13:06 ` [PATCH 06/10] qla2xxx: Remove redundant assignments Bart Van Assche
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:06 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_iocb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 15e4080..b589d24 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -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;
@@ -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;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs()
2013-06-05 13:06 ` [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs() Bart Van Assche
@ 2013-06-12 8:08 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:08 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_iocb.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_iocb.c
>b/drivers/scsi/qla2xxx/qla_iocb.c
>index 15e4080..b589d24 100644
>--- a/drivers/scsi/qla2xxx/qla_iocb.c
>+++ b/drivers/scsi/qla2xxx/qla_iocb.c
>@@ -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;
>@@ -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;
>--
>1.7.10.4
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4472 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 06/10] qla2xxx: Remove redundant assignments
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (4 preceding siblings ...)
2013-06-05 13:06 ` [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs() Bart Van Assche
@ 2013-06-05 13:06 ` Bart Van Assche
2013-06-12 8:09 ` Saurav Kashyap
2013-06-05 13:07 ` [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization Bart Van Assche
` (4 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:06 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_dbg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index cfa2a20..c612785 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -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);
@@ -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);
@@ -2289,7 +2289,7 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked)
copy_queue:
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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 06/10] qla2xxx: Remove redundant assignments
2013-06-05 13:06 ` [PATCH 06/10] qla2xxx: Remove redundant assignments Bart Van Assche
@ 2013-06-12 8:09 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:09 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_dbg.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_dbg.c
>b/drivers/scsi/qla2xxx/qla_dbg.c
>index cfa2a20..c612785 100644
>--- a/drivers/scsi/qla2xxx/qla_dbg.c
>+++ b/drivers/scsi/qla2xxx/qla_dbg.c
>@@ -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);
>@@ -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);
>@@ -2289,7 +2289,7 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int
>hardware_locked)
> copy_queue:
> 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);
>--
>1.7.10.4
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4196 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (5 preceding siblings ...)
2013-06-05 13:06 ` [PATCH 06/10] qla2xxx: Remove redundant assignments Bart Van Assche
@ 2013-06-05 13:07 ` Bart Van Assche
2013-06-12 8:09 ` Saurav Kashyap
2013-06-05 13:08 ` [PATCH 08/10] qla2xxx: Fix qla2xxx_check_risc_status() Bart Van Assche
` (3 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:07 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_gs.c | 86 ++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index f26442a..1ad361b 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -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)
{
- 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
@@ -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 */
@@ -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 *
-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)
{
- 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;
}
/**
@@ -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 *
-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)
{
- 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;
}
/**
@@ -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;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization
2013-06-05 13:07 ` [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization Bart Van Assche
@ 2013-06-12 8:09 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:09 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 9022 bytes --]
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_gs.c | 86
>++++++++++++++++++-----------------------
> 1 file changed, 38 insertions(+), 48 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
>index f26442a..1ad361b 100644
>--- a/drivers/scsi/qla2xxx/qla_gs.c
>+++ b/drivers/scsi/qla2xxx/qla_gs.c
>@@ -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)
> {
>- 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
>@@ -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 */
>@@ -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 *
>-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)
> {
>- 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;
> }
>
> /**
>@@ -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 *
>-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)
> {
>- 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;
> }
>
> /**
>@@ -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;
>
>--
>1.7.10.4
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 6494 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 08/10] qla2xxx: Fix qla2xxx_check_risc_status()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (6 preceding siblings ...)
2013-06-05 13:07 ` [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization Bart Van Assche
@ 2013-06-05 13:08 ` Bart Van Assche
2013-06-12 8:10 ` Saurav Kashyap
2013-06-05 13:09 ` [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one() Bart Van Assche
` (2 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:08 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_isr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 259d920..bd0e2fa 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -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(®->iobase_window, 0x0003);
for (cnt = 100; (RD_REG_DWORD(®->iobase_window) & BIT_0) == 0 &&
rval == QLA_SUCCESS; cnt--) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (7 preceding siblings ...)
2013-06-05 13:08 ` [PATCH 08/10] qla2xxx: Fix qla2xxx_check_risc_status() Bart Van Assche
@ 2013-06-05 13:09 ` Bart Van Assche
2013-06-12 8:10 ` Saurav Kashyap
2013-06-05 13:09 ` [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els() Bart Van Assche
2013-06-07 19:06 ` [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Saurav Kashyap
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:09 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_os.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 1d97626..2176cc9 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one()
2013-06-05 13:09 ` [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one() Bart Van Assche
@ 2013-06-12 8:10 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:10 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_os.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>index 1d97626..2176cc9 100644
>--- a/drivers/scsi/qla2xxx/qla_os.c
>+++ b/drivers/scsi/qla2xxx/qla_os.c
>@@ -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);
>--
>1.7.10.4
>
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4255 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (8 preceding siblings ...)
2013-06-05 13:09 ` [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one() Bart Van Assche
@ 2013-06-05 13:09 ` Bart Van Assche
2013-06-12 8:11 ` Saurav Kashyap
2013-06-07 19:06 ` [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Saurav Kashyap
10 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-05 13:09 UTC (permalink / raw)
To: linux-scsi; +Cc: Chad Dupuis, Saurav Kashyap
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 <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
drivers/scsi/qla2xxx/qla_bsg.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index cf07491..f8a2634 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -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:
- if (bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN)
+ if (bsg_job->request->msgcode != FC_BSG_RPT_ELS)
kfree(fcport);
done:
return rval;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()
2013-06-05 13:09 ` [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els() Bart Van Assche
@ 2013-06-12 8:11 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:11 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 4332 bytes --]
Hi Bart,
In this case online check is move to far, the vha is still not
dereferenced. The right patch is moving online flag just after getting vha.
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 9520b1f..11f84dc 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -269,6 +269,12 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
type = "FC_BSG_HST_ELS_NOLOGIN";
}
+ if (!vha->flags.online) {
+ ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
+ rval = -EIO;
+ goto done;
+ }
+
/* pass through is supported only for ISP 4Gb or higher */
if (!IS_FWI2_CAPABLE(ha)) {
ql_dbg(ql_dbg_user, vha, 0x7001,
@@ -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:
- if (bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN)
+ if (bsg_job->request->msgcode == FC_BSG_RPT_ELS)
kfree(fcport);
done:
return rval;
Thanks,
~Saurav
-----Original Message-----
From: Bart Van Assche <bvanassche@acm.org>
Date: Wed, 5 Jun 2013 15:09:59 +0200
To: linux-scsi <linux-scsi@vger.kernel.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>, Saurav Kashyap
<saurav.kashyap@qlogic.com>
Subject: [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of
qla2x00_process_els()
>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 <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_bsg.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
>b/drivers/scsi/qla2xxx/qla_bsg.c
>index cf07491..f8a2634 100644
>--- a/drivers/scsi/qla2xxx/qla_bsg.c
>+++ b/drivers/scsi/qla2xxx/qla_bsg.c
>@@ -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:
>- if (bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN)
>+ if (bsg_job->request->msgcode != FC_BSG_RPT_ELS)
> kfree(fcport);
> done:
> return rval;
>--
>1.7.10.4
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 5635 bytes --]
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
` (9 preceding siblings ...)
2013-06-05 13:09 ` [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els() Bart Van Assche
@ 2013-06-07 19:06 ` Saurav Kashyap
2013-06-08 8:27 ` Bart Van Assche
10 siblings, 1 reply; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-07 19:06 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi, Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 527 bytes --]
Hi Bart,
Thanks for the patches. Please share the warnings reported by Coverity, so
that its easy for us to review the patches.
Thanks,
~Saurav
>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.
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 3885 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings
2013-06-07 19:06 ` [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Saurav Kashyap
@ 2013-06-08 8:27 ` Bart Van Assche
2013-06-12 8:03 ` Saurav Kashyap
0 siblings, 1 reply; 26+ messages in thread
From: Bart Van Assche @ 2013-06-08 8:27 UTC (permalink / raw)
To: Saurav Kashyap; +Cc: linux-scsi, Chad Dupuis
On 06/07/13 21:06, Saurav Kashyap wrote:
> 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.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings
2013-06-08 8:27 ` Bart Van Assche
@ 2013-06-12 8:03 ` Saurav Kashyap
0 siblings, 0 replies; 26+ messages in thread
From: Saurav Kashyap @ 2013-06-12 8:03 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-scsi, Chad Dupuis
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
Hi Bart,
Thanks for patchset. There are minor issue with some patches like updating
the missing msg_ids in qla_dbg.c file. I am replying to each of the patch.
Thanks,
~Saurav
>On 06/07/13 21:06, Saurav Kashyap wrote:
>> 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.
>
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4277 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread