should failed calls to device_register() always call put_device()?

[greg@kroah.com (Greg KH)]

On Sat, May 28, 2011 at 06:00:10PM +0200, Belisko Marek wrote:
> Hi Robert,
> 
> On Sat, May 28, 2011 at 5:15 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >
> > ?from drivers/base/core.c, we have the fairly unambiguous advice:
> >
> > * NOTE: _Never_ directly free @dev after calling this function, even
> > * if it returned an error! Always use put_device() to give up the
> > * reference initialized in this function instead.
> > */
> > int device_register(struct device *dev)
> > {
> > ? ? ? ?device_initialize(dev);
> > ? ? ? ?return device_add(dev);
> > }
> >
> > ?and yet, there appears to be driver code that does exactly that,
> > such as this snippet from drivers/w1/w1_int.c (line 86):
> >
> > ? ? ? ?... snip ...
> > ? ? ? ?err = device_register(&dev->dev);
> > ? ? ? ?if (err) {
> > ? ? ? ? ? ? ? ?printk(KERN_ERR "Failed to register master device. err=%d\n", err);
> > ? ? ? ? ? ? ? ?memset(dev, 0, sizeof(struct w1_master));
> > ? ? ? ? ? ? ? ?kfree(dev);
> > ? ? ? ? ? ? ? ?dev = NULL;
> > ? ? ? ?}
> Free is for allocated dev not for struct device so it is OK. IMO thi
> snippet should look like:
> err = device_register(&dev->dev);
> if (err) {
>     printk(KERN_ERR "Failed to register master device. err=%d\n", err);
>     put_device(&dev->dev);

Yes, that is correct.

>     memset(dev, 0, sizeof(struct w1_master));
>     kfree(dev);
>     dev = NULL;

Nope, you just accessed memory that was already freed twice, so you
could have oopsed any one of those times.

After the last put_device() happens, you CAN NOT touch the device again,
as it could be gone (might, might not, you don't really know, all you
know is you said you were done with it so you can't access it again.)

Hope this helps,

greg k-h