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:14 PM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> On Wed, Feb 7, 2018 at 3:56 PM, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>> Instead of playing tricks with last invalid entry,
>> return simple -ENODATA error code cast to pointer.
>>
>> It would be good for future in case caller passes NULL pointer for
>> ID table. Moreover, caller can check the code to be sure what happened
>> inside callee.
>>
>> Fixes: 2b9c698efa58 ("ACPI / scan: Take the PRP0001 position in the list of IDs into account")
>
> I still don't think the Fixes: tag here is valid (the code works as is
> AFAICS), but I could drop it when applying the patch just fine. :-)
>
> That said, see below.
>
>> Cc: Sinan Kaya <okaya@xxxxxxxxxxxxxx>
>> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> Cc: Vinod Koul <vinod.koul@xxxxxxxxx>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/acpi/bus.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 58239abb0a72..761286e50067 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -769,7 +769,7 @@ static const struct acpi_device_id *__acpi_match_device(
>>                  */
>>                 if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)
>>                     && acpi_of_match_device(device, of_ids))
>> -                       return id;
>> +                       return ERR_PTR(-ENODATA);
>
> Doesn't the comment above need to be updated?
>
> Also the return value here means "success", so why is an error the right choice?

Moreover, if ids is NULL, then dereferencing it in the loop above
won't work already, so if we get here, there is at least one entry in
it.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux