Messages in this thread |  | | Date | Thu, 20 Nov 2008 17:01:48 +0200 | From | Felipe Balbi <> | Subject | Re: [PATCH] led: simplify led_trigger_register_simple |
| |
On Thu, Nov 20, 2008 at 02:45:59PM +0000, ext Richard Purdie wrote: > On Thu, 2008-11-20 at 16:14 +0200, Felipe Balbi wrote: > > > In answer to your question about kfree, I agree it needs to be called > > > upon error. The callers should just be calling > > > led_trigger_unregister_simple() in their failure paths though which > > > should take care of all problems? I know we used to register the simple > > > triggers late in paths so no error handling was needed to keep the code > > > simple and minimise the LED triggers impact on those systems. > > > > Well, led_trigger_register_simple() doesn't return anything. Imagine > > led_trigger_register_simple() fails, but the driver author decides > > it's not a failure if, let's say, a led doesn't turn on when we insert > > a mmc card to the slot since it doesn't change functionality. > > > > Now, imagine the user notes the led is not turning on and decides to > > unload and reload the module to try again. Once again the led doesn't go > > on. If the user keeps trying, it's quite a dangerous memory leak, right > > ? > > So we have the module loading and one of two things happens: > > led_trigger_register_simple() succeeds > led_trigger_register_simple() fails (probably from kmalloc failure) > > The module doesn't know or care which happened. When the module unloads > it calls led_trigger_unregister_simple() which will free the memory in > the success case and do nothing in the case where it had failed. > > So there is no memory leak?
Hmmm, you are right. Didin't think about the exit path. But freeing in led_triger_register_simple() if led_trigger_register() fails, also doesn't seem wrong.
-- balbi
|  |