[QUICK] talloc bugs

tridge at samba.org tridge at samba.org
Wed Jul 1 05:27:40 GMT 2009


Hi Sam,

 > Maybe I made what should be an obvious mistake, but I can't find it.
 > I'm still looking to see what is going on.

I've spent some more time on this today, and I've commited some
changes to talloc to fix the problems.

The main change is here:

  http://git.samba.org/?p=samba.git;a=commitdiff;h=5fe1d8dc1275e43d96da1297f5fb0d0088a1c3ab

As the commit message explains, this is an expanded version of your
simple abort() patch. It changes talloc_free() and talloc_steal() to
print error messages to stderr like this:

    ERROR: talloc_free with references at some/foo.c:123
	   reference at other/bar.c:201
	   reference at other/foobar.c:641

so it is simple to find all the places in the code that are broken. A
full run of make test then showed there were just a few places that
needed fixing (I think about 10). I've fixed those places in a series
of commits preceding the main talloc change.

The commit also adds a new function talloc_reparent() which is like
talloc_steal(), but it takes both the old parent and the new parent,
thus removing the ambiguity from talloc_steal() when used on a pointer
with more than one parent. If you pass in a reference as the old
reference then the reference is moved to the new parent.

This now means that the whole talloc interface now hides the
difference between a parent and a reference, so we can now really
think of it supporting multi-parent pointers. The only places where an
ambiguity can arise (talloc_free and talloc_steal when there are
multiple parents) will now fail with a error.

The expection is the function talloc_parent() which returns the true
"parent". I used that in a couple of places in fixing the breakages
caused by this patch, with the worst fixes being like this:

  p = talloc_reparent(talloc_parent(ptr), new_ctx, ptr);

If you search for the places where I used talloc_parent() in this way,
you'll find that the old code is basically pretty badly convoluted in
how it handles references. Putting in the above type of code was the
simplest way to fix it, but is certainly not the cleanest approach.

The commit also notes that using stderr like this in a library is very
dogdy, as fd 2 could be a file or a socket in badly behaved
daemons. I'll add a register function that allows a daemon to register
a debug function.

Cheers, Tridge


More information about the samba-technical mailing list