[QUICK] talloc bugs

[ronnie sahlberg]

I am not convinced using references is that useful at all.
Really, the purpose of a referencing system is to make memory
deallocation automatic and easy to use.
A reference system that contains "surprises" or is hard to use
(perhaps even harder than just doing the tracking manually) is less
than useful.


I think that it is a mistake and making it more complex for humans to
use to as now really having two different reference systems in place
but with confusion about what talloc_free() really means.

I think one would have to decide on create time what reference system
is used and that this would affect which operation is used to finally
free the object.
I think an easier API would be :
Single-Parented objects
* normal talloc allocations as today, these allocations always have a
single parent object and they can be reparented with talloc_"steal"
and they are freed by talloc_free().
*You can not add a reference to these single-parented objects.

Multi-Referenced objects
* a new set of APIs that mirror the current allocators but with a
_ref() prefix. These create a "referenced" object.
  Perhaps they should be created similar to
talloc_reference(<parent>, talloc(NULL, <type>))
  I.e. the objects are created without a parent, and instead of a
parent they have an initial reference.
* On objects that were created using *_ref() you can add additional
references using talloc_reference().
* You can not use talloc_free() on these objects, instead you always
use talloc_unreference().
Once the last reference is deleted, the entire object itself is
automatically freed.


This would keep the normal hierarical single-parented pure talloc tree
structure and the refcounted multiparented non-tree useages using
separate API.
The API itself would indicate which kind of reference model is used,
which would make it much easier for code review or when looking at
someone elses code, since the model would be explicit from the API
used.

I think it would be a mistake to try to get the two different models
use the same API or make one model a subset of the other, since this
will just "confuse".



In my usecases in CTDB I never use refcounting, though I du use
talloc_steal() a lot.  The only thing wrong with talloc_steal() is its
name since it inherently suggests one is doing something
invalid/dodgy. It should be renamed talloc_reparent() instead.


regards
ronnie sahlberg



On Mon, Jun 29, 2009 at 2:25 PM, <tridge at samba.org> wrote:
> Hi Sam,
>
> Thanks for bringing this to our attention again.
>
> As I mentioned the last time around, my preference is to keep the
> current behaviour, except to disallow direct talloc_free() of a
> pointer that has outstanding references. The idea is that if you free
> something with a reference either by the reference parent or by a
> direct parent then the intention of the caller is very clear. The only
> ambiguity is where you call talloc_free() on a pointer directly where
> that pointer has a outstanding reference. In that case the talloc code
> has no way of knowing which parent you intend to keep, and the caller
> should instead call a function that provides that information (such as
> talloc_unlink).
>
> What I don't know is whether this should be the default behaviour, or
> if we should have an inheritable flag to set this behaviour. That
> really depends on whether how much we are currently relying on direct
> free of pointers with references in our current code base. I haven't
> run that experiment.
>
> Do you have time to test this solution?
>
> Cheers, Tridge
>