Refactor of Binary DN code

[Stefan (metze) Metzmacher]

Andrew Bartlett schrieb:
> I wanted to bring your attention to my GIT branch dsdb-dn
> http://gitweb.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/dsdb-dn
> 
> This creates a new struct dsdb_dn and associated functions, taking over
> from ldb_dn for the DN+Binary and DN+String code.

It looks good! Here're sme minor comments:

Can you use a helper variable for dn_type here.

+       return dsdb_dn_construct_internal(mem_ctx, dn, extra_part,
dsdb_dn_oid_to_format(oid), oid);

We may need to allow non even number for DN+STRING...

+       if ((blen % 2 != 0)) {
+               DEBUG(10, (__location__ ": blen=%u - not an even
number\n", (unsigned)blen));
+               goto failed;
+       }


Can you split variable assigments from the declaration and add error
checks here:

+char *dsdb_dn_get_linearized(TALLOC_CTX *mem_ctx,
+                             struct dsdb_dn *dsdb_dn)
+{
+       const char *postfix = ldb_dn_get_linearized(dsdb_dn->dn);
+       return dsdb_dn_get_with_postfix(mem_ctx, dsdb_dn, postfix);
+}
+
+char *dsdb_dn_get_casefold(TALLOC_CTX *mem_ctx,
+                          struct dsdb_dn *dsdb_dn)
+{
+       const char *postfix = ldb_dn_get_casefold(dsdb_dn->dn);
+       return dsdb_dn_get_with_postfix(mem_ctx, dsdb_dn, postfix);
+}
+
+char *dsdb_dn_get_extended_linearized(TALLOC_CTX *mem_ctx,
+                                     struct dsdb_dn *dsdb_dn,
+                                     int mode)
+{
+       char *postfix = ldb_dn_get_extended_linearized(mem_ctx,
dsdb_dn->dn, mode);
+       char *ret = dsdb_dn_get_with_postfix(mem_ctx, dsdb_dn, postfix);
+       talloc_free(postfix);
+       return ret;
+}

Can you use a helper variable here too (maybe use ldb_module_get_ctx()
once on top of the function):

dsdb_dn = dsdb_dn_parse(msg, ldb_module_get_ctx(ac->module), plain_dn,
attribute->syntax->ldap_oid);

and here too passing a function as argument is always ugly for usage
in gdb.

-       os->dn = ldb_dn_from_ldb_val(os, ldb_module_get_ctx(ac->module),
plain_dn);
-       if (!os->dn || !ldb_dn_validate(os->dn)) {
+       os->dsdb_dn = dsdb_dn_parse(os, ldb_module_get_ctx(ac->module),
plain_dn, oid);
+       if (!os->dsdb_dn || !ldb_dn_validate(os->dsdb_dn->dn)) {


We also need to keep in mind that there're some special comparison rules
and some controls to change the behavior on searches. Sometimes only the
dn part matters.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091106/58a41222/attachment.pgp>