> JL: Comments on the above warnings, in order: Can you move these comments into the actual commit message instead of the part that gets discarded? > > No-brainer, the enclosing switch statement dereferences 'reply', > so we can't get here unless 'reply' is valid. ok. > "Retry target reset" needs to know the target ID and channel, so > enclose that entire block inside a if (pScsiTmReply) block. 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? > The code before the loop checks for 'port_info->phy_info', but not > many other places in the driver bother. Not sure what to do here. 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. > 'hostdata' is checked and then immediately dereferenced after > that block. How about a default return string of "N/A" to indicate > one isn't available? shost_priv can't return NULL, so the if (h) should be removed. > Not sure what to do here. 'vdevice' is needed to "set up the > message frame" target ID and channel, so it should probably return > an error in in this case. vdevice can't ever be NULL here, it's allocated in ->slave_alloc and thus guaranteed to be around when ->queuecommand is called. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html