LDB hidden memory leaks

tridge at samba.org tridge at samba.org
Thu Jul 15 22:47:00 MDT 2010


Hi Kamen,

Andrew and I looked at your ldb talloc changes, and they look good
except for two little things:

 1) you put the talloc context as the 3rd argument in the new
 functions. The convention we have used (almost) everywhere in Samba
 is that the first argument is the main context structure, and the 2nd
 argument is the talloc context for the result. Other arguments follow
 those. I think making these functions put the talloc context 3rd may
 lead to more typos in future (as TALLOC_CTX is a void, we get no type
 checking on this, so conventions are all we have).

 2) the (TALLOC_CTX *) cast also goes against our established
 convention. If we were to move to doing that, I think it may be
 better to use something like:

   #define TC(x) x

 and use TC(ptr) as a visual marker.

 Don't do that now though - if we decide to adopt TALLOC_CTX casting,
 let's do it as a separate patch.

Note that we do have some functions in ldb that don't follow the
"context then TALLOC_CTX then args" convention (eg. some ldb_dn
functions), but I don't think we have any with TALLOC_CTX 3rd. So
while we haven't been entirely consistent, introducing a 3rd varient
on argument ordering is asking for trouble :-)

Cheers, Tridge


More information about the samba-technical mailing list