Re: [PATCH v6 2/5] ACPI: battery: Fix acpi_battery_exit on acpi_battery_init_async errors

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 18-04-17 15:36, Rafael J. Wysocki wrote:
> On Tue, Apr 18, 2017 at 1:58 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The acpi_lock_battery_dir() / acpi_bus_register_driver() calls in
>> acpi_battery_init_async() may fail. Check that they succeeded before
>> undoing them.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/acpi/battery.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index 4ef1e46..b35fca4 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -67,6 +67,7 @@ MODULE_DESCRIPTION("ACPI Battery Driver");
>>  MODULE_LICENSE("GPL");
>>
>>  static async_cookie_t async_cookie;
>> +static bool battery_driver_registered;
>>  static int battery_bix_broken_package;
>>  static int battery_notification_delay_ms;
>>  static unsigned int cache_time = 1000;
>> @@ -1329,6 +1330,7 @@ static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie)
>>         if (result < 0)
>>                 acpi_unlock_battery_dir(acpi_battery_dir);
>>  #endif
>> +       battery_driver_registered = (result == 0);
>
> Maybe
>
>       battery_driver_registered = !result;

I prefer == 0 here because we are checking for success (which is 0)
not for "not true".

Regards,

Hans



>
>>  }
>>
>>  static int __init acpi_battery_init(void)
>> @@ -1343,9 +1345,11 @@ static int __init acpi_battery_init(void)
>>  static void __exit acpi_battery_exit(void)
>>  {
>>         async_synchronize_cookie(async_cookie + 1);
>> -       acpi_bus_unregister_driver(&acpi_battery_driver);
>> +       if (battery_driver_registered)
>> +               acpi_bus_unregister_driver(&acpi_battery_driver);
>>  #ifdef CONFIG_ACPI_PROCFS_POWER
>> -       acpi_unlock_battery_dir(acpi_battery_dir);
>> +       if (acpi_battery_dir)
>> +               acpi_unlock_battery_dir(acpi_battery_dir);
>>  #endif
>>  }
>
> Thanks,
> Rafael
>