Re: [PATCH 7/7] mptfusion: tweak null pointer checks

[Christoph Hellwig <hch@infradead.org>]

> 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.