mptfusion: tweak null pointer checks
authorJoe Lawrence <joe.lawrence@stratus.com>
Wed, 25 Jun 2014 21:06:54 +0000 (17:06 -0400)
committerChristoph Hellwig <hch@lst.de>
Fri, 25 Jul 2014 21:16:58 +0000 (17:16 -0400)
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>
drivers/message/fusion/mptbase.c
drivers/message/fusion/mptsas.c
drivers/message/fusion/mptscsih.c

index 9d4c7825a5f2afd0be23833672d4b2dc9158cb33..a896d948b79ee12b36f82c3d04c6c3ec34d9d1b0 100644 (file)
@@ -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);
index b0b74c4d26457805308706393fc6c4b34d2270e3..0707fa2c701b543ef597d4797e77b8272c658ca4 100644 (file)
@@ -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;
index 39e566803089bb2b685dc6eb4ea918e0c620ed26..e7dcb25833697e05a8d00742eac640dfd338f71d 100644 (file)
@@ -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) {