[ltp] Re: [Patch 2/3] ibm-acpi: LED Subsystem integration.

Shem Multinymous linux-thinkpad@linux-thinkpad.org
Thu, 26 Oct 2006 15:00:39 +0200


Hi Stefan,

On 10/26/06, Stefan Schmidt <stefan@datenfreihafen.org> wrote:
> > >+       ret = led_classdev_register(parent, &thinklight_led);
> > >+       if (ret < 0) {
> > >+               led_classdev_unregister(&power_led);
> > >+               led_classdev_unregister(&battery_amber_led);
> > >+               led_classdev_unregister(&battery_green_led);
> > >+               led_classdev_unregister(&ultrabase_led);
> > >+               led_classdev_unregister(&ultrabay_led);
> > >+               led_classdev_unregister(&standby_led);
> > >+       }
> > >+
> > >+       return ret;
> > >+}
> >
> > Don't duplicate the cleanup on error. Instead, put a single cleanup
> > sequence at the end, with labels for each line, and use goto to jump
> > to the right point.
>
> Indeed. Even if goto is not the right thing on other problems, it can
> make the code less bloated here.

Coming to think of it, there's an even better way.
Define a static const array containing the LED attributes.
Then have a static pointer to the next LED to initialize. It starts at
the beginning of the array, incremented via a loop in module init, and
decremented in a loop on either init failure or module unload.
The module init code in tp_smapi.c does exactly that, you can copy the code.

  Shem