From mboxrd@z Thu Jan  1 00:00:00 1970
From: greg@kroah.com (Greg KH)
Date: Sun, 29 May 2011 06:01:15 +0800
Subject: should failed calls to device_register() always call put_device()?
In-Reply-To: <BANLkTimeL70SbPgyz+=vnTYyWsYH5rrXcg@mail.gmail.com>
References: <alpine.DEB.2.00.1105281106220.5308@localhost6.localdomain6>
	<BANLkTimSfBQ1JO9+TbdApgFXfqTM=a55nQ@mail.gmail.com>
	<alpine.DEB.2.00.1105281227030.5840@localhost6.localdomain6>
	<BANLkTi=DV73uMcZtzSJi5=CtUZk5nzVDqw@mail.gmail.com>
	<alpine.DEB.2.00.1105281535460.6726@localhost6.localdomain6>
	<BANLkTimeL70SbPgyz+=vnTYyWsYH5rrXcg@mail.gmail.com>
Message-ID: <20110528220115.GF19246@kroah.com>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

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