r11958: - fixed memory leaks in the ldb_result handling in ldb operations
authorAndrew Tridgell <tridge@samba.org>
Tue, 29 Nov 2005 12:34:03 +0000 (12:34 +0000)
committerGerald (Jerry) Carter <jerry@samba.org>
Wed, 10 Oct 2007 18:46:51 +0000 (13:46 -0500)
- removed an unnecessary level of pointer in ldb_search structure
(This used to be commit b8d4afb14a18dfd8bac79882a035e74d3ed312bd)

source4/dsdb/samdb/ldb_modules/proxy.c
source4/dsdb/samdb/ldb_modules/rootdse.c
source4/ldap_server/ldap_simple_ldb.c
source4/lib/ldb/common/ldb.c
source4/lib/ldb/include/ldb.h
source4/lib/ldb/ldb_ildap/ldb_ildap.c
source4/lib/ldb/ldb_tdb/ldb_tdb.c
source4/lib/ldb/modules/ldb_map.c
source4/lib/ldb/modules/operational.c

index 643ff5f3d0e0008d28995ee2625348af84ca98ef..a567db689d3895a5030b103a96ae1aaf164a1114 100644 (file)
@@ -295,14 +295,14 @@ static int proxy_search_bytree(struct ldb_module *module, struct ldb_request *re
                return -1;
        }
 
-       for (i = 0; i < (*newreq.op.search.res)->count; i++) {
+       for (i = 0; i < newreq.op.search.res->count; i++) {
                struct ldb_ldif ldif;
                printf("# record %d\n", i+1);
                
-               proxy_convert_record(module, (*newreq.op.search.res)->msgs[i]);
+               proxy_convert_record(module, newreq.op.search.res->msgs[i]);
 
                ldif.changetype = LDB_CHANGETYPE_NONE;
-               ldif.msg = (*newreq.op.search.res)->msgs[i];
+               ldif.msg = newreq.op.search.res->msgs[i];
        }
 
        return ret;
index df12011d8972a4d0b2b37f76464212097f82b73c..4032aee8b298a988cc9d12abdc0c6b5a87c831c1 100644 (file)
@@ -46,11 +46,11 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_request *re
 
        /* this is gross, and will be removed when I change ldb_result not
           to be so pointer crazy :-) */
-       if (s->res[0][0].msgs == NULL) {
+       if (s->res->msgs == NULL) {
                return LDB_SUCCESS;
        }
 
-       msg = s->res[0][0].msgs[0];
+       msg = s->res->msgs[0];
 
        msg->dn = ldb_dn_explode(msg, "");
 
@@ -74,8 +74,7 @@ failed:
 */
 static int rootdse_search_bytree(struct ldb_module *module, struct ldb_request *req)
 {
-       struct ldb_request r = *req;
-       struct ldb_search *s = &r.op.search;
+       struct ldb_search *s = &req->op.search;
        int ret;
        TALLOC_CTX *tmp_ctx;
 
@@ -97,7 +96,7 @@ static int rootdse_search_bytree(struct ldb_module *module, struct ldb_request *
        }
 
        /* grab the static contents of the record */
-       ret = ldb_next_request(module, &r);
+       ret = ldb_next_request(module, req);
 
        req->op.search.res = s->res;
 
index 481db6052d83345b45ab30adc3bf7c98ca5cd822..b9ef085002d1f6b9aaa709e7332c07a0179421a6 100644 (file)
@@ -171,10 +171,10 @@ static NTSTATUS sldb_Search(struct ldapsrv_partition *partition, struct ldapsrv_
        lreq.op.search.scope = scope;
        lreq.op.search.tree = r->tree;
        lreq.op.search.attrs = attrs;
-       lreq.op.search.res = &res;
 
        ret = ldb_request(samdb, &lreq);
-       talloc_steal(samdb, res);
+
+       res = talloc_steal(samdb, lreq.op.search.res);
 
        if (ret == LDB_SUCCESS) {
                for (i = 0; i < res->count; i++) {
index ea99bf3e8bc31d5942b980d868147b46b8214e30..3dc62fd4d635e9157bd255a66def027fce5f7a42 100644 (file)
@@ -192,29 +192,60 @@ static int ldb_op_finish(struct ldb_context *ldb, int status)
 /*
   start an ldb request
   autostarts a transacion if none active and the operation is not a search
-  returns -1 on errors.
+  returns LDB_ERR_* on errors.
 */
-
 int ldb_request(struct ldb_context *ldb, struct ldb_request *request)
 {
-       int status;
+       int status, started_transaction=0;
+       struct ldb_request *r;
 
        ldb_reset_err_string(ldb);
 
+       /* to allow ldb modules to assume they can use the request ptr
+          as a talloc context for the request, we have to copy the 
+          structure here */
+       r = talloc(ldb, struct ldb_request);
+       if (r == NULL) {
+               ldb_oom(ldb);
+               return LDB_ERR_OPERATIONS_ERROR;
+       }
+
+       *r = *request;
+
+       if (r->operation == LDB_REQ_SEARCH) {
+               r->op.search.res = NULL;
+       }
+
+       /* start a transaction if needed */
        if ((!ldb->transaction_active) &&
            (request->operation == LDB_REQ_ADD ||
             request->operation == LDB_REQ_MODIFY ||
             request->operation == LDB_REQ_DELETE ||
             request->operation == LDB_REQ_RENAME)) {
-
                status = ldb_transaction_start(ldb);
-               if (status != LDB_SUCCESS) return status;
+               if (status != LDB_SUCCESS) {
+                       talloc_free(r);
+                       return status;
+               }
+               started_transaction = 1;
+       }
 
-               status = ldb->modules->ops->request(ldb->modules, request);
+       /* call the first module in the chain */
+       status = ldb->modules->ops->request(ldb->modules, r);
+
+       /* the search call is the only one that returns something
+          other than a status code. We steal the results into
+          the context of the ldb before freeing the request */
+       if (request->operation == LDB_REQ_SEARCH) {
+               request->op.search.res = talloc_steal(ldb, r->op.search.res);
+       }
+       talloc_free(r);
+
+       if (started_transaction) {
                return ldb_op_finish(ldb, status);
        }
 
-       return ldb->modules->ops->request(ldb->modules, request);
+       return status;
 }
 
 /*
@@ -229,34 +260,30 @@ int ldb_search(struct ldb_context *ldb,
               const struct ldb_dn *base,
               enum ldb_scope scope,
               const char *expression,
-              const char * const *attrs, struct ldb_result **res)
+              const char * const *attrs, 
+              struct ldb_result **res)
 {
-       struct ldb_request *request;
+       struct ldb_request request;
        struct ldb_parse_tree *tree;
        int ret;
 
        (*res) = NULL;
 
-       request = talloc(ldb, struct ldb_request);
-       if (request == NULL) {
-               ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory"));
-               return -1;
-       }
-
        tree = ldb_parse_tree(ldb, expression);
        if (tree == NULL) {
                ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Unable to parse search expression"));
                return -1;
        }
 
-       request->operation = LDB_REQ_SEARCH;
-       request->op.search.base = base;
-       request->op.search.scope = scope;
-       request->op.search.tree = tree;
-       request->op.search.attrs = attrs;
-       request->op.search.res = res;
+       request.operation = LDB_REQ_SEARCH;
+       request.op.search.base = base;
+       request.op.search.scope = scope;
+       request.op.search.tree = tree;
+       request.op.search.attrs = attrs;
+
+       ret = ldb_request(ldb, &request);
 
-       ret = ldb_request(ldb, request);
+       (*res) = request.op.search.res;
 
        talloc_free(tree);
 
@@ -270,22 +297,16 @@ int ldb_search(struct ldb_context *ldb,
 int ldb_add(struct ldb_context *ldb, 
            const struct ldb_message *message)
 {
-       struct ldb_request *request;
+       struct ldb_request request;
        int status;
 
        status = ldb_msg_sanity_check(message);
        if (status != LDB_SUCCESS) return status;
 
-       request = talloc(ldb, struct ldb_request);
-       if (request == NULL) {
-               ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory"));
-               return -1;
-       }
+       request.operation = LDB_REQ_ADD;
+       request.op.add.message = message;
 
-       request->operation = LDB_REQ_ADD;
-       request->op.add.message = message;
-
-       return ldb_request(ldb, request);
+       return ldb_request(ldb, &request);
 }
 
 /*
@@ -294,22 +315,16 @@ int ldb_add(struct ldb_context *ldb,
 int ldb_modify(struct ldb_context *ldb, 
               const struct ldb_message *message)
 {
-       struct ldb_request *request;
+       struct ldb_request request;
        int status;
 
        status = ldb_msg_sanity_check(message);
        if (status != LDB_SUCCESS) return status;
 
-       request = talloc(ldb, struct ldb_request);
-       if (request == NULL) {
-               ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory"));
-               return -1;
-       }
-
-       request->operation = LDB_REQ_MODIFY;
-       request->op.mod.message = message;
+       request.operation = LDB_REQ_MODIFY;
+       request.op.mod.message = message;
 
-       return ldb_request(ldb, request);
+       return ldb_request(ldb, &request);
 }
 
 
@@ -318,18 +333,12 @@ int ldb_modify(struct ldb_context *ldb,
 */
 int ldb_delete(struct ldb_context *ldb, const struct ldb_dn *dn)
 {
-       struct ldb_request *request;
-
-       request = talloc(ldb, struct ldb_request);
-       if (request == NULL) {
-               ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory"));
-               return -1;
-       }
+       struct ldb_request request;
 
-       request->operation = LDB_REQ_DELETE;
-       request->op.del.dn = dn;
+       request.operation = LDB_REQ_DELETE;
+       request.op.del.dn = dn;
 
-       return ldb_request(ldb, request);
+       return ldb_request(ldb, &request);
 }
 
 /*
@@ -337,19 +346,13 @@ int ldb_delete(struct ldb_context *ldb, const struct ldb_dn *dn)
 */
 int ldb_rename(struct ldb_context *ldb, const struct ldb_dn *olddn, const struct ldb_dn *newdn)
 {
-       struct ldb_request *request;
-
-       request = talloc(ldb, struct ldb_request);
-       if (request == NULL) {
-               ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory"));
-               return -1;
-       }
+       struct ldb_request request;
 
-       request->operation = LDB_REQ_RENAME;
-       request->op.rename.olddn = olddn;
-       request->op.rename.newdn = newdn;
+       request.operation = LDB_REQ_RENAME;
+       request.op.rename.olddn = olddn;
+       request.op.rename.newdn = newdn;
 
-       return ldb_request(ldb, request);
+       return ldb_request(ldb, &request);
 }
 
 
index 86e6a021da59d6aa48272e56618ad304f5eafcc5..2fdf40b3bc63749479dedaad0899f715b8cc64b6 100644 (file)
@@ -274,7 +274,7 @@ struct ldb_search {
        enum ldb_scope scope;
        struct ldb_parse_tree *tree;
        const char * const *attrs;
-       struct ldb_result **res;
+       struct ldb_result *res;
 };
 
 struct ldb_add {
index cc9b5e65dbbf57bbf25072c7af1522982a8f2cab..e195ec24aa08d8a2e709290df0b293b7cc9c6ff3 100644 (file)
@@ -407,7 +407,7 @@ static int ildb_request(struct ldb_module *module, struct ldb_request *req)
                                          req->op.search.scope, 
                                          req->op.search.tree, 
                                          req->op.search.attrs, 
-                                         req->op.search.res);
+                                         &req->op.search.res);
 
        case LDB_REQ_ADD:
                return ildb_add(module, req->op.add.message);
index d1e60e0f5da6de9f14a89ced84c2cfa48e633e28..0de7a3e558a5b47fd7c5e8ececcd0bc87dc43a27 100644 (file)
@@ -743,7 +743,7 @@ static int ltdb_request(struct ldb_module *module, struct ldb_request *req)
                                          req->op.search.scope, 
                                          req->op.search.tree, 
                                          req->op.search.attrs, 
-                                         req->op.search.res);
+                                         &req->op.search.res);
 
        case LDB_REQ_ADD:
                return ltdb_add(module, req->op.add.message);
index 49261d155cca7dec83b48e7902f3db143e7ec223..b056521f9f629389af210821d2f9dbfde34a557d 100644 (file)
@@ -786,7 +786,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
        enum ldb_scope scope = req->op.search.scope;
        struct ldb_parse_tree *tree = req->op.search.tree;
        const char * const *attrs = req->op.search.attrs;
-       struct ldb_result **res = req->op.search.res;
+       struct ldb_result *res;
        struct ldb_request new_req;
        struct ldb_parse_tree *new_tree;
        struct ldb_dn *new_base;
@@ -818,9 +818,11 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
        new_req.op.search.scope = scope;
        new_req.op.search.tree = new_tree;
        new_req.op.search.attrs = newattrs;
-       new_req.op.search.res = &newres;
+
        mpret = ldb_request(privdat->mapped_ldb, req);
 
+       newres = new_req.op.search.res;
+
        talloc_free(new_base);
        talloc_free(new_tree);
        talloc_free(newattrs);
@@ -835,9 +837,10 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
         - test if (full expression) is now true
        */
 
-       *res = talloc(module, struct ldb_result);
-       (*res)->msgs = talloc_array(module, struct ldb_message *, newres->count);
-       (*res)->count = newres->count;
+       res = talloc(module, struct ldb_result);
+       req->op.search.res = res;
+       res->msgs = talloc_array(module, struct ldb_message *, newres->count);
+       res->count = newres->count;
 
        ret = 0;
 
@@ -860,10 +863,11 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
                mergereq.op.search.scope = LDB_SCOPE_BASE;
                mergereq.op.search.tree = ldb_parse_tree(module, "");
                mergereq.op.search.attrs = NULL;
-               mergereq.op.search.res = &extrares;
 
                extraret = ldb_next_request(module, &mergereq);
 
+               extrares = mergereq.op.search.res;
+
                if (extraret == -1) {
                        ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Error searching for extra data!\n");
                } else if (extraret > 1) {
@@ -883,7 +887,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
                }
                
                if (ldb_match_msg(module->ldb, merged, tree, base, scope) != 0) {
-                       (*res)->msgs[ret] = merged;
+                       res->msgs[ret] = merged;
                        ret++;
                } else {
                        ldb_debug(module->ldb, LDB_DEBUG_TRACE, "Discarded merged message because it did not match");
@@ -892,7 +896,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
 
        talloc_free(newres);
 
-       (*res)->count = ret;
+       res->count = ret;
        return LDB_SUCCESS;
 }
 
@@ -903,39 +907,41 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req)
 static int map_search_bytree(struct ldb_module *module, struct ldb_request *req)
 {
        const struct ldb_dn *base = req->op.search.base;
-       struct ldb_result **res = req->op.search.res;
-       struct ldb_result *fbres, *mpres = NULL;
+       struct ldb_result *fbres, *mpres, *res;
        int i, ret;
 
-       req->op.search.res = &fbres;
        ret = map_search_fb(module, req);
-       req->op.search.res = res;
        if (ret != LDB_SUCCESS)
                return ret;
 
        /* special dn's are never mapped.. */
        if (ldb_dn_is_special(base)) {
-               *res = fbres;
                return ret;
        }
 
-       req->op.search.res = &mpres;
+       fbres = req->op.search.res;
+
        ret = map_search_mp(module, req);
-       req->op.search.res = res;
        if (ret != LDB_SUCCESS) {
                return ret;
        }
 
+       mpres = req->op.search.res;
+
        /* Merge results */
-       *res = talloc(module, struct ldb_result);
-       (*res)->msgs = talloc_array(*res, struct ldb_message *, fbres->count + mpres->count);
+       res = talloc(module, struct ldb_result);
+       res->msgs = talloc_array(res, struct ldb_message *, fbres->count + mpres->count);
 
        ldb_debug(module->ldb, LDB_DEBUG_TRACE, "Merging %d mapped and %d fallback messages", mpres->count, fbres->count);
 
-       for (i = 0; i < fbres->count; i++) (*res)->msgs[i] = fbres->msgs[i];
-       for (i = 0; i < mpres->count; i++) (*res)->msgs[fbres->count + i] = mpres->msgs[i];
+       for (i = 0; i < fbres->count; i++) {
+               res->msgs[i] = talloc_steal(res->msgs, fbres->msgs[i]);
+       }
+       for (i = 0; i < mpres->count; i++) {
+               res->msgs[fbres->count + i] = talloc_steal(res->msgs, mpres->msgs[i]);
+       }
 
-       (*res)->count = fbres->count + mpres->count;
+       res->count = fbres->count + mpres->count;
        return LDB_SUCCESS;
 }
 
index 7c554e8541bc1b86213f455cda66e25cffb9d45f..2fdd255e991219fb3b04b7b119960e207aaee887 100644 (file)
@@ -182,7 +182,7 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque
        const char * const *attrs = req->op.search.attrs;
        const char **search_attrs = NULL;
 
-       *(req->op.search.res) = NULL;
+       req->op.search.res = NULL;
 
        /* replace any attributes in the parse tree that are
           searchable, but are stored using a different name in the
@@ -225,8 +225,8 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque
 
        /* for each record returned post-process to add any derived
           attributes that have been asked for */
-       for (r = 0; r < (*(req->op.search.res))->count; r++) {
-               if (operational_search_post_process(module, (*(req->op.search.res))->msgs[r], attrs) != 0) {
+       for (r = 0; r < req->op.search.res->count; r++) {
+               if (operational_search_post_process(module, req->op.search.res->msgs[r], attrs) != 0) {
                        goto failed;
                }
        }
@@ -237,7 +237,7 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque
 
 failed:
        talloc_free(search_attrs);
-       talloc_free(*(req->op.search.res));
+       talloc_free(req->op.search.res);
        ldb_oom(module->ldb);
        return LDB_ERR_OTHER;
 }