PATCH: Re: talloc -- Eureka*

[Sam Liddicott]

* Stefan (metze) Metzmacher wrote, On 29/07/09 21:11:
..
> See the top 3 commits here (the others have nothing to do with this
> discussion):
> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-talloc-metze-3
> 
>> The first patch adds talloc_reference_sticky and
>> talloc_increase_ref_count_sticky
> 
> What about adding your sticky code above my new branch?
> And please use 'sticky' instead of slippy in all places:-)

OK... first serious patch attempt attached.
I've used the word non_stick is some places, and slippy in none.

I want to say first that I noticed metzes talloc refactoring in the
master4-talloc-sam branch when I was almost done; and I like it very much.

I've used a macro TALLOC_MAGIC_NO_OWNER in preparation for metzes (your)
refactoring, but not bought in the re-factoring because I don't want to
obscure the changes I'm trying to make here.

I hope the refactoring can be applied on top of this at some point as it
makes talloc much more pleasant to read. [I've been breaking apart
talloc as part of a noweb literate-programming book on talloc in attempt
to unify memory management across systems (think of libgd and php and
swig, linking code together that has their own incompatible ref-count
mechanism). How nice for php to transparently use gnomes ref-counting on
gtk widgets and gd's ref counting on gd objects, and even nicer if this
is based on talloc -- but thats all another story]

I haven't added test cases but it does pass current tests. (Probably all
existing tests would want repeating with a varied mix of sticky
references...)

I don't think we need a non-sticky version of talloc_increase_refcount
as it is only used once in samba outside of talloc and it's test suite;
source4/ntvfs/posix/pvfs_wait.c: (req = pwait->req)

        talloc_increase_ref_count(req);
        ntvfs_async_setup(pwait->req, pwait);
        talloc_free(req);

which could be easily adjusted as like the precious function in that file.

I also fix a possible bug at the bottom of talloc_unlink where we would
still make the final call to talloc_unreference even if new_parent == NULL.

I also fix a bug in my previous reference destructor where I fail to
properly check the parent of the referenced pointer.

I independantly fixed some things that I now see metze also fixed in
master4-talloc-sam branch, e.g. checking TALLOC_MAGIC_NO_OWNER at top of
talloc_free_internal.

metzes talloc refactoring of _talloc_free_children also nice answers
this observation:
In talloc_free_internal when we free children, would it be simpler to
call talloc_unlink which seems to employ similar strategies? Likewise
for talloc_free_children.

Maybe I missed other metze fixes; he seems very sharp eyed.

Other observations:

* I note that talloc_reference_count does distinguish between references
and parents and doesn't count parents as references. I don't know if
this is desirable, or what the current parent===reference philosphy is.

* I note that talloc_realloc() is not allowed on referenced pointers
because of course no track is kept of copies to the original pointer. I
mention this only because it is another function that might fail with
references. (One day we could work around this using indirect pointers
whose parent is the thing being tracked).

Sam