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 17 lines · Rafael J. Wysocki
Quoted 7 lines · Andy Shevchenko
Quoted 5 lines · Rafael J. Wysocki

Also the return value here means "success", so why is an error the right choice?

Quoted 7 lines

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

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

Quoted 8 lines
Quoted 8 lines
Quoted 3 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.

Quoted 7 lines
Quoted 4 lines

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

Quoted 2 lines

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.