[PATCH] Add stackframes to public libsmbclient functions

Derrell Lipman derrell.lipman at unwireduniverse.com
Mon Nov 19 15:13:23 GMT 2007


On Nov 19, 2007 9:54 AM, Volker Lendecke <Volker.Lendecke at sernet.de> wrote:
> On Mon, Nov 19, 2007 at 09:40:09AM -0500, Derrell Lipman wrote:
> > The concept has merit.  I wouldn't remove it quite yet.  Why not use
> > it as a debugging tool.  If every layer is _supposed_ to free its
> > locally-allocated memory, then why not have debug capability to
> > display -- at any layer -- memory which was allocated "on the stack"
> > that should have been freed earlier.  Instead of using it as a way for
> > the higher layers to "fix" a bug in the lower layer, why not use it as
> > a way to request a list (including file name and line number of
> > allocation) of memory that didn't get freed.  That way the bugs get
> > fixed where they should, but you retain the cleanliness of your new
> > model...
>
> Well, if no function is allowed to leak memory on
> talloc_tos(), then we should get rid of talloc_tos() at all.
> Not having to pass an explicit temporary talloc context was
> the whole point of it. If that defined and intended memory
> leak is generally seen as a bug, talloc_stackframe() needs
> to go because it was intended to enable it.

I understand.  It's a nice concept, but I think when it becomes
sufficiently onerous on the higher layer that's it's more code there
than the effort required at the lower layer to clean itself up, it's
probably not the way to go.  I'd be particularly concerned that
someone would forget, when adding a new function at the higher layer,
to allocate a stack frame and free that stackframe which is solely for
use by a lower layer.  There's no particular reason that one would
think about the needs of the lower layer if one needn't pass a
parameter to the lower layer.  More importantly, I don't think that
one should need to think about the needs of the lower layer.  The
lower layer should be a black box with its inputs solely defined by
its parameters, not by a required pair of function calls pre- and
post-.

I still think there's a way to make use of what you've done, but it'll
require more thought.

Derrell


More information about the samba-technical mailing list