talloc_tos() in common code

[Volker Lendecke]

On Fri, Apr 15, 2011 at 05:33:20PM +1000, tridge at samba.org wrote:
>  > That's not different from the current situation, except that
>  > with more proper use of talloc_stackframe in public
>  > functions the inner functions are safer from generating
>  > memleaks if due to a bug a temporary talloc context is not
>  > freed. Just see talloc_stackframe() as the normal way to
>  > allocate temporary talloc contexts. With every proper use of
>  > talloc_free(frame) at the exit of a function you gain a
>  > little bit of safety for the functions called from there.
> 
> ok, that could work, although I expect we will end up sometimes
> causing memory leaks in external code, if we aren't careful enough in
> cleaning up. We may not notice it easily as our own code would have
> cleanup calls.
> 
> One approach would be to not put the cleanup stuff in the main s4
> code. So when s4 calls common code, it will get a leak if the common
> code doesn't cleanup properly. Or have the cleanup there, but print a
> message if it ever has to actually free something.

It would be very worthwile to have this, but I guess it
might be something for -DDEVELOPER. For production use of s4
I would rather not have memleaks but to get a warning.

> If we don't adopt some strategy like this then we will be forever
> chasing down memleaks in our public libraries, as developers could
> quickly become complacent about cleanup, relying on it being done
> somewhere else.

talloc_tos() was initially developed for DEBUG and printing
strings. sid_string_tos() in retrospect is not the best
design, but it is just handy. Probably a much better
alternative would have been to extend printf (write our own)
with %S or something like that. Or always hand in an
appropriate buffer or so.

The coding convention that developed around talloc_tos() is
to really not rely on the automatic cleanup, but it gives
you a safety net if you forget it.

> One of the things I've always preferred about talloc over traditional
> garbage collection systems is that the programmer has greater control
> over when memory is released, so you don't get big delays when garbage
> collection kicks in. I'd prefer we didn't end up with that property by
> adopting "global pools" too much. This is especially important as
> talloc is very inefficient at hierarchies with lots of pointers
> handing directly off a single pointer (due to how it deals with the
> lists). It deals much better with "bushier" hierarchies.

That's why I favor use of talloc_stackframe in inner
functions as well.

BTW, handling the parent pointer -- what is it really that
does so many reallocs? Expanding realloc moves stuff around,
so I'm thinking that might be worse than adapting the
children's parent pointers and the realloc source needs
fixing anyway.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen