[PATCH] Add stackframes to public libsmbclient functions

[Derrell Lipman]

On Nov 19, 2007 10:51 AM, Volker Lendecke <Volker.Lendecke at sernet.de> wrote:
> > > DEBUG(10, ("sid: %s\n", sid_string_static(sid)));
> > >
> The problem is that the above 1-liner would turn into
>
> {
>         TALLOC_CTX *ctx = talloc_init("bla");
>         DEBUG(10, ("sid: %s\n", sid_string_talloc(ctx, sid)));
>         talloc_free(ctx);
> }
>
> There are very many functions that right now do not have
> their own temporary talloc context but which want to use
> some sid_string_static equivalent functions. All of these
> would have to add the temporary talloc context that can be
> passed to sid_string_talloc. Overall this is much more
> clutter than having the talloc stackframes at well-defined
> layers. And as libsmbclient.c is the central gateway into the
> lower-level cli_whatever functions I think this is the right
> place.

Ok, I see your point.  I think I would implement it differently: I'd
allocate one temporary talloc context at the beginning of the function
and free it upon exit.  I'd then have that context available to pass
to any function that needed it, such as sid_string_talloc().  I
wouldn't allocate and free it around each call to sid_string_talloc().
 By having any function that needed a temporary talloc context
allocate one and free it on its own, the higher layers which didn't
need a temporary context wouldn't be bothered with requirements that
were irrelevant to it (i.e. allocating and freeing the stackframe).

I believe I understand what you're doing and trying to accomplish.
Certainly having the sid string talloc'ed rather than in a static
buffer is preferable, and ensuring that memory gets cleaned up is
mandatory.  If you have a strong aversion to allocating temporary
talloc context and passing it as a parameter where necessary to
eliminate the need for magic values on a stack, I'll deal with
stackframe calls in libsmbclient even though I don't think they belong
there.

Cheers,

Derrell