prefixMap refactoring
Kamen Mazdrashki
kamen.mazdrashki at postpath.com
Wed Oct 14 09:29:35 MDT 2009
Hi list,
On Mon, Oct 12, 2009 at 00:23, Kamen Mazdrashki <kamen.mazdrashki at postpath.com> wrote:
> 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.
>
If there is someone that disagree with those patches, please comment.
If everyone is OK with them, I will ask Anatoliy to merge them in
master.
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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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: 15680 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/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/20091014/cc21b778/attachment-0014.obj>
More information about the samba-technical
mailing list