summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoe Lawrence <joe.lawrence@stratus.com>2014-06-25 17:06:54 -0400
committerChristoph Hellwig <hch@lst.de>2014-07-25 17:16:58 -0400
commit9f21316fc2f297efd32b40a57083d5cecb4bda26 (patch)
tree1174b29f2f5d6907ede37373227683ff6876c66c
parent32696198745a74b37bc27498a5ade169d2c04c5e (diff)
downloadlinux-9f21316fc2f297efd32b40a57083d5cecb4bda26.tar.bz2
mptfusion: tweak null pointer checks
Fixes the following smatch warnings: drivers/message/fusion/mptbase.c:652 mptbase_reply() warn: variable dereferenced before check 'reply' (see line 639) [JL: No-brainer, the enclosing switch statement dereferences reply, so we can't get here unless reply is valid.] drivers/message/fusion/mptsas.c:1255 mptsas_taskmgmt_complete() error: we previously assumed 'pScsiTmReply' could be null (see line 1227) [HCH: Reading the code in mptsas_taskmgmt_complete it's pretty obvious that it can't do anything useful if mr/pScsiTmReply are NULL, so I suspect it would be best to just return at the beginning of the function. I'd love to understand if it actually could ever be zero, which I doubt. Maybe the LSI people can shed some light on that?] drivers/message/fusion/mptsas.c:3888 mptsas_not_responding_devices() error: we previously assumed 'port_info->phy_info' could be null (see line 3875) [HCH: It's pretty obvious from reading mptsas_sas_io_unit_pg0 that we never register a port_info with a NULL phy_info in the lists, so all NULL checks on it could be deleted.] drivers/message/fusion/mptscsih.c:1284 mptscsih_info() error: we previously assumed 'h' could be null (see line 1274) [HCH: shost_priv can't return NULL, so the if (h) should be removed.] drivers/message/fusion/mptscsih.c:1388 mptscsih_qcmd() error: we previously assumed 'vdevice' could be null (see line 1373) [HCH: vdevice can't ever be NULL here, it's allocated in ->slave_alloc and thus guaranteed to be around when ->queuecommand is called.] Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com> Acked-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
-rw-r--r--drivers/message/fusion/mptbase.c10
-rw-r--r--drivers/message/fusion/mptsas.c52
-rw-r--r--drivers/message/fusion/mptscsih.c19
3 files changed, 37 insertions, 44 deletions
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 9d4c7825a5f2..a896d948b79e 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -649,12 +649,10 @@ mptbase_reply(MPT_ADAPTER *ioc, MPT_FRAME_HDR *req, MPT_FRAME_HDR *reply)
case MPI_FUNCTION_CONFIG:
case MPI_FUNCTION_SAS_IO_UNIT_CONTROL:
ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
- if (reply) {
- ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
- memcpy(ioc->mptbase_cmds.reply, reply,
- min(MPT_DEFAULT_FRAME_SIZE,
- 4 * reply->u.reply.MsgLength));
- }
+ ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
+ memcpy(ioc->mptbase_cmds.reply, reply,
+ min(MPT_DEFAULT_FRAME_SIZE,
+ 4 * reply->u.reply.MsgLength));
if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) {
ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING;
complete(&ioc->mptbase_cmds.done);
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index b0b74c4d2645..0707fa2c701b 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1203,27 +1203,28 @@ mptsas_taskmgmt_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *mr)
"(mf = %p, mr = %p)\n", ioc->name, mf, mr));
pScsiTmReply = (SCSITaskMgmtReply_t *)mr;
- if (pScsiTmReply) {
- dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
- "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n"
- "\ttask_type = 0x%02X, iocstatus = 0x%04X "
- "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, "
- "term_cmnds = %d\n", ioc->name,
- pScsiTmReply->Bus, pScsiTmReply->TargetID,
- pScsiTmReply->TaskType,
- le16_to_cpu(pScsiTmReply->IOCStatus),
- le32_to_cpu(pScsiTmReply->IOCLogInfo),
- pScsiTmReply->ResponseCode,
- le32_to_cpu(pScsiTmReply->TerminationCount)));
-
- if (pScsiTmReply->ResponseCode)
- mptscsih_taskmgmt_response_code(ioc,
- pScsiTmReply->ResponseCode);
- }
-
- if (pScsiTmReply && (pScsiTmReply->TaskType ==
+ if (!pScsiTmReply)
+ return 0;
+
+ dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
+ "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n"
+ "\ttask_type = 0x%02X, iocstatus = 0x%04X "
+ "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, "
+ "term_cmnds = %d\n", ioc->name,
+ pScsiTmReply->Bus, pScsiTmReply->TargetID,
+ pScsiTmReply->TaskType,
+ le16_to_cpu(pScsiTmReply->IOCStatus),
+ le32_to_cpu(pScsiTmReply->IOCLogInfo),
+ pScsiTmReply->ResponseCode,
+ le32_to_cpu(pScsiTmReply->TerminationCount)));
+
+ if (pScsiTmReply->ResponseCode)
+ mptscsih_taskmgmt_response_code(ioc,
+ pScsiTmReply->ResponseCode);
+
+ if (pScsiTmReply->TaskType ==
MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType ==
- MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET)) {
+ MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET) {
ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
memcpy(ioc->taskmgmt_cmds.reply, mr,
@@ -3853,10 +3854,8 @@ retry_page:
phy_info = mptsas_find_phyinfo_by_sas_address(ioc,
sas_info->sas_address);
- if (phy_info) {
- mptsas_del_end_device(ioc, phy_info);
- goto redo_device_scan;
- }
+ mptsas_del_end_device(ioc, phy_info);
+ goto redo_device_scan;
} else
mptsas_volume_delete(ioc, sas_info->fw.id);
}
@@ -3867,9 +3866,8 @@ retry_page:
redo_expander_scan:
list_for_each_entry(port_info, &ioc->sas_topology, list) {
- if (port_info->phy_info &&
- (!(port_info->phy_info[0].identify.device_info &
- MPI_SAS_DEVICE_INFO_SMP_TARGET)))
+ if (!(port_info->phy_info[0].identify.device_info &
+ MPI_SAS_DEVICE_INFO_SMP_TARGET))
continue;
found_expander = 0;
handle = 0xFFFF;
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index 39e566803089..e7dcb2583369 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1271,15 +1271,13 @@ mptscsih_info(struct Scsi_Host *SChost)
h = shost_priv(SChost);
- if (h) {
- if (h->info_kbuf == NULL)
- if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL)
- return h->info_kbuf;
- h->info_kbuf[0] = '\0';
-
- mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0);
- h->info_kbuf[size-1] = '\0';
- }
+ if (h->info_kbuf == NULL)
+ if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL)
+ return h->info_kbuf;
+ h->info_kbuf[0] = '\0';
+
+ mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0);
+ h->info_kbuf[size-1] = '\0';
return h->info_kbuf;
}
@@ -1368,8 +1366,7 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt)
/* Default to untagged. Once a target structure has been allocated,
* use the Inquiry data to determine if device supports tagged.
*/
- if (vdevice
- && (vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
+ if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
&& (SCpnt->device->tagged_supported)) {
scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ;
if (SCpnt->request && SCpnt->request->ioprio) {