Fixes to LDB memory handling

[Matthias Dieter Wallnöfer]

Andrew,

yes, this looks to be my fault. Feel free to revert this code piece!

Cheers,
Matthias

Andrew Bartlett wrote:
> Attached are two patches:
>   - A fix to the memory handling of async responses (ares) in the ldb_tdb
> backend
>   - A revert to the patch I put in for the descriptor module to alliviate
> a double-free observed in upgradeprovision error cases.
>
> Matthias,
>
> The reason I bring this up on the list is that I'm keen to ensure I
> understand the bug you were hoping to fix with:
>
> commit 0941099a2839812b18c2d3db86b18e92b152f1c8
> Author: Matthias Dieter Wallnöfer<mdw at samba.org>
> Date:   Wed Oct 20 14:27:04 2010 +0200
>
>      ldb:ldb_index.c - fix some memory leaks
>
> @@ -962,6 +966,7 @@ static int ltdb_index_filter(const struct dn_list
> *dn_list,
>
>                  ret = ldb_module_send_entry(ac->req, msg, NULL);
>                  if (ret != LDB_SUCCESS) {
> +                       talloc_free(msg);
>                          ac->request_terminated = true;
>                          return ret;
>                  }
>
>
> In particular, what test should I use to show the leaks you saw here?
> I'm keen to ensure that we don't go around in circles here, as we
> presumably need to fix whatever callback you observed the leak in.
>
> I'm confident that reverting this part of your patch is correct, because
> no other ldb backend calls talloc_free() in these circumstances (indeed,
> even the un-indexed ldb_tdb callback did not), but in memory handling,
> I'm particularly keen to understand the full story first.
>
> It also seems to me that any other solution would break the pre-October
> ABI in any case, as user-supplied callers are entitled to rely on the
> previous behaviour (that is, callback to always free).
>
> Nadya,
>
> As I mention above, I'm also including a revert of the patch I pushed to
> descriptor.c.  It may be one of the few callbacks correctly handling
> memory in the failure case, but I'm pretty sure the original code was
> correct.
>
> Simo,
>
> The async behaviour here is yours originally.  Can you confirm that this
> (ldb_module_send_entry() expects the callback to handle the talloc_free)
> is how it is intended to work?
>
> Thanks,
>
> Andrew Bartlett
>