| Message ID | 20170418115842.16214-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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; > } > > 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
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 >
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); } 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 }
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(-)