On Wed, Aug 11, 2021 at 10:28:25PM +0200, Heiner Kallweit wrote: > On 11.08.2021 17:45, Andy Shevchenko wrote: > > On Fri, Aug 06, 2021 at 11:15:15PM +0200, Heiner Kallweit wrote: > >> Replace the ugly cast of the return_value pointer with proper usage. > >> In addition use dmi_match() instead of open-coding it. > > > > ... > > > >> - acpi_get_devices(NULL, check_acpi_smo88xx_device, NULL, > >> - (void **)&found); > >> + acpi_get_devices(NULL, check_acpi_smo88xx_device, NULL, &err); > >> > >> - return found; > >> + return !IS_ERR(err); > > > > Shouldn't you also check the status of acpi_get_device()? > > > This shouldn't be needed because err isn't touched if function fails. For the sake of clearness of the code I would do it. But in any case what really hurt my eye is the last line here. To me sounds like if (IS_ERR(err)) return false; return true; is much better to read (and I bet the compiler will generate the very same code for it). -- With Best Regards, Andy Shevchenko
