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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 8, 2018 at 4:59 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxxxxx> 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

Right.  And the caller knows when to expect the third case which is
the whole point.

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

Please see my reply to the second patch, then.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux