Re: [PATCH] kobject: make sure kobj->ktype is set beforekobject_init

[Kay Sievers]

On Thu, 2007-11-29 at 13:04 -0500, Alan Stern wrote: 
> On Thu, 29 Nov 2007, Kay Sievers wrote:
> 
> > > > Sounds fine, maybe we should also pass the name along, so it will be
> > > > obvious what happens here:
> > > >   int kobject_init(struct kobject *kobj, struct kobj_type *type, const char *fmt, ...)
> > > 
> > > I don't know...  Normally *_init() routines can't fail, but this could.  
> > > Then things like device_register() would run into trouble: The caller 
> > > wouldn't know whether a failure occurred before or after the 
> > > kobject_init() call, so it wouldn't know what sort of cleanup action 
> > > was needed: kfree() or device_put().
> > 
> > But wouldn't device_register() do the kobject cleanup for you when it
> > fails? Why would a caller of device_register() care about the state of
> > the kobject?
> 
> Let's say device_register() calls device_init(), which calls 
> kobject_init(), which fails.  Then there's no cleanup to do -- 
> device_register() returns -ENOMEM or some such code and the caller has 
> to do the kfree().
> 
> Now let's say device_register() calls device_init(), which succeeds, 
> and then calls device_add(), which fails.  To recover properly, 
> somebody then has to call device_put().  That "somebody" can't be the 
> original caller -- according to the previous paragraph the original 
> caller won't do anything but kfree().  So the "somebody" has to be 
> device_register() itself.
> 
> But the device_put() will call kobject_put(), which will invoke the
> device's cleanup routine, which will deallocate the structure.  Now the
> original caller gets an error code (perhaps -ENOMEM again) but must
> _not_ call kfree().
> 
> So what should the original caller do when an error occurs?

Right, and that is not covered today, the current code just leaks the
allocated name.

Your error scenario confirmed my initial concern about suggesting
kobject_put() to clean up an initialized kobject.

We should probably make kobject_cleanup() free only the resources taken
by kobject_init(), and use kobject_cleanup() instead of kobject_put()?

Kay

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/