LDB hidden memory leaks
abartlet at samba.org
Sun Jul 11 16:17:11 MDT 2010
On Sun, 2010-07-11 at 21:32 +0300, Kamen Mazdrashki wrote:
> On Sat, Jul 10, 2010 at 17:00, Kamen Mazdrashki <kamenim at gmail.com>
> On Sat, Jul 10, 2010 at 11:16, Stefan (metze) Metzmacher
> <metze at samba.org> wrote:
> The way you added this:
> +#ifndef _DEPRECATED_
> +#define _DEPRECATED_ __attribute__ ((deprecated))
> is not portable, not every compiler supports this.
> The correct definition already comes from replace.h.
> So ldb.h only needs this:
> +#ifndef _DEPRECATED_
> +#define _DEPRECATED_
> You are right but... this is not what my intention is.
> I am pretty sure I've caught all places in Samba4 where
> functions are used. In your way we will receive warning only
> when compiling Samba.
> I wanted this for the users of LDB (not just Samba).
> So, how to achieve this?
> (I thought I did it the same way as with PRINTF_ATTRIBUTE
> macro, but now
> I see I am not)
> Perhaps pasting the whole snippet from replace.h like this?
> #ifndef _DEPRECATED_
> #if (__GNUC__ >= 3) && (__GNUC_MINOR__ >= 1 )
> #define _DEPRECATED_ __attribute__ ((deprecated))
> #define _DEPRECATED_
> After Metze's comment I've updated the last commit:
> As I want to warn not only in Samba's source base that those functions
> are deprecated,
> I've copy&paste the whole snippet for _DEPRECATED_ from replace.h
OK, so this is because we need _DEPRECATED_ to work in code that isn't
Samba, and while ldb uses replace.h, it does not expose replace.h into
it's caller's namespace.
> Any comments?
I looked at
http://gitweb.samba.org/?p=kamenim/samba.git;a=commitdiff;h=35a3455f1a0062d77ba7e03d4da2bd6eab7bb040 (s4-ldb: Implement ldb_msg_diff_ex() function to accept a memory context from client) and I wondered why you didn't do it in the same commit as http://gitweb.samba.org/?p=kamenim/samba.git;a=commitdiff;h=c3db036163bbb780047dd69725eae01e94825cf7 (s4-ldb: Refactor ldb_msg_diff() to be based on ldb_msg_diff_ex() implementatoin) to show that you are not really changing the code, just adding a memory context.
However, looking at this commit, I do have one problem: You should not
change which memory context ldb_msg_diff() uses. It is now a deprecated
function, so you can assume that smart callers are no longer using it -
but older callers may still assume that the result will go away when the
ldb does, so you can't use a NULL context here.
The same applies to:
(s4-ldb: Add ldb_msg_canonicalize_ex() to accept a memory context from
(s4-ldb: Refactor ldb_msg_canonicalize() to be based on
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 190 bytes
Desc: This is a digitally signed message part
More information about the samba-technical