[PATCH] Add stackframes to public libsmbclient functions

[Derrell Lipman]

On Nov 19, 2007 1:14 PM, Jeremy Allison <jra at samba.org> wrote:
> My vote is to commit this patch. It fixes an immediate
> showstopper in 3.2.0, tidies up client memory handling
> and is IMHO the correct mechanism to use.
>
> It uses the same idiom as libsmbclient itself. libsmbclient
> exports a context handle to its callers that they must remember to
> close/free. All this does it causes libsmbclient to handle
> memory in the same way.
>
> If it's good enough for your callers why isn't it good
> enough for your code ? :-) :-).

I believe that libsmbclient behaves like open/close/read/write: you
get a handle from open(), pass it to read() and write(), and then
indicate that you're finished with it by calling close().  I don't
believe there's anyplace in the base libsmbclient code where a handle
was set previously by some ancestor calling a function (or other
magic) and then used internally, is there?

Again (since I don't seem to have gotten across exactly what my
objection is), I have no problem with handles being passed back and
forth.  In fact, I *encourage* it.  There *should* be a talloc context
passed to functions that need to return a value attached to a context.
 My objection is only that the inputs to some function are now not
solely via parameters.  The inputs to functions now include a talloc
context stored on a separate stack and not passed as a parameter, and
it's up to some ANCESTOR of the caller to push and pop that stack.  I
believe that each function should be cleaning up its own mess, and if
a function needs for function X to attach its return value to a
particular talloc context, it should be providing that talloc context
to function X via a parameter.  It should not be an ANCESTOR's
responsibility to push a context onto a stack and later clean up that
context, if the ancestor itself doesn't require that context.

Clearly there's a lot of push to use Volker's patch.  I REALLY hope
that Jerry's statement that this is a somewhat interim solution is
correct and that these calls in libsmbclient to clean up for
lower-layer functions (which should, I think, have done so themselves)
can go away at some time in the near future.

I've presented my case against this patch. Yes, go ahead and apply it.
I'll live.  :-)

Derrell