[RFC] Making talloc_reference() safer.

[Michael Adam]

ronnie sahlberg wrote:
> The drawback is that without forcing the choice of API ar creation time you
> have situation where an object is sometimes a hierarical object and at
> other times it suddently switches to a different model and a different
> api.
> What if this is changed in an external module I call and that module
> decides for me "your object is now using a completely different
> model".
> 
> The implication then is that all codepaths will have to change for
> these objects like this
> 
> from
> talloc_free(object)
> 
> into
> 
> if (talloc_is_referenced(object)) {
>      talloc_unref(parent, object)
> } else {
>     talloc_free(object)
> }
> 
> Or similar for "talloc_steal()"  that would conditionally now become
> talloc_ref() instead.
> Teh horrors.
> 
> I personally think that the "hierarcy" vs "multi-parent" has to be
> made at creation time and creation time only.

Precisely the point I was trying to make in my email a couple of
hours ago. ;-)

Full ACK.

> A bigger picture is that TALLOC is a hugely useful library which
> should be encouraged to be used also externally in non-samba projects
> as well.
> So API is really important. It is even more important that just "what
> API change is needed to solve a problem in samba", it is "what API
> makes sense to the average OSS developer working on his/her pet
> project".

Full ACK again.

Let me rephrase and emphasize it like this:

The hierarchical "simple" talloc is very easy to understand
and use. Basically the "malloc/free" mantra with all the extra
goodies. In this simplicity lies much of its great appeal, imho.

It is admittedly extremely practical in some circumstances to have
reference counting on memory contexts. But the hierarchical
nature of talloc implies that this can not simply be added by
bumping a counter. Instead the implementation adds additional
parents to a context, this is hence and this creates a whole new kind of
memory system with its own api that is a lot more complicated
and may not be mixed with the simple functions.

It should be unthinkable that a library (or function) should be
able to change the mode of a talloc context created and handed in
by the caller. This should be left under the control of the
caller. I.e. the library should not be able to force a complicated
talloc api onto a caller that only is interested in simple talloc.
We may not even have control over all callers.
If that was allowed, then we should remove the simple api and
only ever use the referencing api.

But I think, on the contrary, the simple talloc/talloc_free mode
and API should be sacred.
This can be enforced by fixing the talloc mode
(simple vs. referencable) at creation time.
As said in my previous mail (and e.g. many times by Jeremy),
if a library needs references, it should encapsulate this.
And if it relies on talloc contexts handed in by the caller, then
wrapping referencable talloc structures can be created
internally.

(Sorry, that should have been shorter, but maybe longish
 repeated arguments can even help... ;-)

Cheers - Michael

-------------- 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/20111026/d2892f2d/attachment.pgp>