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