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