From: Roel Kluin <roel.kluin@gmail.com>
To: abjoglek@cisco.com, jeykholt@cisco.com, linux-scsi@vger.kernel.org
Subject: NULL check after starget_to_rport()?
Date: Wed, 15 Jul 2009 23:12:40 +0200 [thread overview]
Message-ID: <4A5E4648.5020006@gmail.com> (raw)
// vi include/scsi/scsi_transport_fc.h +396
#define starget_to_rport(s) \
scsi_is_fc_rport(s->dev.parent) ? dev_to_rport(s->dev.parent) : NULL
So shouldn't we test for this NULL value (something like the untested patch
below?)
Also I had two more questions concerning this file:
In fnic_clean_pending_aborts(), rport isn't used after the starget_to_rport().
is the rport needed?
Should fnic_block_error_handler() return an error value if rport is NULL?
Roel
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index bfc9969..8a6bb3e 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -346,6 +346,9 @@ int fnic_queuecommand(struct scsi_cmnd *sc, void (*done)(struct scsi_cmnd *))
unsigned long ptr;
rport = starget_to_rport(scsi_target(sc->device));
+ if (rport == NULL)
+ return 0;
+
ret = fc_remote_port_chkready(rport);
if (ret) {
sc->result = ret;
@@ -1230,6 +1233,9 @@ static void fnic_block_error_handler(struct scsi_cmnd *sc)
struct fc_rport *rport = starget_to_rport(scsi_target(sc->device));
unsigned long flags;
+ if (rport == NULL)
+ return;
+
spin_lock_irqsave(shost->host_lock, flags);
while (rport->port_state == FC_PORTSTATE_BLOCKED) {
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -1265,11 +1271,12 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
lp = shost_priv(sc->device->host);
fnic = lport_priv(lp);
- FNIC_SCSI_DBG(KERN_DEBUG,
- fnic->lport->host,
- "Abort Cmd called FCID 0x%x, LUN 0x%x TAG %d\n",
- (starget_to_rport(scsi_target(sc->device)))->port_id,
- sc->device->lun, sc->request->tag);
+ rport = starget_to_rport(scsi_target(sc->device));
+ if (rport != NULL)
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "Abort Cmd called FCID 0x%x, LUN 0x%x TAG %d\n",
+ rport->port_id, sc->device->lun,
+ sc->request->tag);
if (lp->state != LPORT_ST_READY || !(lp->link_up)) {
ret = FAILED;
@@ -1319,6 +1326,9 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
* the IO. Else, just locally terminate the IO in the firmware
*/
rport = starget_to_rport(scsi_target(sc->device));
+ if (rport == NULL)
+ goto fnic_abort_cmd_end;
+
if (fc_remote_port_chkready(rport) == 0)
task_req = FCPIO_ITMF_ABT_TASK;
else
@@ -1478,6 +1488,8 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
/* Now queue the abort command to firmware */
int_to_scsilun(sc->device->lun, &fc_lun);
rport = starget_to_rport(scsi_target(sc->device));
+ if (rport == NULL)
+ goto clean_pending_aborts_end;
if (fnic_queue_abort_io_req(fnic, tag,
FCPIO_ITMF_ABT_TASK_TERM,
@@ -1547,19 +1559,19 @@ int fnic_device_reset(struct scsi_cmnd *sc)
lp = shost_priv(sc->device->host);
fnic = lport_priv(lp);
- FNIC_SCSI_DBG(KERN_DEBUG,
- fnic->lport->host,
- "Device reset called FCID 0x%x, LUN 0x%x\n",
- (starget_to_rport(scsi_target(sc->device)))->port_id,
- sc->device->lun);
+ rport = starget_to_rport(scsi_target(sc->device));
+ if (rport != NULL)
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "Device reset called FCID 0x%x, LUN 0x%x\n",
+ rport->port_id, sc->device->lun);
if (lp->state != LPORT_ST_READY || !(lp->link_up))
goto fnic_device_reset_end;
/* Check if remote port up */
rport = starget_to_rport(scsi_target(sc->device));
- if (fc_remote_port_chkready(rport))
+ if (rport == NULL || fc_remote_port_chkready(rport))
goto fnic_device_reset_end;
io_lock = fnic_io_lock_hash(fnic, sc);
next reply other threads:[~2009-07-15 21:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-15 21:12 Roel Kluin [this message]
2009-07-16 8:28 ` NULL check after starget_to_rport()? Abhijeet Joglekar (abjoglek)
2009-07-16 11:23 ` [PATCH] fnic: clean up Roel Kluin
2009-07-17 17:14 ` Abhijeet Joglekar (abjoglek)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A5E4648.5020006@gmail.com \
--to=roel.kluin@gmail.com \
--cc=abjoglek@cisco.com \
--cc=jeykholt@cisco.com \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.