On Wed, Sep 26, 2018 at 09:52:36AM +0530, Suganath Prabu S wrote:
> The code for getting shost and IOC is redundant so
> moved that to function "scsih_get_shost_and_ioc".
> Also checks for NULL are added to IOC and shost.
>
> Signed-off-by: Suganath Prabu S <suganath-prabu.subram...@broadcom.com>
> ---
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 98
> ++++++++++++++++++++++++++++++------
> 1 file changed, 82 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 566a550..f6e92eb 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -9809,6 +9809,35 @@ _scsih_ir_shutdown(struct MPT3SAS_ADAPTER *ioc)
> }
>
> /**
> + * _scsih_get_shost_and_ioc - get shost and ioc
> + * and verify whether they are NULL or not
> + * @pdev: PCI device struct
> + * @shost: address of scsi host pointer
> + * @ioc: address of HBA adapter pointer
> + *
> + * Return zero if *shost and *ioc are not NULL otherwise return error number.
> + */
> +static int
> +_scsih_get_shost_and_ioc(struct pci_dev *pdev,
> + struct Scsi_Host **shost, struct MPT3SAS_ADAPTER **ioc)
> +{
> + *shost = pci_get_drvdata(pdev);
> + if (*shost == NULL) {
> + dev_err(&pdev->dev, "pdev's driver data is null\n");
> + return -ENXIO;
> + }
> +
> + *ioc = shost_priv(*shost);
> + if (*ioc == NULL) {
> + dev_err(&pdev->dev, "shost's private data is null\n");
> + return -ENXIO;
I think it's better to omit NULL pointer checks like these because
there should not be a path where we can execute this code when these
pointers are NULL.
If there *is* such a path, I think that's a serious bug and it's
better to oops when we try to dereference the NULL pointer. If we
just return an error code, it's likely the bug will be ignored and
never fixed.
Bjorn
