s4:samldb LDB module - simplify/unify the message handling on add and modify operations

- Perform only shallow copies (should be enough)
- Perform only one copy per operation (also on modifications)
- Build a new request on modify operations if needed ("modified" flag) - this
  makes it look cleaner
- Fix an important bug: the "el" pointers could have changed after
  modifications. Therefore we have to refresh them on the FLAG_DELETE checks

Autobuild-User: Matthias Dieter Wallnöfer <mdw@samba.org>
Autobuild-Date: Tue Oct  5 09:24:57 UTC 2010 on sn-devel-104

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 0e4d852d1562b243dfa68dceb0b7eaf64d997d5d..f3a9e08e7277bd6fa6940fe925f91c7c057af8f2 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1093,11 +1093,11 @@ static int samldb_add(struct ldb_module *module, struct ldb_request *req)
        }
 
        /* build the new msg */
-       ac->msg = ldb_msg_copy(ac, ac->req->op.add.message);
-       if (!ac->msg) {
+       ac->msg = ldb_msg_copy_shallow(ac, req->op.add.message);
+       if (ac->msg == NULL) {
                talloc_free(ac);
                ldb_debug(ldb, LDB_DEBUG_FATAL,
-                         "samldb_add: ldb_msg_copy failed!\n");
+                         "samldb_add: ldb_msg_copy_shallow failed!\n");
                return ldb_operr(ldb);
        }
 
@@ -1151,8 +1151,8 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 {
        struct ldb_context *ldb;
        struct samldb_ctx *ac;
-       struct ldb_message *msg;
        struct ldb_message_element *el, *el2;
+       bool modified = false;
        int ret;
        uint32_t account_type;
 
@@ -1182,101 +1182,127 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
                return ldb_operr(ldb);
        }
 
-       /* TODO: do not modify original request, create a new one */
+       /* build the new msg */
+       ac->msg = ldb_msg_copy_shallow(ac, req->op.mod.message);
+       if (ac->msg == NULL) {
+               talloc_free(ac);
+               ldb_debug(ldb, LDB_DEBUG_FATAL,
+                         "samldb_modify: ldb_msg_copy_shallow failed!\n");
+               return ldb_operr(ldb);
+       }
 
-       el = ldb_msg_find_element(req->op.mod.message, "groupType");
+       el = ldb_msg_find_element(ac->msg, "groupType");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
                uint32_t group_type;
 
-               req->op.mod.message = msg = ldb_msg_copy_shallow(req,
-                       req->op.mod.message);
+               modified = true;
 
                group_type = strtoul((const char *)el->values[0].data, NULL, 0);
                account_type =  ds_gtype2atype(group_type);
-               ret = samdb_msg_add_uint(ldb, msg, msg,
+               ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
                                         "sAMAccountType",
                                         account_type);
                if (ret != LDB_SUCCESS) {
                        return ret;
                }
-               el2 = ldb_msg_find_element(msg, "sAMAccountType");
+               el2 = ldb_msg_find_element(ac->msg, "sAMAccountType");
                el2->flags = LDB_FLAG_MOD_REPLACE;
        }
+       el = ldb_msg_find_element(ac->msg, "groupType");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE)) {
                return LDB_ERR_UNWILLING_TO_PERFORM;
        }
 
-       el = ldb_msg_find_element(req->op.mod.message, "primaryGroupID");
+       el = ldb_msg_find_element(ac->msg, "primaryGroupID");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
-               req->op.mod.message = ac->msg = ldb_msg_copy_shallow(req,
-                       req->op.mod.message);
+               modified = true;
 
                ret = samldb_prim_group_change(ac);
                if (ret != LDB_SUCCESS) {
                        return ret;
                }
        }
+       el = ldb_msg_find_element(ac->msg, "primaryGroupID");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE)) {
                return LDB_ERR_UNWILLING_TO_PERFORM;
        }
 
-       el = ldb_msg_find_element(req->op.mod.message, "userAccountControl");
+       el = ldb_msg_find_element(ac->msg, "userAccountControl");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
                uint32_t user_account_control;
 
-               req->op.mod.message = msg = ldb_msg_copy_shallow(req,
-                       req->op.mod.message);
+               modified = true;
 
                user_account_control = strtoul((const char *)el->values[0].data,
                        NULL, 0);
                account_type = ds_uf2atype(user_account_control);
-               ret = samdb_msg_add_uint(ldb, msg, msg,
+               ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
                                         "sAMAccountType",
                                         account_type);
                if (ret != LDB_SUCCESS) {
                        return ret;
                }
-               el2 = ldb_msg_find_element(msg, "sAMAccountType");
+               el2 = ldb_msg_find_element(ac->msg, "sAMAccountType");
                el2->flags = LDB_FLAG_MOD_REPLACE;
 
                if (user_account_control & (UF_SERVER_TRUST_ACCOUNT | UF_PARTIAL_SECRETS_ACCOUNT)) {
-                       ret = samdb_msg_add_string(ldb, msg, msg,
-                                                  "isCriticalSystemObject", "TRUE");
+                       ret = samdb_msg_add_string(ldb, ac->msg, ac->msg,
+                                                  "isCriticalSystemObject",
+                                                  "TRUE");
                        if (ret != LDB_SUCCESS) {
                                return ret;
                        }
-                       el2 = ldb_msg_find_element(msg, "isCriticalSystemObject");
+                       el2 = ldb_msg_find_element(ac->msg,
+                                                  "isCriticalSystemObject");
                        el2->flags = LDB_FLAG_MOD_REPLACE;
                }
 
-               if (!ldb_msg_find_element(msg, "primaryGroupID")) {
+               if (!ldb_msg_find_element(ac->msg, "primaryGroupID")) {
                        uint32_t rid = ds_uf2prim_group_rid(user_account_control);
 
-                       ret = samdb_msg_add_uint(ldb, msg, msg,
+                       ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
                                                 "primaryGroupID", rid);
                        if (ret != LDB_SUCCESS) {
                                return ret;
                        }
-                       el2 = ldb_msg_find_element(msg,
+                       el2 = ldb_msg_find_element(ac->msg,
                                                   "primaryGroupID");
                        el2->flags = LDB_FLAG_MOD_REPLACE;
                }
        }
+       el = ldb_msg_find_element(ac->msg, "userAccountControl");
        if (el && (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE)) {
                return LDB_ERR_UNWILLING_TO_PERFORM;
        }
 
-       el = ldb_msg_find_element(req->op.mod.message, "member");
+       el = ldb_msg_find_element(ac->msg, "member");
        if (el && el->flags & (LDB_FLAG_MOD_ADD|LDB_FLAG_MOD_REPLACE) && el->num_values == 1) {
-               req->op.mod.message = ac->msg = ldb_msg_copy_shallow(req,
-                       req->op.mod.message);
-
                ret = samldb_member_check(ac);
                if (ret != LDB_SUCCESS) {
                        return ret;
                }
        }
 
+       if (modified) {
+               struct ldb_request *child_req;
+
+               /* Now perform the real modifications as a child request */
+               ret = ldb_build_mod_req(&child_req, ldb, ac,
+                                       ac->msg,
+                                       req->controls,
+                                       req, dsdb_next_callback,
+                                       req);
+               LDB_REQ_SET_LOCATION(child_req);
+               if (ret != LDB_SUCCESS) {
+                       return ret;
+               }
+
+               return ldb_next_request(module, child_req);
+       }
+
+       talloc_free(ac);
+
+       /* no change which interests us, go on */
        return ldb_next_request(module, req);
 }