LDB hidden memory leaks

[Kamen Mazdrashki]

On Mon, Jul 12, 2010 at 02:53, Andrew Bartlett <abartlet at samba.org> wrote:

>
> I still think the commit would have been clearer if these were combined
> - or if it isn't clearer, then we should examine why that isn't the
> case.
>

I have no objections squashing those commits after review is done.


>
> > 2. "smart" caller is very vague you know. Are we in Samba "smart
> > callers"?
> >    I think we are :).
> >    Despite our "smartness" though, a quick look into build log shows
> > we have 2 or 3
> >    "this function is deprecated" warning that nobody does anything to
> > fix this. And those
> >    warning are there for months.
> >    I still thing we are "smart callers", we just don't have enough
> > time to fix this
> >    (and it is working, so nobody wants to spend time on something that
> > is working)
> >    On the other hand - if those "deprecated" functions start leaking
> > memory,
> >    or produce segfaults, them I am pretty sure someone will fix them
> > asap.
>
> No, it means that another project that uses LDB and had one behaviour,
> now has another behaviour because LDB on the user's system was upgraded
> by their distributor. This isn't acceptable under the current promises
> we make to our users about library versions.


> >    I can continue giving examples in this direction, but I thing my
> > point is clear now :)
> >    In resume, I think that if we notice some part of the code behaves
> > in not a nice way,
> >    then we should make sure it is  as visible as possible.
>
> None of this allows us to breach the ABI we have promised.  All we can
> do is change the compile-time marking so that a developer knows to use
> something else.  We MUST NOT change the semantics of the original
> call.


Perhaps you are right... if there was a spec describing in more details
what those functions should do.
As far as I see, the only spec we have is in the code in form
of code snippets using those functions (I may be quite wrong here though).
Despite those code snippets, we have different usage even in Samba -
in few places returned message is moved into another mem context,
in others it is not.

I still think that making this leak more obvious (in a way) is
the right way to do this.
May be not so rapidly - for example we may just warn with DEPRECATED
now and in a few months moving allocation from ldb_context to NULL?
Anyway, I can write down a Big Comment to emphasize that memory
is allocated in ldb_context and caller should steal it.

-- 
CU,
Kamen