| Message ID | 1234731354-7472-1-git-send-email-jirislaby@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Sun, 15 Feb 2009, Jiri Slaby wrote: > There was a misplaced status test. Move it to correct place and > rollback appropriately. hmm, looks like this has been broken since the day acpi_os_initialize1() was invented in 2004. Turns out that the bug and its fix are moot, however, as acpi_os_initialize1() is hard-coded to return success, opting for BUG_ON() if it sees a failure... So unless that changes, I'd prefer to keep the code simple and just not check its status -- which is effectively what we're doing right now. thanks, -Len > Signed-off-by: Jiri Slaby <jirislaby@gmail.com> > --- > drivers/acpi/bus.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 765fd1c..79efef6 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -760,18 +760,17 @@ static int __init acpi_bus_init(void) > > > status = acpi_os_initialize1(); > - > - status = > - acpi_enable_subsystem(ACPI_NO_HARDWARE_INIT | ACPI_NO_ACPI_ENABLE); > if (ACPI_FAILURE(status)) { > printk(KERN_ERR PREFIX > - "Unable to start the ACPI Interpreter\n"); > - goto error1; > + "Unable to initialize ACPI OS objects\n"); > + goto error0; > } > > + status = > + acpi_enable_subsystem(ACPI_NO_HARDWARE_INIT | ACPI_NO_ACPI_ENABLE); > if (ACPI_FAILURE(status)) { > printk(KERN_ERR PREFIX > - "Unable to initialize ACPI OS objects\n"); > + "Unable to start the ACPI Interpreter\n"); > goto error1; > } > > @@ -832,6 +831,7 @@ static int __init acpi_bus_init(void) > /* Mimic structured exception handling */ > error1: > acpi_terminate(); > +error0: > return -ENODEV; > } > > -- > 1.6.1.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21.2.2009 05:58, Len Brown wrote: > On Sun, 15 Feb 2009, Jiri Slaby wrote: > >> There was a misplaced status test. Move it to correct place and >> rollback appropriately. > > hmm, looks like this has been broken since the day > acpi_os_initialize1() was invented in 2004. It comes from bitkeeper times, I checked, it looked like a typo. > Turns out that the bug and its fix are moot, however, > as acpi_os_initialize1() is hard-coded to return success, > opting for BUG_ON() if it sees a failure... BUG_ON is not noreturn e.g. on embedded, anyway it would crash later in this case. I was thinking about returning a failure when one of them is NULL, but fell back to not do so and check only retval in the caller. > So unless that changes, I'd prefer to keep the code > simple and just not check its status -- which is effectively > what we're doing right now. Ok, no problem. Will repost. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 765fd1c..79efef6 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -760,18 +760,17 @@ static int __init acpi_bus_init(void) status = acpi_os_initialize1(); - - status = - acpi_enable_subsystem(ACPI_NO_HARDWARE_INIT | ACPI_NO_ACPI_ENABLE); if (ACPI_FAILURE(status)) { printk(KERN_ERR PREFIX - "Unable to start the ACPI Interpreter\n"); - goto error1; + "Unable to initialize ACPI OS objects\n"); + goto error0; } + status = + acpi_enable_subsystem(ACPI_NO_HARDWARE_INIT | ACPI_NO_ACPI_ENABLE); if (ACPI_FAILURE(status)) { printk(KERN_ERR PREFIX - "Unable to initialize ACPI OS objects\n"); + "Unable to start the ACPI Interpreter\n"); goto error1; } @@ -832,6 +831,7 @@ static int __init acpi_bus_init(void) /* Mimic structured exception handling */ error1: acpi_terminate(); +error0: return -ENODEV; }
There was a misplaced status test. Move it to correct place and rollback appropriately. Signed-off-by: Jiri Slaby <jirislaby@gmail.com> --- drivers/acpi/bus.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)