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

[Kay Sievers]

On Thu, 2007-11-29 at 10:54 -0500, Alan Stern wrote:
> On Thu, 29 Nov 2007, Kay Sievers wrote:
> 
> > > And if someone calls kobject_put() after kobject_init() to clean up,
> > > their release function will not be called if they didn't set the ktype.
> > > So the check really belongs into kobject_init() IMO.
> 
> Right.  And even though cleaning up no longer needs to drop a reference
> to the kset, it still might need to free the kobject's name.  So for 
> example, either of these sequences:
> 
> 	kobject_init();			kobject_set_name();
> 	kobject_set_name();		kobject_init();
> 	...				...
> 	kobject_free();			kobject_free();
> 
> would leak memory.

Yeah, only the kobject_put() would free the name.

> In fact, if we were designing the kobject API from scratch, I'd suggest 
> making the ktype value an argument to kobject_init() so that it 
> _couldn't_ be omitted.

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, ...)

> > Hmm, will one expect that the whole object will also be free'd when we
> > suggest to call kobject_put() to cleanup? That might be pretty
> > unexpected, right?
> 
> I don't understand the question.  People _already_ expect the cleanup 
> routine to free the kobject when the last reference is dropped.  Why 
> should there be any confusion over this?

Oh, if you want to rewind on error and have an initialized but still
unregistered kobject, and just want to free the allocated name by
calling kobject_cleanup() or kobject_put() you might not expect, that
your whole object that embeds the kobject will be gone. Just something
we need to document ...

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/