s4:ldb_tdb - Rework/Various
authorMatthias Dieter Wallnöfer <mwallnoefer@yahoo.de>
Tue, 6 Oct 2009 07:30:53 +0000 (09:30 +0200)
committerMatthias Dieter Wallnöfer <mwallnoefer@yahoo.de>
Wed, 7 Oct 2009 22:18:06 +0000 (00:18 +0200)
- 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

index 66a10b64138ecfa111b76aa9caf492c9fda728b8..7693ffeb81c566769d70b394982e5116a2d5de0b 100644 (file)
@@ -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
  *  - 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;i<msg->num_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;i<el->num_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;i<msg->num_elements;i++) {
-               struct ldb_message_element *el = &msg->elements[i];
-               struct ldb_message_element *el2;
+       for (i=0; i<msg->num_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;j<el->num_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; j<el->num_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;j<el->num_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; j<el->num_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;j<el->num_values;j++) {
+                       for (j=0; j<el->num_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;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 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;
        }