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 > > > 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@xxxxxxxxxxxxxxx> Intel Finland Oy -- 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
