[RFC] Making talloc_reference() safer.

[Michael Adam]

Volker Lendecke wrote:
> On Mon, Oct 24, 2011 at 04:46:27PM -0700, Jeremy Allison wrote:
> > On Mon, Oct 24, 2011 at 05:50:38PM +1100, ronnie sahlberg wrote:
> > >
> > >
> > > I think it is almost always a mistake when mixing single-parent
> > > hierarical allocations with a multi-parent graph allocation.

That about sums it up nicely. :-)

> > > Even better I think would be if you had a new set of creator functions
> > >  where you could set the "no longer hierarical, now its multi-parent
> > > referenced" already from the initial creation. That would be even
> > > better imho  to force the choice of api at object creation time and
> > > not create it as ahierarical object and then later "upgrade" it to a
> > > different model.
> > > 
> > > 
> > > I.e. offer something like this
> > > talloc_new_ref(...)
> > > instead of using the pattern
> > > talloc_new(...)
> > > ...
> > > talloc_make_reference_ptr(...)
> > 
> > Yes, this makes much more sense to me too.
> 
> On further thought, I might be okay with calls like
> 
> talloc_set_referencable()
> talloc_unset_referencable()
> talloc_is_referencable()
> 
> talloc_reference() would return NULL for a non-referencable
> object, gensec could return EINVAL by first checking via
> talloc_is_referencable(). talloc_free() on a referencable
> object would just abort. Abort to me is preferable over just
> leaking because this makes sure we don't leak by accident.

This is the best suggestion so far (if we really need that
multi-parent nightmare :-).

But apart from being force to being somewhat
more explicit, can't a function (say in gensec) still
make an originally not referencable pointer referencable
before taking a reference?

I think this is of little effective use then.
I think it will make much more sense to only allow
setting referencability at creation time. With
Volker's talloc_new_referencable() suggestion.
So a libraray or function can not turn a non-refrencable
pointer into a referencable one.

In such a case, the library like gensec could not
take reference of the pointer handed in by the caller
but would have to create a higher level structure
created as referencable which would contain a pointer
to the original structure handed in by the client.
This is something Metze mentioned to me as a possible
solution for the "gensec-hands-out-references" (if I
got it right), which would go nocely go nicely with the above.

Volker, if I only repeated your idea, I'm sorry.
But I wanted to emphasize the point of not allowing
changing referencability after creation.

Cheers - Michael

> I still don't like talloc_reference() at all and I would
> much rather live without it, but with the set of calls it
> can become a very obvious part of an API. It complicates it,
> but if API developers see it as necessary, so be it. We can
> always work on alternatives.
> 
> Why no talloc_new_referencable() as I first thought? This
> would trickle up all credential_init() style calls and
> unnecessarily clutter those APIs for non-referencing use.
> 
> With best regards,
> 
> Volker Lendecke
> 
> -- 
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20111025/35854ffd/attachment.pgp>