diff options
author | Joe Lawrence <joe.lawrence@stratus.com> | 2014-06-25 23:06:54 +0200 |
---|---|---|
committer | Christoph Hellwig <hch@lst.de> | 2014-07-25 23:16:58 +0200 |
commit | 9f21316fc2f297efd32b40a57083d5cecb4bda26 (patch) | |
tree | 1174b29f2f5d6907ede37373227683ff6876c66c /drivers/message/fusion/mptsas.c | |
parent | mptfusion: combine fw_event_work and its event_data (diff) | |
download | linux-9f21316fc2f297efd32b40a57083d5cecb4bda26.tar.xz linux-9f21316fc2f297efd32b40a57083d5cecb4bda26.zip |
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>
Diffstat (limited to 'drivers/message/fusion/mptsas.c')
-rw-r--r-- | drivers/message/fusion/mptsas.c | 52 |
1 files changed, 25 insertions, 27 deletions
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; |