[U-Boot] [PATCH 2/3] dm: sata: add null pointer check for dev

[Simon Glass]

Hi Marcel,

On Fri, 25 Jan 2019 at 03:30, Marcel Ziswiler <marcel at ziswiler.com> wrote:
>
> From: Marcel Ziswiler <marcel.ziswiler at toradex.com>
>
> Given ahci_get_ops() being a macro not checking anything make sure we
> only call it if we do indeed have a dev pointer.
>
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler at toradex.com>
>
> ---
>
>  drivers/ata/sata.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
> index e384b805b2..4e41f09c87 100644
> --- a/drivers/ata/sata.c
> +++ b/drivers/ata/sata.c
> @@ -20,9 +20,14 @@ struct blk_desc sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
>
>  int sata_reset(struct udevice *dev)
>  {
> -       struct ahci_ops *ops = ahci_get_ops(dev);
> +       struct ahci_ops *ops = NULL;
>
> -       if (!ops->reset)
> +       if (!dev)
> +               return -ENODEV;

This should not happen, i.e. we should not call a DM function with a
NULL pointer. That is the caller's problem.

Similarly with ops. It's OK to check for one of the operations being
NULL, but the operation pointer itself should never be NULL.

These checks add to code side with no benefit in the normal case.

We did talk about checking this in the uclass or in DM core, but I
don't believe a patch was sent.

> +
> +       ops = ahci_get_ops(dev);
> +
> +       if (!ops || !ops->reset)
>                 return -ENOSYS;
>
>         return ops->reset(dev);
> @@ -30,9 +35,14 @@ int sata_reset(struct udevice *dev)
>
[..]

Regards,
Simon