[PATCH][SAMBA4] Handle linked attributes

Stefan (metze) Metzmacher metze at samba.org
Mon Oct 29 08:17:27 GMT 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Andrew,

That looks nice. Some minor comments inline.

Do we give the correct error if the link target doesn't exists?

What about the other linked attributes in our provision*.ldif files?
I assume 'make test' pass because no schema is loaded...

> +/* add */
> +static int linked_attributes_add(struct ldb_module *module, struct ldb_request *req)
> +{
> +	int i, j, ret;
> +	struct linked_attributes_context *ac;
> +
> +	const struct dsdb_schema *schema = dsdb_get_schema(module->ldb);
> +	if (ldb_dn_is_special(req->op.mod.message->dn)) { /* do not manipulate our control entries */
> +		return ldb_next_request(module, req);
> +	}

Please change it to this, it makes the logic more clear

	int i, j, ret;
	struct linked_attributes_context *ac;
	const struct dsdb_schema *schema = dsdb_get_schema(module->ldb);

	/* if no schema is loaded do nothing */
	if (!schema) {
		return ldb_next_request(module, req);
	}

	/* do not manipulate our control entries */
	if (ldb_dn_is_special(req->op.mod.message->dn)) {
		return ldb_next_request(module, req);
	}

> +	for (i=0; schema && i < req->op.add.message->num_elements; i++) {

No check for schema needed here

	for (i=0; i < req->op.add.message->num_elements; i++) {

> +		/* For each value being added, we need to setup the modify */
> +		for (j=0; j < el->num_values; j++) {
> +			struct ldb_request *new_req;
> +			/* Create the modify request */
> +			struct ldb_message *new_msg = ldb_msg_new(ac->down_req);
> +			if (!new_msg) {
> +				ldb_oom(module->ldb);
> +				return LDB_ERR_OPERATIONS_ERROR;
> +			}
> +			new_msg->dn = ldb_dn_new(new_msg, module->ldb, (char *)el->values[j].data);
> +			if (!new_msg->dn) {
> +				ldb_asprintf_errstring(module->ldb, 
> +					       "attribute %s value %s was not a valid DN", req->op.add.message->elements[i].name,
> +						       el->values[j].data);
> +				return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
> +			}
> +
> +			ret = ldb_msg_add_empty(new_msg, target_attr->lDAPDisplayName, 
> +						LDB_FLAG_MOD_ADD, NULL);
> +			if (ret != LDB_SUCCESS) {
> +				return ret;
> +			}
> +			
> +			ret = ldb_msg_add_string(new_msg, target_attr->lDAPDisplayName, 
> +						 ldb_dn_get_linearized(ac->orig_req->op.add.message->dn));
> +			if (ret != LDB_SUCCESS) {
> +				return ret;
> +			}

Does ldb_dn_get_linearized(ac->orig_req->op.add.message->dn) give the DN
in the form we want to store? I assume yes because of the module ordering...

metze
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFHJZcXm70gjA5TCD8RArvrAKDGMRssleLlgZh15pm5eeVyVmN+UwCggPNd
SBjgCTIJOroWomW01XMkkLo=
=li50
-----END PGP SIGNATURE-----


More information about the samba-technical mailing list