[PATCH][SAMBA4] Handle linked attributes

[Andrew Bartlett]

On Mon, 2007-10-29 at 09:17 +0100, Stefan (metze) Metzmacher wrote:
> -----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?

Any idea what that error would be?

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

Yeah, if the other attributes are only in the configuration container,
then that would make sense.  We may need to move to your 'load schema
from ldif' mode to handle this.

> > +/* 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);
> 	}

Yeah, that makes sense.  I suppose I was trying to make the 'no schema'
case just like the 'nothing changed' case, but it's probably not worth
it...

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

It does, if the other patch is applied.

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20071029/4ed70dce/attachment.bin