On Sat, 28 May 2011, 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); 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 ========================================================================