Re: [PATCH v3 1/5] ACPI / bus: Return error code from __acpi_match_device() in one case

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

On Thu, 2018-02-08 at 16:48 +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 8, 2018 at 4:44 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, 2018-02-08 at 16:14 +0100, Rafael J. Wysocki wrote:
> > > 

> > 
> > > Also the return value here means "success", so why is an error the
> > > right choice?
> > 
> > Because we need to return something which is not NULL. Naturally
> > feels
> > the error code, esp. ENODATA, is quite suitable. We indeed have no
> > data
> > in this case, and it's not a NULL case (not found / not match) — we
> > have
> > a match.
> 
> But this is an error code that means "success".  May I call it rather
> confusing?

This function AFAICS does two things at once:
- matches device against ID
- returns matched ID entry in the table

Return value combines those two into actually ternary option:
- no match
- match with ID
- match without ID

> > > Overall, this really looks like a preparation for a future patch,
> > > so
> > > why not just say that straight away in the changelog?
> > 
> > It's not _just_ a preparation, it mitigates the trick used in
> > mentioned
> > by Fixes tag commit.
> > 
> > I would rather update comment here, and add explanation to the
> > commit
> > message to be sure it covers tricks mitigation and preparation
> > purposes.
> 
> This is not mitigation, sorry.  It just replaces one possibly
> confusing thing with another.

I would agree here...

> The code as is works as I said and this patch doesn't make it any
> better as far as I'm concerned.

...but not here. Instead of returning pointer to *something* (from
caller point of view), we explicitly tell caller what of the above
happened. We don't rely on the organization  of ID table or its life
time (though it's forever).

I can say that is *slightly* better. But agree that is not cleanest
solution I can come up with.

I'm all ears on other possibilities how to get rid of that trick.

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