tldap and LDB

[Andrew Bartlett]

On Sat, 2009-06-06 at 06:10 -0500, Volker Lendecke wrote:
> The branch, master has been updated
>        via  23b501e02a15fe94e807e279c224e5657ce47af2 (commit)
>        via  256b227b27b599fffe5746bae7132a27e2c59dd4 (commit)
>        via  1769c8d81b8b4ad7bae77fabce2bf2051a7d32c1 (commit)
>        via  7194937eea7f12a9408655654777fe19832e338a (commit)
>       from  0e261d0e9c89ff11dc37b2bfd70c74c3a06486bd (commit)
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 

> commit 7194937eea7f12a9408655654777fe19832e338a
> Author: Volker Lendecke <vl at samba.org>
> Date:   Sat Jun 6 12:30:57 2009 +0200
> 
>     Add the early start of an async ldap library
>     
>     There's a lot of things this does not do yet: For example it does not parse the
>     reply blob in the sasl bind, it does not do anything with controls yet, a lot
>     of the ldap requests are not covered yet. But it provides a basis for me to
>     play with a pdb_ads passdb module.

Volker,

You seem to have missed the mail I sent earlier, and I've done some more
research, so I thought I would bring this up again.

I'm rather confused by your recent work on tldap, and am not really sure
I understand what you are doing.  

What I'm wondering is why you feel the need to re-invent the whole
infrastructure here, yet again.  For example, even if you cannot for
some reason use ldb for the synchronous pdb calls (I would understand
this effort much better if pdb were async), why not at least re-use the
infrastructure?

tldap creates structures like:

struct tldap_attribute {
	char *name;
	int num_values;
	DATA_BLOB *values;
};

which is almost, but not entirely like:
struct ldb_message_element {
	unsigned int flags;
	const char *name;
	unsigned int num_values;
	struct ldb_val *values;
};

and a return structure:
struct tldap_message {
	struct asn1_data *data;
	uint8_t *inbuf;
	int type;
	int id;

	/* RESULT_ENTRY */
	char *dn;
	struct tldap_attribute *attribs;
};

which aside from the housekeeping looks so very similar to:
struct ldb_message {
	struct ldb_dn *dn;
	unsigned int num_elements;
	struct ldb_message_element *elements;
};

And now, having created this 'just different enough to be incompatible'
library, you are building yet another set of wrapper functions, to do
each of the little things that need to be done.  I know them well, as
both LDB and Samba4 has an impressive collection of functions (which I
see you have started to duplicate) like:

bool tldap_pull_binsid(struct tldap_message *msg, const char *attribute,
		       struct dom_sid *sid)
as a duplicate for
struct dom_sid *samdb_result_dom_sid(TALLOC_CTX *mem_ctx, 
                                     const struct ldb_message *msg,
                                     const char *attr)

bool tldap_pull_uint64(struct tldap_message *msg, const char *attr,
		       uint64_t *presult)
as a duplicate for
int64_t ldb_msg_find_attr_as_int64(const struct ldb_message *msg, 
				   const char *attr_name,
				   int64_t default_value)

Had you chosen the at least use the LDB structures, then this much you
would have had for free!

If you must insist on the new library, could you at least make it
compatible enough that it could be abstracted by LDB in future, if not
for your use, then for others?

Soon you will find it very useful to have extended DNs returned.  I
presume you will simply parse them by hand, but similarly Samba4 and LDB
already have the support for this. 

Why do you insist on so much duplication?  Why must every single aspect
be reinvented?  Why must we then re-learn all the lessons of LDAP
features and behaviour that LDB has implemented, for yet another time?

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20090610/876e85c9/attachment.bin