talloc issues

Sam Liddicott sam at liddicott.com
Tue Jul 28 07:33:31 MDT 2009


* tridge wrote, On 28/07/09 13:35:
> Hi Sam,
> 
>  > But only when the code is exercised in particular circumstances.
> 
> no, it finds every case where the semantics have changed. 
> 
>  > We can never be sure that this covers all cases in a finite amount of
>  > time, or that we get the notices in all the failures. It's not
>  > definitely reliable.
> 
> yes, we can be sure, as the only ways in which the behaviour has
> changed is in ways that produce a warning. If a warning is not
> produced then the semantics haven't changed. If you think I'm wrong on
> this then please provide an example.

I thought the warning was only produced if additional references were
held at the time. The number of references don't change the semantics
because the number of references isn't predictable at compile time.

> 
>  > Your approach was successful only in that it completed, but I don't find
>  > the results more attractive than what they replace.
>  > 
>  > The large code base is a red herring, it just takes more time.
> 
> no, it isn't a red herring. 
> 
> When we change semantics in Samba we usually try to change the
> function form, so the compiler catches the error. We can't do that in
> this case in any reasonable way. 

yes

> The alternative is to print a clear
> warning when a call is made where the semantics differs from the
> previous semantics. That is what the patch tried to do. 

The other alternative is to examine all the code as I described. which
you reject /because/ of the size of code base. I guess rejecting because
of size is a way of saying "size is a not red herring".

>  > Either you want me to provide a way of auditing the code that we both
>  > think doesn't exist... or we don't agree on what definitely reliable means.
> 
> If you want to propose these changes to go in then you need to take
> seriously the existing codebase that relied on the existing
> semantics. That is not something that can be just ignored. 

I wrote my last post taking this seriously showing how we can classify
uses and recognize uses that had relied on existing semantics which
were/are to change.

I'm not sure why you think I'm ignoring it and not taking it seriously.

>  > Because we don't rigorously declare functions that take ownership, we
>  > don't have the information we need to audit the code automatically.
>  > 
>  > We also need to use human reasoning to work out what the buggy code
>  > should have been.
> 
> not so. As the patches I've made show, it is possible to make changes
> in a way that is safe. 

For certain meanings of safe; I recognized this when I said:
> > Your approach was successful only in that it completed, but I don't
> > find the results more attractive than what they replace.

- I can't use your proposition because it is a big run-time error plot.

> I'm just finding it rather frustrating to have
> to explain this so many times. 

I know how you feel... really really... but I can't use your semantics.
It may be easy to make samba work "correctly" according to the new
semantics but it's like building a second floor on a straw house. It can
work, but nobody wants to go upstairs.

> Go and look at your patches and see if
> you can find a way to make the change you want safely. If you can then
> we can look at the semantics you propose. 

We are arguing over acceptable meanings of safely. The post you replied
to contained just what you have now asked me to do; which is to identify
all uses of talloc_free, tick-off simplistic safe uses and examine the
rest, re-writing where necessary according as Obnox has said.

No-one should write code that calls talloc_free or talloc_steal, or call
code that does this unless they are demonstratably working on behalf of,
and with the knowledge of, the allocator - and this whether or not there
USUALLY happens to only be one parent.

- and this criteria for using talloc_free can only be evaluated by
humans; for code does not express enough human intent to be analysed for
this criteria by a machine.

Sam


More information about the samba-technical mailing list