[PATCH] Add stackframes to public libsmbclient functions

simo idra at samba.org
Mon Nov 19 18:58:24 GMT 2007


On Mon, 2007-11-19 at 13:50 -0500, Derrell Lipman wrote:
> 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.  :-)

I have to say I agree with this reasoning and with the conclusion :-)

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Senior Software Engineer at Red Hat Inc. <ssorce at redhat.com>



More information about the samba-technical mailing list