should failed calls to device_register() always call put_device()?
Robert P. J. Day
rpjday at crashcourse.ca
Sat May 28 12:29:49 EDT 2011
On Sat, 28 May 2011, Belisko Marek wrote:
> Hi Robert,
>
> On Sat, May 28, 2011 at 5:15 PM, Robert P. J. Day <rpjday at 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);
> memset(dev, 0, sizeof(struct w1_master));
> kfree(dev);
> dev = NULL;
> }
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.
rday
p.s. i would also never do a memset() to zero, followed by a kfree(),
when a kzfree() is so much more concise.
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
More information about the Kernelnewbies
mailing list