r11958: - fixed memory leaks in the ldb_result handling in ldb operations

- removed an unnecessary level of pointer in ldb_search structure
(This used to be commit b8d4afb14a18dfd8bac79882a035e74d3ed312bd)

6 files changed, 99 insertions, 90 deletions

diff --git a/source4/lib/ldb/common/ldb.c b/source4/lib/ldb/common/ldb.c
index ea99bf3e8b..3dc62fd4d6 100644
--- a/source4/lib/ldb/common/ldb.c
+++ b/source4/lib/ldb/common/ldb.c
@@ -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);
 }
 
 
diff --git a/source4/lib/ldb/include/ldb.h b/source4/lib/ldb/include/ldb.h
index 86e6a021da..2fdf40b3bc 100644
--- a/source4/lib/ldb/include/ldb.h
+++ b/source4/lib/ldb/include/ldb.h
@@ -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 {
diff --git a/source4/lib/ldb/ldb_ildap/ldb_ildap.c b/source4/lib/ldb/ldb_ildap/ldb_ildap.c
index cc9b5e65db..e195ec24aa 100644
--- a/source4/lib/ldb/ldb_ildap/ldb_ildap.c
+++ b/source4/lib/ldb/ldb_ildap/ldb_ildap.c
@@ -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);
diff --git a/source4/lib/ldb/ldb_tdb/ldb_tdb.c b/source4/lib/ldb/ldb_tdb/ldb_tdb.c
index d1e60e0f5d..0de7a3e558 100644
--- a/source4/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/source4/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -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);
diff --git a/source4/lib/ldb/modules/ldb_map.c b/source4/lib/ldb/modules/ldb_map.c
index 49261d155c..b056521f9f 100644
--- a/source4/lib/ldb/modules/ldb_map.c
+++ b/source4/lib/ldb/modules/ldb_map.c
@@ -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;
 }
 
diff --git a/source4/lib/ldb/modules/operational.c b/source4/lib/ldb/modules/operational.c
index 7c554e8541..2fdd255e99 100644
--- a/source4/lib/ldb/modules/operational.c
+++ b/source4/lib/ldb/modules/operational.c
@@ -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;
 }