Re: [PATCH v3 2/5] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, 2018-02-08 at 16:59 +0100, Rafael J. Wysocki wrote:
> On Wed, Feb 7, 2018 at 3:56 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > When __acpi_match_device() is called it would be possible to have
> > ACPI ID table a MULL pointer. To avoid potential dereference,
> > check for this before traverse.
> > 
> > While here, remove redundant 'else'.
> > 
> > 

> > +               if (ids) {
> > +                       for (id = ids; id->id[0] || id->cls; id++) {
> > +                               if (id->id[0] && !strcmp((char *)id-
> > >id, hwid->id))
> > +                                       return id;
> > +                               if (id->cls &&
> > __acpi_match_device_cls(id, hwid))
> > +                                       return id;
> > +                       }
> > 
> 

> The return value below should be updated in *this* patch, because this
> is what allows ids to be NULL in the first place.

OK, so I'll fold it into patch 1 then.

> And as far as I'm concerned you can do:
> 
> if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)
>         return (const struct acpi_device_id
> *)acpi_of_match_device(device, of_ids);
> 
> and update the comment accordingly.

But it's still a trick.

Okay, what comes to my mind (yes, not so simple, but cleaner I suppose)
is to define

struct acpi_of_device_id {
  struct acpi_device_id *acpi_id;
  struct of_device_id *of_id;
};

Add a new parameter to acpi_of_match_device(..., struct
acpi_of_device_id *id).


Update __acpi_match_device() in the similar way.

Update callers.

It looks to me much more cleaner.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy