All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Purdie <rpurdie@linux.intel.com>
To: felipe.balbi@nokia.com
Cc: me@felipebalbi.com, linux-kernel@vger.kernel.org,
	Anton Vorontsov <cbou@mail.ru>,
	David Woodhouse <dwmw2@infradead.org>, Greg KH <greg@kroah.com>,
	Pierre Ossman <drzeus@drzeus.cx>
Subject: Re: [PATCH] led: simplify led_trigger_register_simple
Date: Thu, 20 Nov 2008 14:45:59 +0000	[thread overview]
Message-ID: <1227192359.22263.23.camel@ted> (raw)
In-Reply-To: <20081120141438.GK7476@gandalf.research.nokia.com>

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?

Cheers,

Richard


  reply	other threads:[~2008-11-20 15:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-13  3:09 [PATCH] led: simplify led_trigger_register_simple Felipe Balbi
2008-11-13 12:03 ` Anton Vorontsov
2008-11-13 12:38 ` Richard Purdie
2008-11-13 18:14   ` Felipe Balbi
2008-11-13 19:10     ` Felipe Balbi
2008-11-20 13:10       ` Felipe Balbi
2008-11-20 13:33       ` Richard Purdie
2008-11-20 14:14         ` Felipe Balbi
2008-11-20 14:45           ` Richard Purdie [this message]
2008-11-20 15:01             ` Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1227192359.22263.23.camel@ted \
    --to=rpurdie@linux.intel.com \
    --cc=cbou@mail.ru \
    --cc=drzeus@drzeus.cx \
    --cc=dwmw2@infradead.org \
    --cc=felipe.balbi@nokia.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@felipebalbi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.