All of lore.kernel.org
 help / color / mirror / Atom feed
From: greg@kroah.com (Greg KH)
To: kernelnewbies@lists.kernelnewbies.org
Subject: should failed calls to device_register() always call put_device()?
Date: Sun, 29 May 2011 06:01:15 +0800	[thread overview]
Message-ID: <20110528220115.GF19246@kroah.com> (raw)
In-Reply-To: <BANLkTimeL70SbPgyz+=vnTYyWsYH5rrXcg@mail.gmail.com>

On Sat, May 28, 2011 at 10:22:20PM +0200, Belisko Marek wrote:
> On Sat, May 28, 2011 at 9:43 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > On Sat, 28 May 2011, Belisko Marek wrote:
> >
> >> On Sat, May 28, 2011 at 6:29 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >> >
> >> > ?i agree that there should be a "put_device(&dev->dev);" statement
> >> > as you show above. ?however, i still don't see how this can be
> >> > just a stylistic improvement as you seem to suggest. ?based on the
> >> > warning from the kernel source file, it would seem that you *must*
> >> > do a put_device() in that situation -- it's not optional.
> >
> >> Sure you're right. You can send a patch to fix this problem. Good
> >> catch.
> >
> > ?i didn't want to submit anything until i verified what correct code
> > should look like. ?and it's not like that's the only example -- others
> > are trivially easy to find, like this in
> > drivers/media/video/bt8xx/bttv-gpio.c (line 97):
> >
> > ?err = device_register(&sub->dev);
> > ?if (0 != err) {
> > ? ? ? ? ? kfree(sub);
> > ? ? ? ? ? return err;
> > ?}
> I'm little bit confused. If you look at device_add which is part of
> device_register if something in device_add failed it will always call
> put_device (in any error case) so why then is necessary to call it
> again when device_register return error?

Because there still is a "dangling" reference on the device.

You wouldn't want the device to be "gone" without you really knowing
about it, right?  If the device_register fails, you might need to do
something with the larger "device" structure before doing the last
put_device() to clean up the memory (like unwind other things you set up
before calling device_register.)

Trust me, dig through the driver core and kobject model, it's tricky to
follow, but it's there.  Or at least it was there the last time I did
this, that is why I documented it so that no one would have to do that
again and they could just easily follow the rules.

Hope this helps,

greg k-h

  reply	other threads:[~2011-05-28 22:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-28 15:15 should failed calls to device_register() always call put_device()? Robert P. J. Day
2011-05-28 16:00 ` Belisko Marek
2011-05-28 16:29   ` Robert P. J. Day
2011-05-28 18:56     ` Belisko Marek
2011-05-28 19:43       ` Robert P. J. Day
2011-05-28 20:22         ` Belisko Marek
2011-05-28 22:01           ` Greg KH [this message]
2011-05-29 11:21             ` Robert P. J. Day
2011-05-29 11:49               ` Greg KH
2011-05-29 12:49                 ` Robert P. J. Day
2011-05-28 21:57   ` Greg KH

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=20110528220115.GF19246@kroah.com \
    --to=greg@kroah.com \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /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.