LDB hidden memory leaks

Sam Liddicott sam at liddicott.com
Tue Jul 13 03:41:51 MDT 2010


  On 13/07/10 01:17, Andrew Bartlett wrote:
> On Tue, 2010-07-13 at 02:43 +0300, Kamen Mazdrashki wrote:
>> On Tue, Jul 13, 2010 at 01:10, simo<idra at samba.org>  wrote:
>>          Even if it were I would not be thrilled by such a change.
>>          Why you pass an argument is clear as soon as you look at the
>>          function
>>          definition of the function you are calling.
>>
>>
>> This is exactly what I want to avoid, right :)
>>
>>
>> Forcing TALLOC_CTX* explicit cast may be implemented
>> with compile time warnings - we just need to re-typedef TALLOC_CTX.
>> Look what a patch I got this way:
>> http://gitweb.samba.org/?p=kamenim/samba.git;a=commitdiff;h=6a3aaa1197950d16bb623c26f910347bb1fceb83
> I don't see how that patch would change anything in terms of the
> warnings, but in terms of self-documenting code, this is a change that
> should be applied.  The void * here comes from an earlier time in ldb
> when it did not always have to use talloc().  This particular oddity was
> done away with a long time ago.
>
> Anyway, your patch is good, the TALLOC_CTX * is still a void *, but at
> least now the programmer has a better clue as to what it is used for.
>
> However, we can't change TALLOC_CTX * from void * without breaking the
> taloc API.

Maybe we can. We can change it and break the API, but I think we can 
also change it without breaking the API or ABI - and this because void* 
and TALLOC_CTX* are both pointers of the same size and any differences 
are compile time only.

Macro's will do the trick nicely and define the new API which is fully 
backwards compatible at a link level (because there are no changes in 
generated code) and at compiler level - because a macro can make the 
typecast (which maybe isn't needed where warnings are not fatal).

First a wrapper to be used in macro's to cast the mem_ctx to TALLOC_CTX* 
to be automatically for old code that did not know to do this.

#ifdef compat_old_talloc_client
#define TALLOC_CTX_CAST(mem_ctx) ((TALLOC_CTX*)(mem_ctx))
#else
#define TALLOC_CTX_CAST(mem_ctx) (mem_ctx)
#endif

If compat_old_talloc_client is defined, then the cast will be done 
without checking, otherwise the caller is presumed to know what they are 
doing and to have made any casts explicitly. Old code being re-compiled 
*may* need to #define compat_old_talloc_client before including talloc.h 
*if* warnings are fatal, or perhaps rather use some other define like 
compat_new_talloc_client, or maybe a lib version number or whatever, I 
merely intend to demonstrate the point.

We can insert that wrapper in definitions of macro based talloc calls, e.g.

#define talloc(ctx, type) (type 
*)talloc_named_const(TALLOC_CTX_CAST(ctx), sizeof(type), #type)

And for non-macro based talloc calls, we can define a macro with the 
same name and use ## to avoid recursive expansion in some compilers:

void talloc_set_destructor(const void *ptr, int (*destructor)(void *));
#define talloc_set_destructor(ptr, destructor) 
talloc_##set_destructor(TALLOC_CTX_CAST(ptr), destructor)

or if you prefer, that last #define can be enclosed in an #ifdef 
compat_old_talloc_client (or whatever).

Sam


More information about the samba-technical mailing list