the sorry saga of the talloc soname 'fix'

tridge at samba.org tridge at samba.org
Thu Jul 9 10:02:12 GMT 2009


Hi Sam,

My apologies for not having done a proper analysis of your proposal. 

I have just tried to apply your patches to a test tree, but they fail
to apply, and the fixups do not look trivial. Could you send me a
clean copy of talloc.[ch] and testsuite.c for me to look at?

I do have some comments based on your description of the patch though,
although whether those comments are useful depends on my correct
interpretation of your text description of the changes. I always
prefer to work from real code, as descriptions are rarely sufficient,
especially with such a complex change.

ok, so some wild specualation based on your description:

 1) you propose that talloc_free() be allowed with a reference is
 present, but that it should always free the 'original' or 'real'
 parent. That is exactly the opposite behaviour of what talloc has
 done since talloc_reference() was added, so changing it would be
 rather a surprise to existing code. The existing behaviour, before my
 recent changes, was to remove the most recent reference. I am always
 reluctant to completely invert existing behaviour. To prevent
 ambiguous calls is one thing, especially when a clear warning is
 given, but to do the exact opposite of what has been documented seems
 to be asking for trouble. I certainly wouldn't call it a ABI (or
 API) compatible change.

 2) I think, from your description, that talloc_steal would change to
 become a function that sometimes changes the effective reference
 count and sometimes doesn't. Let's look at an example:

    int *p1 = talloc(NULL, int);
    int *p2 = talloc(NULL, int);
    int *p3 = talloc(NULL, int);
    int *c1 = talloc(p1, int);
    int *r1 = talloc_reference(p2, c1);

at this point the effective reference count of c1 is 2. It will take 2
free calls for it to go away.

Now we do this:

    talloc_free(r1);

and if I have understood your patches correctly (a big if, given I
can't run them!) then we end at this point with p1 having a reference
to c1, but c1 also having a new type of pseudo-parent.

At this point c1 has a effective reference count of 1, so a single
free will get rid of it.

Now we do this:

    talloc_steal(p3, r1);

and now, if I have understood your patches correctly, p3 becomes the
'real' parent, but the effective reference count of c1 has increased!
It will now take two free operations for it to go away. So in total we
will need 3 frees to get rid of this pointer if you use a steal, and
two frees if you don't use a steal.

So that means that talloc_steal() now will, sometimes, change the
reference count of a pointer. That is not a property it has had
before, and it is not a desirable property I think. 

Have I interpreted the patches correctly, or should I wait until I can
actually run the code?

Cheers, Tridge


More information about the samba-technical mailing list