prefixMap refactoring

Kamen Mazdrashki kamen.mazdrashki at postpath.com
Sun Oct 11 15:23:20 MDT 2009


Hi Metze,

On Fri, Oct 9, 2009 at 21:20, Stefan (metze) Metzmacher <metze at samba.org> wrote:
>> On Thu, Oct 8, 2009 at 19:05, Stefan (metze) Metzmacher <metze at samba.org> wrote:
>>> Kamen Mazdrashki schrieb:
>>
>> Here it is -> patch #0006
>>
>> Please, bear in mind that this is an intermediate patch.
>> I've tested it with RPS-DSSYNC test and it seems to work.
>
>> -     schema_info = data_blob_hex_string(pfm.ctr.dsdb.mappings, >schemaInfo);
>> -     W_ERROR_HAVE_NO_MEMORY(schema_info);
>>+      schema_info_blob = data_blob_dup_talloc(pfm.ctr.dsdb.mappings,
>>schemaInfo);
>
> W_ERROR_HAVE_NO_MEMORY(schema_info_blob.data);
> is missing
>
Agree. Fixed.

>>@@ -158,18 +171,24 @@ WERROR dsdb_get_oid_mappings_drsuapi(const struct
>>dsdb_schema *schema,
>>       W_ERROR_HAVE_NO_MEMORY(ctr->mappings);
>>
>>       for (i=0; i < schema->num_prefixes; i++) {
>>+              if (!ber_write_partial_OID_String(ctr->mappings, >&oid_blob,
> schema->prefixes[i].oid)) {
>>+                      DEBUG(0, ("write_partial_OID failed for %s",
>>schema->prefixes[i].oid));
>>+                      return WERR_INTERNAL_ERROR;
>>+              }
>>+
>>               ctr->mappings[i].id_prefix      = >schema->prefixes[i].id>>16;
>>-              ctr->mappings[i].oid.oid        = >talloc_strndup(ctr->mappings,
>>-                                                               >schema->prefixes[i].oid,
>>-                                                               >schema->prefixes[i].oid_len - 1);
>>-              W_ERROR_HAVE_NO_MEMORY(ctr->mappings[i].oid.oid);
>>+              ctr->mappings[i].oid.length     = oid_blob.length;
>>+              ctr->mappings[i].oid.binary_oid = oid_blob.data;
>>+              W_ERROR_HAVE_NO_MEMORY(ctr->mappings[i].oid.binary_oid);
>>       }
>
> Here we can remote the W_ERROR_HAVE_NO_MEMORY() as we check the result
> of ber_write_partial_OID_String().
>
Agree. Fixed.

>
>>       if (include_schema_info) {
>>+              oid_blob = strhex_to_data_blob(ctr->mappings, >schema->schema_info);
>
> We should have W_ERROR_HAVE_NO_MEMORY(oid_blob.data) here...
>
>>               ctr->mappings[i].id_prefix      = 0;
>>-              ctr->mappings[i].oid.oid        = >talloc_strdup(ctr->mappings,
>>-                                                              >schema->schema_info);
>>-              W_ERROR_HAVE_NO_MEMORY(ctr->mappings[i].oid.oid);
>>+              ctr->mappings[i].oid.length     = oid_blob.length;
>>+              ctr->mappings[i].oid.binary_oid = oid_blob.data;
>>+              W_ERROR_HAVE_NO_MEMORY(ctr->mappings[i].oid.binary_oid);
>>       }
>
> ...and can remove W_ERROR_HAVE_NO_MEMORY() here.
>
> We should handle error directly when they happen.
>
Agree. Fixed.

>>-                      if (strcasecmp(schema->schema_info, >ctr->mappings[i].oid.oid) != 0) {
>>+                      oid_blob = strhex_to_data_blob(NULL, >schema->schema_info);
>
> strhex_to_data_blob() can fail!
>
>>+                      if (memcmp(oid_blob.data, >ctr->mappings[i].oid.binary_oid, 21) != 0) {
>>+                              data_blob_free(&oid_blob);
>>                               return WERR_DS_DRA_SCHEMA_MISMATCH;
>>                       }
>
>
Agree. Fixed.

>>-              if (strncmp(prefixes[i].oid, in, prefixes[i].oid_len) != >0) {
>>-                      continue;
>>-              }
>>+      /* make oid prefix, i.e. oid w/o last subidentifier */
>>+      pstr = strrchr(in, '.');
>>+      if (!pstr)      return WERR_INVALID_PARAM;
>>+      if (pstr < in)  return WERR_INVALID_PARAM;
>>+      if ((pstr - in) < 4) return WERR_INVALID_PARAM;
>>
>>-              val_str = in + prefixes[i].oid_len;
>>-              end_str = NULL;
>>-              errno = 0;
>>+      oid_prefix = talloc_strndup(0, in, pstr - in);
>>
>>-              if (val_str[0] == '\0') {
>>-                      return WERR_INVALID_PARAM;
>>+      for (i=0; i < num_prefixes; i++) {
>>+              if (strcmp(prefixes[i].oid, oid_prefix) == 0) {
>>+                      break;
>>               }
>>+      }
>>
>>-              /* two '.' chars are invalid */
>>-              if (val_str[0] == '.') {
>>-                      return WERR_INVALID_PARAM;
>>-              }
>>+      talloc_free(oid_prefix);
>
> I think we should avoid using talloc_strdup() here, and better use
> strncmp() as before. This lookup needs to be fast...

Please bear in mind that this is a work in progress and this is reference implementation, 
which is to be removed shortly. This implementation does exactly what  should be done: 
searches for an exact prefix. That's all.
Final implementation will be much faster, even from the original one ;)

I am sending you the whole bunch of patches again for review.

Thanks,
Kamen Mazdrashki
kamen.mazdrashki at postpath.com
http://repo.or.cz/w/Samba/kamenim.git
-------------------------------------
CISCO SYSTEMS BULGARIA EOOD
http://www.cisco.com/global/BG/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4-drs-idl-Redefine-drsuapi_DsReplicaOID-in-drsuap.patch
Type: application/octet-stream
Size: 1109 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s4-drs-idl-Regenerate-idl.patch
Type: application/octet-stream
Size: 5129 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-s4-asn1-Use-explicite-TALLOC_CTX-in-ber_write_OID-f.patch
Type: application/octet-stream
Size: 5274 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-s4-drs-NDR-Remove-push-pull-code-for-drsuapi_DsRep.patch
Type: application/octet-stream
Size: 4619 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-s4-drs-NDR-Print-implementation-for-drsuapi_DsRepl.patch
Type: application/octet-stream
Size: 1661 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-s4-drs-Propagate-redefinition-of-drsuapi_DsReplicaO.patch
Type: application/octet-stream
Size: 15632 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-s4-drs-tort-TORTURE_DRS-torture-module-initial-i.patch
Type: application/octet-stream
Size: 5176 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0006.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-s4-drs-tort-oid_from_attid-reference-implementat.patch
Type: application/octet-stream
Size: 4346 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0007.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-s4-drs-tort-ignore-drs-proto.h-file.patch
Type: application/octet-stream
Size: 650 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0008.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-s4-drs-tort-_drs_ldap_attr_by_oid-implementation.patch
Type: application/octet-stream
Size: 2359 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0009.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0011-s4-drs-tort-drs_util_DsAttributeId_to_string-fun.patch
Type: application/octet-stream
Size: 6371 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0010.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0012-s4-drs-tort-_drs_util_verify_attids-to-verify-AT.patch
Type: application/octet-stream
Size: 3054 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0011.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0013-s4-drs-tort-fix-compile-time-warning.patch
Type: application/octet-stream
Size: 659 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0012.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0014-s4-drs-prefixMap-module-initial-definition.patch
Type: application/octet-stream
Size: 3594 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0013.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0015-s4-drs-tort-prefixMap-unit-test-initial-implementa.patch
Type: application/octet-stream
Size: 4048 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091012/2e1411fd/attachment-0014.obj>


More information about the samba-technical mailing list