Skip to contentSkip to conversation

ACPI / bus: Return error code from __acpi_match_device() in one case

PATCH New Andy Shevchenko 1 participant 1 message Feb 8, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

Quoted 10 lines · Rafael J. Wysocki
Quoted 8 lines · Andy Shevchenko

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.

Quoted 5 lines
Quoted 2 lines
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. :-)

OK.

Quoted 6 lines

Review

Quoted 4 lines
                if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)
                    && acpi_of_match_device(device, of_ids))
(quoted diff) +1 -1
-                       return id;
+                       return ERR_PTR(-ENODATA);

Doesn't the comment above need to be updated?

Will do.

Quoted 2 lines

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.

Quoted 2 lines

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.