ldb_search interface considered harmful

[Andrew Bartlett]

On Tue, 2006-01-31 at 09:18 -0500, simo wrote:
> On Tue, 2006-01-31 at 16:02 +1100, Andrew Bartlett wrote:
> > I've recently been working on the Samba4 code, and the output of
> > --leak-report-full in particular.
> > 
> > The contents of this (for an inactive connection) should be quite small,
> > but I initially found a large number of ldb_message structures being
> > kept around.  While some of this was easy to clean up, the problem
> > appears to be the interface:
> > 
> > int ldb_search(struct ldb_context *ldb,
> >                const struct ldb_dn *base,
> >                enum ldb_scope scope,
> >                const char *expression,
> >                const char * const *attrs, struct ldb_result **res);
> > 
> > Because the *res is returned on the ldb as a context, it often stays
> > there.  The gendb wrapper 'fixes' this, but we don't always use it.
> > Should we modify the interface to have an explicit mem_ctx argument, to
> > avoid the caller needing to manually talloc_steal?
> 
> I would like to avoid passing down a mem_ctx and just document the fact.
> The API is made to be usable in any program and probably avoiding
> programs to be forced into using talloc is preferable.
> You should always free the result once done with a call, and talloc
> should then free everything down that path.

If the caller has to call talloc_free(), doesn't that mean that they
must know about talloc()?

The problem I have is that the default behaviour is to leak memory onto
the long-term context.  Everything continues to work perfectly, but
memory leaks.  Interfaces should be designed so that the easiest way to
code them is also the correct one.  I assert (by handwaving) that
calling both ldb_search and talloc_steal (on success) isn't the easiest
way to use the interface, and therefore callers will get it wrong.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20060201/40dd9b79/attachment.bin