From 32a7b82e87c71e103c47fee787ed81db6266921f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Matthias=20Dieter=20Walln=C3=B6fer?= Date: Tue, 6 Oct 2009 09:30:53 +0200 Subject: [PATCH] s4:ldb_tdb - Rework/Various - Unify the error handling method with "done" mark in all longer functions - Fix up result codes to match more the real MS AD - Some cosmetic fixups --- source4/lib/ldb/ldb_tdb/ldb_tdb.c | 392 +++++++++++++++--------------- 1 file changed, 200 insertions(+), 192 deletions(-) diff --git a/source4/lib/ldb/ldb_tdb/ldb_tdb.c b/source4/lib/ldb/ldb_tdb/ldb_tdb.c index 66a10b64138..7693ffeb81c 100644 --- a/source4/lib/ldb/ldb_tdb/ldb_tdb.c +++ b/source4/lib/ldb/ldb_tdb/ldb_tdb.c @@ -1,10 +1,10 @@ /* ldb database library - Copyright (C) Andrew Tridgell 2004 - Copyright (C) Stefan Metzmacher 2004 - Copyright (C) Simo Sorce 2006-2008 - + Copyright (C) Andrew Tridgell 2004 + Copyright (C) Stefan Metzmacher 2004 + Copyright (C) Simo Sorce 2006-2008 + Copyright (C) Matthias Dieter Wallnöfer 2009 ** NOTE! The following LGPL license applies to the ldb ** library. This does NOT imply that all of Samba is released @@ -43,6 +43,10 @@ * - description: make it possible to use event contexts * date: Jan 2008 * Author: Simo Sorce + * + * - description: fix up memory leaks and small bugs + * date: Oct 2009 + * Author: Matthias Dieter Wallnöfer */ #include "ldb_tdb.h" @@ -169,7 +173,7 @@ static int ltdb_check_special_dn(struct ldb_module *module, if (! ldb_dn_is_special(msg->dn) || ! ldb_dn_check_special(msg->dn, LTDB_ATTRIBUTES)) { - return 0; + return LDB_SUCCESS; } /* we have @ATTRIBUTES, let's check attributes are fine */ @@ -183,7 +187,7 @@ static int ltdb_check_special_dn(struct ldb_module *module, } } - return 0; + return LDB_SUCCESS; } @@ -218,7 +222,7 @@ int ltdb_store(struct ldb_module *module, const struct ldb_message *msg, int flg void *data = ldb_module_get_private(module); struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private); TDB_DATA tdb_key, tdb_data; - int ret; + int ret = LDB_SUCCESS; tdb_key = ltdb_key(module, msg->dn); if (!tdb_key.dptr) { @@ -254,15 +258,16 @@ static int ltdb_add_internal(struct ldb_module *module, const struct ldb_message *msg) { struct ldb_context *ldb = ldb_module_get_ctx(module); - int ret, i; + int ret = LDB_SUCCESS, i; ret = ltdb_check_special_dn(module, msg); if (ret != LDB_SUCCESS) { - return ret; + goto done; } if (ltdb_cache_load(module) != 0) { - return LDB_ERR_OPERATIONS_ERROR; + ret = LDB_ERR_OPERATIONS_ERROR; + goto done; } for (i=0;inum_elements;i++) { @@ -272,38 +277,40 @@ static int ltdb_add_internal(struct ldb_module *module, if (el->num_values == 0) { ldb_asprintf_errstring(ldb, "attribute %s on %s specified, but with 0 values (illegal)", el->name, ldb_dn_get_linearized(msg->dn)); - return LDB_ERR_CONSTRAINT_VIOLATION; + ret = LDB_ERR_CONSTRAINT_VIOLATION; + goto done; } if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) { if (el->num_values > 1) { - ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s speicified more than once", + ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once", el->name, ldb_dn_get_linearized(msg->dn)); - return LDB_ERR_CONSTRAINT_VIOLATION; + ret = LDB_ERR_CONSTRAINT_VIOLATION; + goto done; } } } ret = ltdb_store(module, msg, TDB_INSERT); - - if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) { - ldb_asprintf_errstring(ldb, - "Entry %s already exists", - ldb_dn_get_linearized(msg->dn)); - return ret; + if (ret != LDB_SUCCESS) { + if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) { + ldb_asprintf_errstring(ldb, + "Entry %s already exists", + ldb_dn_get_linearized(msg->dn)); + } + goto done; } - if (ret == LDB_SUCCESS) { - ret = ltdb_index_one(module, msg, 1); - if (ret != LDB_SUCCESS) { - return ret; - } + ret = ltdb_index_one(module, msg, 1); + if (ret != LDB_SUCCESS) { + goto done; + } - ret = ltdb_modified(module, msg->dn); - if (ret != LDB_SUCCESS) { - return ret; - } + ret = ltdb_modified(module, msg->dn); + if (ret != LDB_SUCCESS) { + goto done; } +done: return ret; } @@ -314,16 +321,17 @@ static int ltdb_add(struct ltdb_context *ctx) { struct ldb_module *module = ctx->module; struct ldb_request *req = ctx->req; - int tret; + int ret = LDB_SUCCESS; ldb_request_set_state(req, LDB_ASYNC_PENDING); - tret = ltdb_add_internal(module, req->op.add.message); - if (tret != LDB_SUCCESS) { - return tret; + if (ltdb_cache_load(module) != 0) { + return LDB_ERR_OPERATIONS_ERROR; } - return LDB_SUCCESS; + ret = ltdb_add_internal(module, req->op.add.message); + + return ret; } /* @@ -355,7 +363,7 @@ int ltdb_delete_noindex(struct ldb_module *module, struct ldb_dn *dn) static int ltdb_delete_internal(struct ldb_module *module, struct ldb_dn *dn) { struct ldb_message *msg; - int ret; + int ret = LDB_SUCCESS; msg = talloc(module, struct ldb_message); if (msg == NULL) { @@ -404,7 +412,7 @@ static int ltdb_delete(struct ltdb_context *ctx) { struct ldb_module *module = ctx->module; struct ldb_request *req = ctx->req; - int tret; + int ret = LDB_SUCCESS; ldb_request_set_state(req, LDB_ASYNC_PENDING); @@ -412,12 +420,9 @@ static int ltdb_delete(struct ltdb_context *ctx) return LDB_ERR_OPERATIONS_ERROR; } - tret = ltdb_delete_internal(module, req->op.del.dn); - if (tret != LDB_SUCCESS) { - return tret; - } + ret = ltdb_delete_internal(module, req->op.del.dn); - return LDB_SUCCESS; + return ret; } /* @@ -453,6 +458,11 @@ static int msg_add_element(struct ldb_context *ldb, struct ldb_message_element *e2; unsigned int i; + if (el->num_values == 0) { + /* nothing to do here - we don't add empty elements */ + return 0; + } + e2 = talloc_realloc(msg, msg->elements, struct ldb_message_element, msg->num_elements+1); if (!e2) { @@ -466,21 +476,18 @@ static int msg_add_element(struct ldb_context *ldb, e2->name = el->name; e2->flags = el->flags; - e2->values = NULL; - if (el->num_values != 0) { - e2->values = talloc_array(msg->elements, - struct ldb_val, el->num_values); - if (!e2->values) { - errno = ENOMEM; - return -1; - } + e2->values = talloc_array(msg->elements, + struct ldb_val, el->num_values); + if (!e2->values) { + errno = ENOMEM; + return -1; } for (i=0;inum_values;i++) { e2->values[i] = el->values[i]; } e2->num_values = el->num_values; - msg->num_elements++; + ++msg->num_elements; return 0; } @@ -516,12 +523,17 @@ static int msg_delete_attribute(struct ldb_module *module, msg->num_elements--; i--; msg->elements = talloc_realloc(msg, msg->elements, - struct ldb_message_element, - msg->num_elements); + struct ldb_message_element, + msg->num_elements); + + /* per definition we find in a canonicalised message an + attribute only once. So we are finished here. */ + return 0; } } - return 0; + /* Not found */ + return -1; } /* @@ -562,10 +574,14 @@ static int msg_delete_element(struct ldb_module *module, return msg_delete_attribute(module, ldb, msg, name); } + + /* per definition we find in a canonicalised message an + attribute value only once. So we are finished here */ return 0; } } + /* Not found */ return -1; } @@ -586,7 +602,7 @@ int ltdb_modify_internal(struct ldb_module *module, TDB_DATA tdb_key, tdb_data; struct ldb_message *msg2; unsigned i, j; - int ret, idx; + int ret = LDB_SUCCESS, idx; tdb_key = ltdb_key(module, msg->dn); if (!tdb_key.dptr) { @@ -602,172 +618,174 @@ int ltdb_modify_internal(struct ldb_module *module, msg2 = talloc(tdb_key.dptr, struct ldb_message); if (msg2 == NULL) { free(tdb_data.dptr); - talloc_free(tdb_key.dptr); - return LDB_ERR_OTHER; + ret = LDB_ERR_OTHER; + goto done; } ret = ltdb_unpack_data(module, &tdb_data, msg2); free(tdb_data.dptr); if (ret == -1) { ret = LDB_ERR_OTHER; - goto failed; + goto done; } if (!msg2->dn) { msg2->dn = msg->dn; } - for (i=0;inum_elements;i++) { - struct ldb_message_element *el = &msg->elements[i]; - struct ldb_message_element *el2; + for (i=0; inum_elements; i++) { + struct ldb_message_element *el = &msg->elements[i], *el2; struct ldb_val *vals; - const char *dn; const struct ldb_schema_attribute *a = ldb_schema_attribute_by_name(ldb, el->name); + const char *dn; if (ldb_attr_cmp(el->name, "distinguishedName") == 0) { - ldb_asprintf_errstring(ldb, "it is not permitted to perform a modify on distinguishedName (use rename instead): %s", + ldb_asprintf_errstring(ldb, "it is not permitted to perform a modify on 'distinguishedName' (use rename instead): %s", ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_UNWILLING_TO_PERFORM; - goto failed; + ret = LDB_ERR_CONSTRAINT_VIOLATION; + goto done; } switch (msg->elements[i].flags & LDB_FLAG_MOD_MASK) { case LDB_FLAG_MOD_ADD: - - /* add this element to the message. fail if it - already exists */ - idx = find_element(msg2, el->name); - if (el->num_values == 0) { - ldb_asprintf_errstring(ldb, "attribute %s on %s speicified, but with 0 values (illigal)", - el->name, ldb_dn_get_linearized(msg->dn)); + ldb_asprintf_errstring(ldb, "attribute %s on %s specified, but with 0 values (illigal)", + el->name, ldb_dn_get_linearized(msg->dn)); ret = LDB_ERR_CONSTRAINT_VIOLATION; - goto failed; - } - if (idx == -1) { - if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) { - if (el->num_values > 1) { - ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s speicified more than once", - el->name, ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_CONSTRAINT_VIOLATION; - goto failed; - } - } - if (msg_add_element(ldb, msg2, el) != 0) { - ret = LDB_ERR_OTHER; - goto failed; - } - continue; + goto done; } - /* If this is an add, then if it already - * exists in the object, then we violoate the - * single-value rule */ if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) { - ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto failed; + if (el->num_values > 1) { + ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once", + el->name, ldb_dn_get_linearized(msg->dn)); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; + } } - el2 = &msg2->elements[idx]; - - /* An attribute with this name already exists, - * add all values if they don't already exist - * (check both the other elements to be added, - * and those already in the db). */ - - for (j=0;jnum_values;j++) { - if (ldb_msg_find_val(el2, &el->values[j])) { - ldb_asprintf_errstring(ldb, "%s: value #%d already exists", el->name, j); - ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto failed; + /* Checks if element already exists */ + idx = find_element(msg2, el->name); + if (idx == -1) { + if (msg_add_element(ldb, msg2, el) != 0) { + ret = LDB_ERR_OTHER; + goto done; } - if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) { - ldb_asprintf_errstring(ldb, "%s: value #%d provided more than once", el->name, j); + } else { + /* We cannot add another value on a existing one + if the attribute is single-valued */ + if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) { + ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once", + el->name, ldb_dn_get_linearized(msg->dn)); ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto failed; + goto done; } - } - vals = talloc_realloc(msg2->elements, el2->values, struct ldb_val, - el2->num_values + el->num_values); + el2 = &(msg2->elements[idx]); - if (vals == NULL) { - ret = LDB_ERR_OTHER; - goto failed; - } + /* Check that values don't exist yet on multi- + valued attributes or aren't provided twice */ + for (j=0; jnum_values; j++) { + if (ldb_msg_find_val(el2, &el->values[j]) != NULL) { + ldb_asprintf_errstring(ldb, "%s: value #%d already exists", el->name, j); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; + } + if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) { + ldb_asprintf_errstring(ldb, "%s: value #%d provided more than once", el->name, j); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; + } + } - for (j=0;jnum_values;j++) { - vals[el2->num_values + j] = - ldb_val_dup(vals, &el->values[j]); - } + /* Now combine existing and new values to a new + attribute record */ + vals = talloc_realloc(msg2->elements, + el2->values, struct ldb_val, + el2->num_values + el->num_values); + if (vals == NULL) { + ldb_oom(ldb); + ret = LDB_ERR_OTHER; + goto done; + } + + for (j=0; jnum_values; j++) { + vals[el2->num_values + j] = + ldb_val_dup(vals, &el->values[j]); + } - el2->values = vals; - el2->num_values += el->num_values; + el2->values = vals; + el2->num_values += el->num_values; + } break; case LDB_FLAG_MOD_REPLACE: if (a && a->flags & LDB_ATTR_FLAG_SINGLE_VALUE) { if (el->num_values > 1) { - ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s speicified more than once", - el->name, ldb_dn_get_linearized(msg->dn)); - ret = LDB_ERR_CONSTRAINT_VIOLATION; - goto failed; + ldb_asprintf_errstring(ldb, "SINGLE-VALUE attribute %s on %s specified more than once", + el->name, ldb_dn_get_linearized(msg->dn)); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; } } - /* replace all elements of this attribute name with the elements - listed. The attribute not existing is not an error */ - msg_delete_attribute(module, ldb, msg2, el->name); - for (j=0;jnum_values;j++) { + for (j=0; jnum_values; j++) { if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) { ldb_asprintf_errstring(ldb, "%s: value #%d provided more than once", el->name, j); ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto failed; + goto done; } } - /* add the replacement element, if not empty */ - if (el->num_values != 0 && - msg_add_element(ldb, msg2, el) != 0) { + /* Delete the attribute if it exists in the DB */ + msg_delete_attribute(module, ldb, msg2, el->name); + + /* Recreate it with the new values */ + if (msg_add_element(ldb, msg2, el) != 0) { ret = LDB_ERR_OTHER; - goto failed; + goto done; } + break; case LDB_FLAG_MOD_DELETE: - dn = ldb_dn_get_linearized(msg->dn); if (dn == NULL) { ret = LDB_ERR_OTHER; - goto failed; + goto done; } - /* we could be being asked to delete all - values or just some values */ if (msg->elements[i].num_values == 0) { - if (msg_delete_attribute(module, ldb, msg2, + /* Delete the whole attribute */ + if (msg_delete_attribute(module, ldb, msg2, msg->elements[i].name) != 0) { - ldb_asprintf_errstring(ldb, "No such attribute: %s for delete on %s", msg->elements[i].name, dn); + ldb_asprintf_errstring(ldb, "No such attribute: %s for delete on %s", + msg->elements[i].name, dn); ret = LDB_ERR_NO_SUCH_ATTRIBUTE; - goto failed; + goto done; } - break; - } - for (j=0;jelements[i].num_values;j++) { - if (msg_delete_element(module, - msg2, - msg->elements[i].name, - &msg->elements[i].values[j]) != 0) { - ldb_asprintf_errstring(ldb, "No matching attribute value when deleting attribute: %s on %s", msg->elements[i].name, dn); - ret = LDB_ERR_NO_SUCH_ATTRIBUTE; - goto failed; - } - ret = ltdb_index_del_value(module, dn, &msg->elements[i], j); - if (ret != LDB_SUCCESS) { - goto failed; + } else { + /* Delete specified values from an attribute */ + for (j=0; j < msg->elements[i].num_values; j++) { + if (msg_delete_element(module, + msg2, + msg->elements[i].name, + &msg->elements[i].values[j]) != 0) { + ldb_asprintf_errstring(ldb, "No matching attribute value when deleting attribute: %s on %s", + msg->elements[i].name, dn); + ret = LDB_ERR_NO_SUCH_ATTRIBUTE; + goto done; + } + + ret = ltdb_index_del_value(module, dn, + &msg->elements[i], j); + if (ret != LDB_SUCCESS) { + goto done; + } } } + break; default: ldb_asprintf_errstring(ldb, @@ -775,26 +793,21 @@ int ltdb_modify_internal(struct ldb_module *module, msg->elements[i].name, msg->elements[i].flags & LDB_FLAG_MOD_MASK); ret = LDB_ERR_PROTOCOL_ERROR; - goto failed; + goto done; } } - /* we've made all the mods - * save the modified record back into the database */ ret = ltdb_store(module, msg2, TDB_MODIFY); if (ret != LDB_SUCCESS) { - goto failed; + goto done; } ret = ltdb_modified(module, msg->dn); if (ret != LDB_SUCCESS) { - goto failed; + goto done; } - talloc_free(tdb_key.dptr); - return ret; - -failed: +done: talloc_free(tdb_key.dptr); return ret; } @@ -806,25 +819,22 @@ static int ltdb_modify(struct ltdb_context *ctx) { struct ldb_module *module = ctx->module; struct ldb_request *req = ctx->req; - int tret; - - ldb_request_set_state(req, LDB_ASYNC_PENDING); + int ret = LDB_SUCCESS; - tret = ltdb_check_special_dn(module, req->op.mod.message); - if (tret != LDB_SUCCESS) { - return tret; + ret = ltdb_check_special_dn(module, req->op.mod.message); + if (ret != LDB_SUCCESS) { + return ret; } + ldb_request_set_state(req, LDB_ASYNC_PENDING); + if (ltdb_cache_load(module) != 0) { return LDB_ERR_OPERATIONS_ERROR; } - tret = ltdb_modify_internal(module, req->op.mod.message); - if (tret != LDB_SUCCESS) { - return tret; - } + ret = ltdb_modify_internal(module, req->op.mod.message); - return LDB_SUCCESS; + return ret; } /* @@ -835,7 +845,7 @@ static int ltdb_rename(struct ltdb_context *ctx) struct ldb_module *module = ctx->module; struct ldb_request *req = ctx->req; struct ldb_message *msg; - int tret; + int ret = LDB_SUCCESS; ldb_request_set_state(req, LDB_ASYNC_PENDING); @@ -845,37 +855,37 @@ static int ltdb_rename(struct ltdb_context *ctx) msg = talloc(ctx, struct ldb_message); if (msg == NULL) { - return LDB_ERR_OPERATIONS_ERROR; + ret = LDB_ERR_OPERATIONS_ERROR; + goto done; } /* in case any attribute of the message was indexed, we need to fetch the old record */ - tret = ltdb_search_dn1(module, req->op.rename.olddn, msg); - if (tret != LDB_SUCCESS) { + ret = ltdb_search_dn1(module, req->op.rename.olddn, msg); + if (ret != LDB_SUCCESS) { /* not finding the old record is an error */ - return tret; + goto done; } msg->dn = ldb_dn_copy(msg, req->op.rename.newdn); - if (!msg->dn) { - return LDB_ERR_OPERATIONS_ERROR; + if (msg->dn == NULL) { + ret = LDB_ERR_OPERATIONS_ERROR; + goto done; } /* Always delete first then add, to avoid conflicts with * unique indexes. We rely on the transaction to make this * atomic */ - tret = ltdb_delete_internal(module, req->op.rename.olddn); - if (tret != LDB_SUCCESS) { - return tret; + ret = ltdb_delete_internal(module, req->op.rename.olddn); + if (ret != LDB_SUCCESS) { + goto done; } - tret = ltdb_add_internal(module, msg); - if (tret != LDB_SUCCESS) { - return tret; - } + ret = ltdb_add_internal(module, msg); - return LDB_SUCCESS; +done: + return ret; } static int ltdb_start_trans(struct ldb_module *module) @@ -975,7 +985,7 @@ static int ltdb_sequence_number(struct ltdb_context *ctx, struct ldb_message *msg = NULL; struct ldb_dn *dn; const char *date; - int ret; + int ret = LDB_SUCCESS; ldb = ldb_module_get_ctx(module); @@ -1042,8 +1052,6 @@ static int ltdb_sequence_number(struct ltdb_context *ctx, (*ext)->oid = LDB_EXTENDED_SEQUENCE_NUMBER; (*ext)->data = talloc_steal(*ext, res); - ret = LDB_SUCCESS; - done: talloc_free(tmp_ctx); ltdb_unlock_read(module); @@ -1228,7 +1236,7 @@ static int ltdb_handle_request(struct ldb_module *module, ac = talloc_zero(ldb, struct ltdb_context); if (ac == NULL) { - ldb_set_errstring(ldb, "Out of Memory"); + ldb_oom(ldb); return LDB_ERR_OPERATIONS_ERROR; } -- 2.34.1