acpi_pcc_retrieve_biosdata() returns success instead of error if HKEY.SINF is invalid. Fix this.
Furthermore, if acpi_pcc_retrieve_biosdata() returns an error during device addition, initialization is properly reverted but value 0 is returned, which means success. This would cause a crash when later using or removing the device, so fix this too. Signed-off-by: Jean Delvare <[email protected]> Cc: Harald Welte <[email protected]> Cc: Bruno Premont <[email protected]> --- Note: bugs found by code inspection, I don't have the hardware so I can't test. Harald, I am not sure what to do in the "Invalid HKEY.SINF data" case in acpi_pcc_retrieve_biosdata(). For now the driver is ignoring the case and going on as if nothing happened, but this means some values in pcc->sinf have undefined values. Shouldn't we return with error in this case? Also, I am curious why you are using ACPI_DEBUG_PRINT() for all error messages? ACPI_DEBUG_PRINT() resolves to nothing by default (I'm not even sure if it is possible to get it to resolve to actual error messages without tinkering deep into ACPI header files). Shouldn't we use ACPI_ERROR(()) instead? drivers/platform/x86/panasonic-laptop.c | 2 ++ 1 file changed, 2 insertions(+) --- linux-2.6.36.orig/drivers/platform/x86/panasonic-laptop.c 2010-10-21 10:01:13.000000000 +0200 +++ linux-2.6.36/drivers/platform/x86/panasonic-laptop.c 2010-10-21 11:18:23.000000000 +0200 @@ -285,6 +285,7 @@ static int acpi_pcc_retrieve_biosdata(st hkey = buffer.pointer; if (!hkey || (hkey->type != ACPI_TYPE_PACKAGE)) { ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid HKEY.SINF\n")); + status = AE_ERROR; goto end; } @@ -642,6 +643,7 @@ static int acpi_pcc_hotkey_add(struct ac if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) { ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Couldn't retrieve BIOS data\n")); + result = -EIO; goto out_input; } /* initialize backlight */ -- Jean Delvare Suse L3 -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
