prefixMap refactoring

Stefan (metze) Metzmacher metze at samba.org
Fri Oct 9 12:20:52 MDT 2009


Hi Kamen,

> On Thu, Oct 8, 2009 at 19:05, Stefan (metze) Metzmacher <metze at samba.org> wrote:
>> Kamen Mazdrashki schrieb:
>>> On Wed, Oct 7, 2009 at 18:22, Stefan (metze) Metzmacher <metze at samba.org> wrote:
>>>> I'm mostly happy with the patches, there're are some minor things in
>>>> the 0006-... patch, I'll try to comment on them in detail tomorrow.
>>>>
>>> Oh, I just noticed patch 0006 is not actually complete :(
>>> Sorry, I am on it right away.
>> Ok, then I wait for the new patch...
> 
> 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

>@@ -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().


> 	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.

>-			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;
> 			}


>-		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...

metze


More information about the samba-technical mailing list