qla4xx: a small loop fix

PATCH In review Tomas Henzl 2 participants 4 messages Nov 1, 2011

Subsystems (2)

SCSI QLogic QLA4XXX iSCSI driver

lore

Share & tools

lore

Tomas Henzl

author

2011-11-01 16:46 UTC #0 lore raw

PATCH 1/1 qla4xx: a small loop fix

From: Tomas Henzl <[email protected]>

When the qla4xxx_get_fwddb_entry returns QLA_ERROR the nex_idx is not updated,

      for (idx = 0; idx < max_ddbs; idx = next_idx) {
                ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
                                              &next_idx, &state, &conn_err,
                                                NULL, NULL);
                if (ret == QLA_ERROR)
                        continue;

This means there is a risk that the 'idx < max_ddbs' condition will never met and the loop will loop forever.
Fix this by explicitly increasing the next_idx in the error condition.

Maybe a break instead of continue is more appropriate, leaving the decision on the qlogic maintainer.

Signed-off-by Tomas Henzl <[email protected]>

drivers/scsi/qla4xxx/ql4_init.c +3 -1

index 3075fba..17acb17 100644
@@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
 		ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
 					      &next_idx, &state, &conn_err,
 						NULL, NULL);
-		if (ret == QLA_ERROR)
+		if (ret == QLA_ERROR) {
+			next_idx++;
 			continue;
+		}
 		if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
 		    state == DDB_DS_SESSION_FAILED) {
 			DEBUG2(ql4_printk(KERN_INFO, ha,

Signature · 1 line

1.7.6.4

Mike Christie

2011-11-01 18:05 UTC #1 lore raw

Quoted 26 lines · Tomas Henzl

From: Tomas Henzl <[email protected]>
When the qla4xxx_get_fwddb_entry returns QLA_ERROR the nex_idx is not updated,
      for (idx = 0; idx < max_ddbs; idx = next_idx) {
                ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
                                              &next_idx, &state, &conn_err,
                                                NULL, NULL);
                if (ret == QLA_ERROR)
                        continue;
This means there is a risk that the 'idx < max_ddbs' condition will never met and the loop will loop forever.
Fix this by explicitly increasing the next_idx in the error condition.
Maybe a break instead of continue is more appropriate, leaving the decision on the qlogic maintainer.
Signed-off-by Tomas Henzl <[email protected]>
drivers/scsi/qla4xxx/ql4_init.c +0 -0
index 3075fba..17acb17 100644
@@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
 		ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,

drivers/scsi/qla4xxx/ql4_init.c static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha) · line 796

Quoted 10 lines

drivers/scsi/qla4xxx/ql4_init.c +3 -1

 					      &next_idx, &state, &conn_err,
 						NULL, NULL);
-		if (ret == QLA_ERROR)
+		if (ret == QLA_ERROR) {
+			next_idx++;
 			continue;
+		}
 		if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
 		    state == DDB_DS_SESSION_FAILED) {
 			DEBUG2(ql4_printk(KERN_INFO, ha,

Patch looks correct.

James, this is a patch for something that is not yet upstream. I will just merge this patch with the patch I was going to send upstream, so do not worry about it.

Tomas Henzl

author

2011-11-01 20:22 UTC #2 lore raw

Quoted 39 lines · Mike Christie

Quoted 26 lines · Tomas Henzl
From: Tomas Henzl <[email protected]>
When the qla4xxx_get_fwddb_entry returns QLA_ERROR the nex_idx is not updated,
      for (idx = 0; idx < max_ddbs; idx = next_idx) {
                ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
                                              &next_idx, &state, &conn_err,
                                                NULL, NULL);
                if (ret == QLA_ERROR)
                        continue;
This means there is a risk that the 'idx < max_ddbs' condition will never met and the loop will loop forever.
Fix this by explicitly increasing the next_idx in the error condition.
Maybe a break instead of continue is more appropriate, leaving the decision on the qlogic maintainer.
Signed-off-by Tomas Henzl <[email protected]>
drivers/scsi/qla4xxx/ql4_init.c +0 -0
index 3075fba..17acb17 100644
@@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
 		ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
drivers/scsi/qla4xxx/ql4_init.c static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha) · line 796
Quoted 10 lines
drivers/scsi/qla4xxx/ql4_init.c +3 -1
 					      &next_idx, &state, &conn_err,
 						NULL, NULL);
-		if (ret == QLA_ERROR)
+		if (ret == QLA_ERROR) {
+			next_idx++;
 			continue;
+		}
 		if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
 		    state == DDB_DS_SESSION_FAILED) {
 			DEBUG2(ql4_printk(KERN_INFO, ha,
Patch looks correct.

Quoted 3 lines

James, this is a patch for something that is not yet upstream. I will just merge this patch with the patch I was going to send upstream, so do not worry about it.

It's created on top of scsi-misc (99a700bcc75429ba84a672d04f0b650dcc5b3042 [SCSI] mv_sas: OCZ RevoDrive3 & zDrive R4 support) it looks like being created on top of the not yet posted patch, but it isn't so :)
To the method how this gets accepted i don't care.

Quoted 4 lines

--
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

Mike Christie

Reviewed-by

2011-11-01 20:26 UTC #3 lore raw

Quoted 40 lines · Tomas Henzl

Quoted 39 lines · Mike Christie
Quoted 26 lines · Tomas Henzl
From: Tomas Henzl <[email protected]>
When the qla4xxx_get_fwddb_entry returns QLA_ERROR the nex_idx is not updated,
      for (idx = 0; idx < max_ddbs; idx = next_idx) {
                ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
                                              &next_idx, &state, &conn_err,
                                                NULL, NULL);
                if (ret == QLA_ERROR)
                        continue;
This means there is a risk that the 'idx < max_ddbs' condition will never met and the loop will loop forever.
Fix this by explicitly increasing the next_idx in the error condition.
Maybe a break instead of continue is more appropriate, leaving the decision on the qlogic maintainer.
Signed-off-by Tomas Henzl <[email protected]>
drivers/scsi/qla4xxx/ql4_init.c +0 -0
index 3075fba..17acb17 100644
@@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
 		ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
drivers/scsi/qla4xxx/ql4_init.c static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha) · line 796
Quoted 10 lines
drivers/scsi/qla4xxx/ql4_init.c +3 -1
 					      &next_idx, &state, &conn_err,
 						NULL, NULL);
-		if (ret == QLA_ERROR)
+		if (ret == QLA_ERROR) {
+			next_idx++;
 			continue;
+		}
 		if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
 		    state == DDB_DS_SESSION_FAILED) {
 			DEBUG2(ql4_printk(KERN_INFO, ha,
Patch looks correct.

Quoted 7 lines

Quoted 3 lines
James, this is a patch for something that is not yet upstream. I will just merge this patch with the patch I was going to send upstream, so do not worry about it.
It's created on top of scsi-misc (99a700bcc75429ba84a672d04f0b650dcc5b3042 [SCSI] mv_sas: OCZ RevoDrive3 & zDrive R4 support) it looks like being created on top of the not yet posted patch, but it isn't so :)
To the method how this gets accepted i don't care.

Ah yeah, you are right. Go ahead and take it.

Reviewed-by Mike Christie <[email protected]>