r4844: - Remove the unused attrsyn structure

- Change 0 to NULL when checking allocations
- Introduce the schema_attr_cmp hepler function
- Do not allow auxiliary classes to be missing
- Try to ease code readability and try to get
  main code out of loops when possibile.
(This used to be commit a30f647b8abe6b77ef73018f58af2f8a11689b91)

1 files changed, 164 insertions, 175 deletions

diff --git a/source4/lib/ldb/modules/schema.c b/source4/lib/ldb/modules/schema.c
index 912c8657d2..b396ae0459 100644
--- a/source4/lib/ldb/modules/schema.c
+++ b/source4/lib/ldb/modules/schema.c
@@ -36,33 +36,6 @@
 #include "ldb/include/ldb.h"
 #include "ldb/include/ldb_private.h"
 
-struct attribute_syntax {
-	const char *name;
-	const char *syntax_id;
-};
-
-static struct attribute_syntax attrsyn[] = {
-		{ "Object(DS-DN)", "2.5.5.1"},
-		{ "String(Object-Identifier)", "2.5.5.2"},
-		{ "", "2.5.5.3"},
-		{ "String(Teletex)", "2.5.5.4"},
-		{ "String(IA5)", "2.5.5.5"}, /* Also String(Printable) */
-		{ "String(Numeric)", "2.5.5.6"},
-		{ "Object(DN-Binary)", "2.5.5.7"}, /* Also Object(OR-Name) */
-		{ "Boolean", "2.5.5.8"},
-		{ "Integer", "2.5.5.9"}, /* Also Enumeration (3 types ?) ... */
-		{ "String(Octet)", "2.5.5.10"}, /* Also Object(Replica-Link) */
-		{ "String(UTC-Time)", "2.5.5.11"}, /* Also String(Generalized-Time) */
-		{ "String(Unicode)", "2.5.5.12"},
-		{ "Object(Presentation-Address)", "2.5.5.13"},
-		{ "Object(DN-String)", "2.5.5.14"}, /* Also Object(Access-Point) */
-		{ "String(NT-Sec-Desc))", "2.5.5.15"},
-		{ "LargeInteger", "2.5.5.16"}, /* Also Interval ... */
-		{ "String(Sid)", "2.5.5.17"}
-	};
-
-#define SCHEMA_TALLOC_CHECK(root, mem, ret) do { if (!mem) { talloc_free(root); return ret;} } while(0);
-
 #define SCHEMA_FLAG_RESET	0
 #define SCHEMA_FLAG_MOD_MASK	0x03
 #define SCHEMA_FLAG_MOD_ADD	0x01
@@ -114,6 +87,18 @@ static int schema_attr_cmp(const char *attr1, const char *attr2)
 	return ret;
 }
 
+struct attribute_list *schema_find_attribute(struct attribute_list *list, int attr_num, const char *attr_name)
+{
+	unsigned int i;
+	for (i = 0; i < attr_num; i++) {
+		if (ldb_attr_cmp(list[i].name, attr_name) == 0) {
+			return &list[i];
+		}
+	}
+	return NULL;
+}
+
+/* get objectclasses of dn */
 static int get_object_objectclasses(struct ldb_context *ldb, const char *dn, struct schema_structures *schema_struct)
 {
 	char *filter = talloc_asprintf(schema_struct, "dn=%s", dn);
@@ -124,36 +109,39 @@ static int get_object_objectclasses(struct ldb_context *ldb, const char *dn, str
 	schema_struct->objectclass_list = NULL;
 	schema_struct->objectclass_list_num = 0;
 	ret = ldb_search(ldb, NULL, LDB_SCOPE_SUBTREE, filter, attrs, &srch);
-	if (ret == 1) {
-		for (i = 0; i < (*srch)->num_elements; i++) {
-			schema_struct->objectclass_list_num = (*srch)->elements[i].num_values;
-			schema_struct->objectclass_list = talloc_array(schema_struct,
-									 struct attribute_list,
-									 schema_struct->objectclass_list_num);
-			if (schema_struct->objectclass_list == 0) {
+	if (ret != 1) {
+		ldb_search_free(ldb, srch);
+		return -1;
+	}
+
+	for (i = 0; i < (*srch)->num_elements; i++) {
+		schema_struct->objectclass_list_num = (*srch)->elements[i].num_values;
+		schema_struct->objectclass_list = talloc_array(schema_struct,
+								 struct attribute_list,
+								 schema_struct->objectclass_list_num);
+		if (schema_struct->objectclass_list == NULL) {
+			ldb_search_free(ldb, srch);
+			return -1;
+		}
+		for (j = 0; j < schema_struct->objectclass_list_num; j++) {
+			schema_struct->objectclass_list[j].name = talloc_strndup(schema_struct->objectclass_list,
+									 (*srch)->elements[i].values[j].data,
+										 (*srch)->elements[i].values[j].length);
+			if (schema_struct->objectclass_list[j].name == NULL) {
 				ldb_search_free(ldb, srch);
 				return -1;
 			}
-			for (j = 0; j < schema_struct->objectclass_list_num; j++) {
-				schema_struct->objectclass_list[j].name = talloc_strndup(schema_struct->objectclass_list,
-											 (*srch)->elements[i].values[j].data,
-											 (*srch)->elements[i].values[j].length);
-				if (schema_struct->objectclass_list[j].name == 0) {
-					ldb_search_free(ldb, srch);
-					return -1;
-				}
-				schema_struct->objectclass_list[j].flags = SCHEMA_FLAG_RESET;
-			}
+			schema_struct->objectclass_list[j].flags = SCHEMA_FLAG_RESET;
 		}
-		ldb_search_free(ldb, srch);
-	} else {
-		ldb_search_free(ldb, srch);
-		return -1;
 	}
+	ldb_search_free(ldb, srch);
 
 	return 0;
 }
 
+/* get all the attributes and objectclasses found in msg and put them in schema_structure
+   attributes go in the check_list structure for later checking
+   objectclasses go in the objectclass_list structure */
 static int get_check_list(struct ldb_module *module, struct schema_structures *schema_struct, const struct ldb_message *msg)
 {
 	int i, j, k;
@@ -164,7 +152,7 @@ static int get_check_list(struct ldb_module *module, struct schema_structures *s
 	schema_struct->check_list = talloc_array(schema_struct,
 						   struct attribute_list,
 						   schema_struct->check_list_num);
-	if (schema_struct->check_list == 0) {
+	if (schema_struct->check_list == NULL) {
 		return -1;
 	}
 	for (i = 0, j = 0; i < msg->num_elements; i++) {
@@ -173,14 +161,14 @@ static int get_check_list(struct ldb_module *module, struct schema_structures *s
 			schema_struct->objectclass_list = talloc_array(schema_struct,
 									 struct attribute_list,
 									 schema_struct->objectclass_list_num);
-			if (schema_struct->objectclass_list == 0) {
+			if (schema_struct->objectclass_list == NULL) {
 				return -1;
 			}
 			for (k = 0; k < schema_struct->objectclass_list_num; k++) {
 				schema_struct->objectclass_list[k].name = talloc_strndup(schema_struct->objectclass_list,
 											 msg->elements[i].values[k].data,
 											 msg->elements[i].values[k].length);
-				if (schema_struct->objectclass_list[k].name == 0) {
+				if (schema_struct->objectclass_list[k].name == NULL) {
 					return -1;
 				}
 				schema_struct->objectclass_list[k].flags = msg->elements[i].flags;
@@ -190,7 +178,7 @@ static int get_check_list(struct ldb_module *module, struct schema_structures *s
 		schema_struct->check_list[j].flags = msg->elements[i].flags;
 		schema_struct->check_list[j].name = talloc_strdup(schema_struct->check_list,
 								  msg->elements[i].name);
-		if (schema_struct->check_list[j].name == 0) {
+		if (schema_struct->check_list[j].name == NULL) {
 			return -1;
 		}
 		j++;
@@ -199,6 +187,7 @@ static int get_check_list(struct ldb_module *module, struct schema_structures *s
 	return 0;
 }
 
+/* add all attributes in el avoiding duplicates in attribute_list */
 static int add_attribute_uniq(struct attribute_list **list, int *list_num, int flags, struct ldb_message_element *el, void *mem_ctx)
 {
 	int i, j, vals;
@@ -235,6 +224,9 @@ static int add_attribute_uniq(struct attribute_list **list, int *list_num, int f
 	return 0;
 }
 
+
+/* we need to get all attributes referenced by the entry objectclasses,
+   recursively get parent objectlasses attributes */
 static int get_attr_list_recursive(struct ldb_module *module, struct ldb_context *ldb, struct schema_structures *schema_struct)
 {
 	struct private_data *data = (struct private_data *)module->private_data;
@@ -253,46 +245,27 @@ static int get_attr_list_recursive(struct ldb_module *module, struct ldb_context
 			continue;
 		}
 		filter = talloc_asprintf(schema_struct, "lDAPDisplayName=%s", schema_struct->objectclass_list[i].name);
-		SCHEMA_TALLOC_CHECK(schema_struct, filter, -1);
+		if (filter == NULL) {
+			return -1;
+		}
+
 		ret = ldb_search(ldb, NULL, LDB_SCOPE_SUBTREE, filter, NULL, &srch);
-		if (ret == 0) {
-			int ok;
-
-			ok = 0;
-			/* suppose auxiliary classeschema_struct are not required */
-			if (schema_struct->objectclass_list[i].flags & SCHEMA_FLAG_AUXCLASS) {
-				int d;
-				ok = 1;
-				schema_struct->objectclass_list_num -= 1;
-				for (d = i; d < schema_struct->objectclass_list_num; d++) {
-					schema_struct->objectclass_list[d] = schema_struct->objectclass_list[d + 1];
-				}
-				i -= 1;
-			}
-			if (!ok) {
-				/* Schema Violation: Object Class Description Not Found */
-				ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Objectclass %s not found.\n", schema_struct->objectclass_list[i].name);
-				data->error_string = "ObjectClass not found";
-				return -1;
-			}
-			continue;
-		} else {
-			if (ret < 0) {
-				/* Schema DB Error: Error occurred retrieving Object Class Description */
-				ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Error retrieving Objectclass %s.\n", schema_struct->objectclass_list[i].name);
-				data->error_string = "Internal error. Error retrieving schema objectclass";
-				return -1;
-			}
-			if (ret > 1) {
-				/* Schema DB Error: Too Many Records */
-				ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Too many records found retrieving Objectclass %s.\n", schema_struct->objectclass_list[i].name);
-				data->error_string = "Internal error. Too many records searching for schema objectclass";
-				return -1;
-			}
+
+		if (ret <= 0) {
+			/* Schema DB Error: Error occurred retrieving Object Class Description */
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Error retrieving Objectclass %s.\n", schema_struct->objectclass_list[i].name);
+			data->error_string = "Internal error. Error retrieving schema objectclass";
+			return -1;
+		}
+		if (ret > 1) {
+			/* Schema DB Error: Too Many Records */
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Too many records found retrieving Objectclass %s.\n", schema_struct->objectclass_list[i].name);
+			data->error_string = "Internal error. Too many records searching for schema objectclass";
+			return -1;
 		}
 
 		/* Add inherited classes eliminating duplicates */
-		/* fill in kust and may attribute lists */
+		/* fill in required and optional attribute lists */
 		for (j = 0; j < (*srch)->num_elements; j++) {
 			int is_aux, is_class;
 
@@ -372,12 +345,13 @@ static int schema_add_record(struct ldb_module *module, const struct ldb_message
 {
 	struct private_data *data = (struct private_data *)module->private_data;
 	struct schema_structures *entry_structs;
-	int i, j;
+	unsigned int i;
 	int ret;
 
 	/* First implementation:
-		Build up a list of must and mays from each objectclass
-		Check all the musts are there and all the other attributes are mays
+		Build up a list of required and optional attributes from each objectclass
+		Check all the required attributes are present and all the other attributes
+		are optional attributes
 		Throw an error in case a check fail
 		Free all structures and commit the change
 	*/
@@ -404,44 +378,43 @@ static int schema_add_record(struct ldb_module *module, const struct ldb_message
 		return ret;
 	}
 
-	/* now check all musts are present */
+	/* now check all required attributes are present */
 	for (i = 0; i < entry_structs->must_num; i++) {
-		int found;
+		struct attribute_list *attr;
 
-		found = 0;
-		for (j = 0; j < entry_structs->check_list_num; j++) {
-			if (schema_attr_cmp(entry_structs->must[i].name, entry_structs->check_list[j].name) == 0) {
-				entry_structs->check_list[j].flags = SCHEMA_FLAG_CHECKED;
-				found = 1;
-				break;
-			}
-		}
+		attr = schema_find_attribute(entry_structs->check_list,
+					     entry_structs->check_list_num,
+					     entry_structs->must[i].name);
+
+		if (attr == NULL) { /* not found */
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+				  "The required attribute %s is missing.\n",
+				  entry_structs->must[i].name);
 
-		if ( ! found ) {
-			ldb_debug(module->ldb, LDB_DEBUG_ERROR, "The required attribute %s is missing.\n", entry_structs->must[i].name);
 			data->error_string = "Objectclass violation, a required attribute is missing";
 			talloc_free(entry_structs);
 			return -1;
 		}
+
+		/* mark the attribute as checked */
+		attr->flags = SCHEMA_FLAG_CHECKED;
 	}
 
-	/* now check all others atribs are found in mays */
+	/* now check all others atribs are at least optional */
 	for (i = 0; i < entry_structs->check_list_num; i++) {
 
 		if (entry_structs->check_list[i].flags != SCHEMA_FLAG_CHECKED) {
-			int found;
+			struct attribute_list *attr;
 
-			found = 0;
-			for (j = 0; j < entry_structs->may_num; j++) {
-				if (schema_attr_cmp(entry_structs->may[j].name, entry_structs->check_list[i].name) == 0) {
-					entry_structs->check_list[i].flags = SCHEMA_FLAG_CHECKED;
-					found = 1;
-					break;
-				}
-			}
+			attr = schema_find_attribute(entry_structs->may,
+						     entry_structs->may_num,
+						     entry_structs->check_list[i].name);
+
+			if (attr == NULL) { /* not found */
+				ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+					  "The attribute %s is not referenced by any objectclass.\n",
+					  entry_structs->check_list[i].name);
 
-			if ( ! found ) {
-				ldb_debug(module->ldb, LDB_DEBUG_ERROR, "The attribute %s is not referenced by any objectclass.\n", entry_structs->check_list[i].name);
 				data->error_string = "Objectclass violation, an invalid attribute name was found";
 				talloc_free(entry_structs);
 				return -1;
@@ -459,16 +432,16 @@ static int schema_modify_record(struct ldb_module *module, const struct ldb_mess
 {
 	struct private_data *data = (struct private_data *)module->private_data;
 	struct schema_structures *entry_structs, *modify_structs;
-	int i, j;
+	unsigned int i;
 	int ret;
 
 	/* First implementation:
 		Retrieve the ldap entry and get the objectclasses,
 		add msg contained objectclasses if any.
-		Build up a list of must and mays from each objectclass
-		Check all musts for the defined objectclass and it's specific
-		inheritance are there.
-		Check all other the attributes are mays or musts.
+		Build up a list of required and optional attributes from each objectclass
+		Check all required one for the defined objectclass and all its parent
+		objectclasses.
+		Check all other the attributes are optional or required.
 		Throw an error in case a check fail.
 		Free all structures and commit the change.
 	*/
@@ -478,13 +451,13 @@ static int schema_modify_record(struct ldb_module *module, const struct ldb_mess
 	}
 
 	/* allocate object structs */
-	entry_structs = talloc(module, struct schema_structures);
+	entry_structs = talloc_zero(module, struct schema_structures);
 	if (!entry_structs) {
 		return -1;
 	}
 
 	/* allocate modification entry structs */
-	modify_structs = talloc(entry_structs, struct schema_structures);
+	modify_structs = talloc_zero(entry_structs, struct schema_structures);
 	if (!modify_structs) {
 		talloc_free(entry_structs);
 		return -1;
@@ -518,85 +491,98 @@ static int schema_modify_record(struct ldb_module *module, const struct ldb_mess
 		return ret;
 	}
 
-	/* now check all entries are present either as musts or mays of curent objectclasses */
-	/* do not return errors there may be attirbutes defined in new objectclasses */
-	/* just mark them as being proved valid attribs */
+	/* now check all entries are present either as required or optional atributes of entry objectclasses */
+	/* if they are required and we are going to delete them then throw an error */
+	/* just mark them if being proved valid attribs */
 	for (i = 0; i < modify_structs->check_list_num; i++) {
-		int found;
+		struct attribute_list *attr;
 
-		found = 0;
-		for (j = 0; j < entry_structs->must_num; j++) {
-			if (schema_attr_cmp(entry_structs->must[j].name, modify_structs->check_list[i].name) == 0) {
-				if ((modify_structs->check_list[i].flags & SCHEMA_FLAG_MOD_MASK) == SCHEMA_FLAG_MOD_DELETE) {
-					ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Trying to delete the required attribute %s.\n", modify_structs->check_list[i].name);
-					data->error_string = "Objectclass violation: trying to delete a required attribute";
-					talloc_free(entry_structs);
-					return -1;
-				}
+		attr = schema_find_attribute(entry_structs->must,
+					     entry_structs->must_num,
+					     modify_structs->check_list[i].name);
+
+		if (attr == NULL) { /* not found */
+
+			attr = schema_find_attribute(entry_structs->may,
+						     entry_structs->may_num,
+						     modify_structs->check_list[i].name);
+
+			if (attr != NULL) { /* found*/
 				modify_structs->check_list[i].flags |= SCHEMA_FLAG_CHECKED;
-				found = 1;
-				break;
 			}
+
+			break; /* not found, go on */
 		}
-		if ( ! found) {
-			for (j = 0; j < entry_structs->may_num; j++) {
-				if (schema_attr_cmp(entry_structs->may[j].name, modify_structs->check_list[i].name) == 0) {
-					modify_structs->check_list[i].flags |= SCHEMA_FLAG_CHECKED;
-					break;
-				}
-			}
+
+		if ((modify_structs->check_list[i].flags & SCHEMA_FLAG_MOD_MASK) == SCHEMA_FLAG_MOD_DELETE) {
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+				  "Trying to delete the required attribute %s.\n",
+				  modify_structs->check_list[i].name);
+
+			data->error_string = "Objectclass violation: trying to delete a required attribute";
+			talloc_free(entry_structs);
+			return -1;
 		}
+
+		modify_structs->check_list[i].flags |= SCHEMA_FLAG_CHECKED;
 	}
 
-	/* now check all new objectclasses musts are present */
+	/* now check all new objectclasses required attributes are present */
 	for (i = 0; i < modify_structs->must_num; i++) {
-		int found;
+		struct attribute_list *attr;
 
-		found = 0;
-		for (j = 0; j < modify_structs->check_list_num; j++) {
-			if (schema_attr_cmp(modify_structs->must[i].name, modify_structs->check_list[j].name) == 0) {
-				if ((modify_structs->check_list[i].flags & SCHEMA_FLAG_MOD_MASK) == SCHEMA_FLAG_MOD_DELETE) {
-					ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Trying to delete the required attribute %s.\n", modify_structs->must[i].name);
-					data->error_string = "Objectclass violation: trying to delete a required attribute";
-					talloc_free(entry_structs);
-					return -1;
-				}
-				modify_structs->check_list[j].flags |= SCHEMA_FLAG_CHECKED;
-				found = 1;
-				break;
-			}
-		}
+		attr = schema_find_attribute(modify_structs->check_list,
+					     modify_structs->check_list_num,
+					     modify_structs->must[i].name);
+
+
+		if (attr == NULL) { /* not found */
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+				  "The required attribute %s is missing.\n",
+				  modify_structs->must[i].name);
 
-		if ( ! found ) {
-			ldb_debug(module->ldb, LDB_DEBUG_ERROR, "The required attribute %s is missing.\n", modify_structs->must[i].name);
 			data->error_string = "Objectclass violation, a required attribute is missing";
 			talloc_free(entry_structs);
 			return -1;
 		}
+
+		if ((modify_structs->check_list[i].flags & SCHEMA_FLAG_MOD_MASK) == SCHEMA_FLAG_MOD_DELETE) {
+			ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+				  "Trying to delete the required attribute %s.\n",
+				  modify_structs->must[i].name);
+
+			data->error_string = "Objectclass violation: trying to delete a required attribute";
+			talloc_free(entry_structs);
+			return -1;
+		}
+
+		attr->flags |= SCHEMA_FLAG_CHECKED;
 	}
 
-	/* now check all others atribs are found in mays */
+	/* now check all others attributes are at least optional */
 	for (i = 0; i < modify_structs->check_list_num; i++) {
 
 		if ((modify_structs->check_list[i].flags & SCHEMA_FLAG_CHECKED) == 0 &&
 		    (modify_structs->check_list[i].flags & SCHEMA_FLAG_MOD_MASK) != SCHEMA_FLAG_MOD_DELETE) {
-			int found;
+			struct attribute_list *attr;
+
+			attr = schema_find_attribute(modify_structs->may,
+						     modify_structs->may_num,
+						     modify_structs->check_list[i].name);
 
-			found = 0;
-			for (j = 0; j < modify_structs->may_num; j++) {
-				if (schema_attr_cmp(modify_structs->may[j].name, modify_structs->check_list[i].name) == 0) {
-					modify_structs->check_list[i].flags |= SCHEMA_FLAG_CHECKED;
-					found = 1;
-					break;
-				}
-			}
 
-			if ( ! found ) {
-				ldb_debug(module->ldb, LDB_DEBUG_ERROR, "The attribute %s is not referenced by any objectclass.\n", modify_structs->check_list[i].name);
+			if (attr == NULL) { /* not found */
+				ldb_debug(module->ldb, LDB_DEBUG_ERROR,
+					  "The attribute %s is not referenced by any objectclass.\n",
+					  modify_structs->check_list[i].name);
+
 				data->error_string = "Objectclass violation, an invalid attribute name was found";
 				talloc_free(entry_structs);
 				return -1;
 			}
+
+			modify_structs->check_list[i].flags |= SCHEMA_FLAG_CHECKED;
+
 		}
 	}
 
@@ -671,7 +657,10 @@ struct ldb_module *schema_module_init(struct ldb_context *ldb, const char *optio
 	}
 
 	data = talloc(ctx, struct private_data);
-	SCHEMA_TALLOC_CHECK(ctx, data, NULL);
+	if (data == NULL) {
+		talloc_free(ctx);
+		return NULL;
+	}
 
 	data->error_string = NULL;
 	ctx->private_data = data;