Messages in this thread | | | Date | Thu, 2 Feb 2012 11:54:56 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH v2] add LED driver for PCA9663 I2C chip |
| |
On Thu, 2 Feb 2012 11:42:09 +0100 (CET) Peter Meerwald <pmeerw@pmeerw.net> wrote:
> > > Andrew Morton <akpm@linux-foundation.org> wrote: > > > > +exit: > > > > + while (i--) { > > > > + led_classdev_unregister(&pca9633[i].led_cdev); > > > > + cancel_work_sync(&pca9633[i].work); > > > > + } > > > This (untested!) loop has an off-by-one error. > > > Well, it's partly wrong. It's wrong for the cancel_work_sync() but not > > for the led_classdev_unregister(). The cancel_work_sync() can just be > > removed: we haven't scheduled any works yet. > > there are 4 leds with get led_classdev_register()ed > > wouldn't it be possible that someone is starting to use led1 while led3 has > not been registered yet? if registration for led3 fails, the exit code > (above) is called and there might be work scheduled > > so (hypothetically): > register led1 -> ok > register led2 -> ok > use led1 (schedule work) > register led3 -> fail -> exit code > > the code has been copied from other led drivers > I think there is no harm to keep cancel_work_sync()
Spose so - it won't hurt. And I was wrong about there being an off-by-one.
| |