Messages in this thread | | | Date | Sun, 24 Aug 2008 12:08:52 +0200 | From | Stefan Richter <> | Subject | Re: [PATCH 2.6.27-rc3] led: driver for LEDs on PCEngines ALIX.2 and ALIX.3 boards |
| |
Andrew Morton wrote: > On Tue, 19 Aug 2008 22:23:41 +0500 > Constantin Baranov <const@const.mimas.ru> wrote: >> --- linux-2.6.27-rc3/drivers/leds/leds-alix.c 1970-01-01 04:00:00.000000000 +0400 >> +++ linux-2.6.27-rc3-alix/drivers/leds/leds-alix.c 2008-08-19 21:59:05.207153570 +0500 ... >> +static int __init alix_led_probe(struct platform_device *pdev) >> +{ >> + int i; >> + int ret = 0; >> + >> + for (i = 0; i < ARRAY_SIZE(alix_leds) && ret >= 0; i++) >> + ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev); >> + >> + if (ret < 0) { >> + for (i = i - 2; i >= 0; i--) > > this is off-by-one, surely. > > If we get here with i==1, we'll loop 4 billion times. > >> + led_classdev_unregister(&alix_leds[i].cdev); >> + } >> + >> + return ret; >> +} ... > How's this look? ... > @@ -92,7 +95,7 @@ static int __init alix_led_probe(struct > ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev); > > if (ret < 0) { > - for (i = i - 2; i >= 0; i--) > + while (--i >= 0) > led_classdev_unregister(&alix_leds[i].cdev); > }
Really? Constantin's code looks correct to me. He increments i once more after failure.
Perhaps write it easier to read; i.e. in the same way as most error return checks everywhere in the kernel:
for (i = 0; i < ARRAY_SIZE(alix_leds); i++) { ret = led_classdev_register(...etc...); if (ret < 0) break; }
if (ret < 0) while (i--) led_classdev_unregister(&alix_leds[i].cdev);
Or while (--i >= 0)
or for (i--; i >= 0; i--) -- Stefan Richter -=====-==--- =--- ==--- http://arcgraph.de/sr/
| |