[PATCH] Add stackframes to public libsmbclient functions
derrell.lipman at unwireduniverse.com
Mon Nov 19 18:50:52 GMT 2007
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. :-)
More information about the samba-technical