summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimo Sorce <idra@samba.org>2009-01-28 18:04:42 -0500
committerSimo Sorce <idra@samba.org>2009-01-28 18:04:42 -0500
commitb34dd4805a8b0c7e91a80268607fe28378637b33 (patch)
treeb10aab2f1f9b31f8c6004222dae9a5747daaeb84
parentb4e5ec1ebed65ed3ed85fc3eb775a3c762b9b585 (diff)
downloadsssd-b34dd4805a8b0c7e91a80268607fe28378637b33.tar.gz
sssd-b34dd4805a8b0c7e91a80268607fe28378637b33.tar.bz2
sssd-b34dd4805a8b0c7e91a80268607fe28378637b33.zip
Simplify delete path by removing effectively redundant code.
Thanks Nathan for the review that lead to this!
-rw-r--r--server/ldb_modules/memberof.c262
1 files changed, 41 insertions, 221 deletions
diff --git a/server/ldb_modules/memberof.c b/server/ldb_modules/memberof.c
index aa498032..05418adb 100644
--- a/server/ldb_modules/memberof.c
+++ b/server/ldb_modules/memberof.c
@@ -37,16 +37,6 @@ struct mbof_dn {
struct ldb_dn *dn;
};
-struct mbof_array_entry {
- struct ldb_dn *dn;
- struct ldb_message *pmsg;
-};
-
-struct mbof_array {
- struct mbof_array_entry *entries;
- int num;
-};
-
struct mbof_ctx {
struct ldb_module *module;
struct ldb_request *req;
@@ -73,11 +63,11 @@ struct mbof_add_ctx {
};
struct mbof_del_ancestors_ctx {
- struct mbof_dn_array *orig_list;
- struct mbof_array *new_list;
+ struct mbof_dn_array *new_list;
+ int num_direct;
+ int cur;
struct ldb_message *entry;
- int cur;
};
struct mbof_del_operation {
@@ -88,7 +78,6 @@ struct mbof_del_operation {
int cur_child;
struct ldb_dn *entry_dn;
- struct mbof_dn_array *rm_list;
struct ldb_message *entry;
struct ldb_message **parents;
@@ -635,9 +624,8 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
*
* Then we proceed to calculate the new memberof list that we are going to set
* on the target object.
- * The new memberof list starts with including the original memberof list
- * excluding each entry in the parent remove list. Then we readd any object that
- * has the target as a direct member.
+ * The new memberof list starts with including all the objects that have the
+ * target as their direct member.
* Finally for each entry in this provisional new memberof list we add all its
* memberof elements to the new memberof list (taking care of excluding
* duplicates). This way we are certain all direct and indirect membership are
@@ -668,8 +656,6 @@ static int mbof_del_cleanup_parents(struct mbof_del_ctx *del_ctx);
static int mbof_del_clean_par_callback(struct ldb_request *req,
struct ldb_reply *ares);
static int mbof_del_cleanup_children(struct mbof_del_ctx *del_ctx);
-static int mbof_build_first_rm_list(struct ldb_context *ldb,
- struct mbof_del_operation *first);
static int mbof_append_delop(struct mbof_del_operation *parent,
struct ldb_dn *entry_dn);
static int mbof_del_execute_op(struct mbof_del_operation *delop);
@@ -1036,11 +1022,6 @@ static int mbof_del_cleanup_children(struct mbof_del_ctx *del_ctx)
ctx = del_ctx->ctx;
ldb = ldb_module_get_ctx(ctx->module);
- ret = mbof_build_first_rm_list(ldb, first);
- if (ret != LDB_SUCCESS) {
- return ret;
- }
-
el = ldb_msg_find_element(first->entry, "member");
/* prepare del sets */
@@ -1059,58 +1040,6 @@ static int mbof_del_cleanup_children(struct mbof_del_ctx *del_ctx)
return mbof_del_execute_op(first->children[0]);
}
-static int mbof_build_first_rm_list(struct ldb_context *ldb,
- struct mbof_del_operation *first)
-{
- const struct ldb_message_element *el;
- struct mbof_dn_array *rm_list;
- struct ldb_dn *valdn;
- int i;
-
- rm_list = talloc_zero(first, struct mbof_dn_array);
- if (!rm_list) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- /* create memberof removal list if part of any group */
- el = ldb_msg_find_element(first->entry, "memberof");
- if (el) {
- rm_list->dns = talloc_array(rm_list, struct ldb_dn *,
- el->num_values + 1);
- if (!rm_list->dns) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- rm_list->num = el->num_values + 1;
-
- for (i = 0; i < el->num_values; i++) {
- valdn = ldb_dn_from_ldb_val(rm_list, ldb, &el->values[i]);
- if (!valdn || !ldb_dn_validate(valdn)) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- rm_list->dns[i] = valdn;
- }
- }
-
- if (!rm_list->dns) {
- rm_list->dns = talloc_array(rm_list, struct ldb_dn *, 1);
- if (!rm_list->dns) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- rm_list->num = 1;
- i = 0;
- }
-
- rm_list->dns[i] = first->entry_dn;
- if (!rm_list->dns[i]) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- first->rm_list = rm_list;
-
- return LDB_SUCCESS;
-}
-
static int mbof_append_delop(struct mbof_del_operation *parent,
struct ldb_dn *entry_dn)
{
@@ -1261,7 +1190,6 @@ static int mbof_del_exop_search_callback(struct ldb_request *req,
return LDB_SUCCESS;
}
-/* FIXME: very inefficient */
static int mbof_del_execute_cont(struct mbof_del_operation *delop)
{
struct mbof_del_ancestors_ctx *anc_ctx;
@@ -1269,12 +1197,8 @@ static int mbof_del_execute_cont(struct mbof_del_operation *delop)
struct mbof_del_ctx *del_ctx;
struct mbof_ctx *ctx;
struct ldb_context *ldb;
- const struct ldb_message_element *el;
- struct mbof_dn_array *orig_list;
- struct mbof_array *new_list;
- struct ldb_dn *valdn;
- int i, j;
- bool found;
+ struct mbof_dn_array *new_list;
+ int i;
del_ctx = delop->del_ctx;
parent = delop->parent;
@@ -1287,99 +1211,21 @@ static int mbof_del_execute_cont(struct mbof_del_operation *delop)
}
delop->anc_ctx = anc_ctx;
- orig_list = talloc_zero(anc_ctx, struct mbof_dn_array);
- if (!orig_list) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- anc_ctx->orig_list = orig_list;
-
- new_list = talloc_zero(anc_ctx, struct mbof_array);
+ new_list = talloc_zero(anc_ctx, struct mbof_dn_array);
if (!new_list) {
return LDB_ERR_OPERATIONS_ERROR;
}
- /* create new memberof list */
- el = ldb_msg_find_element(delop->entry, "memberof");
- if (!el) {
- /* can't be, if we get there it is because this entry
- * had at least one parent */
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- orig_list->num = el->num_values;
- orig_list->dns = talloc_zero_array(orig_list,
- struct ldb_dn *,
- orig_list->num);
- if (!orig_list->dns) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- new_list->entries = talloc_zero_array(new_list,
- struct mbof_array_entry,
- orig_list->num);
- if (!new_list->entries) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- /* add to the list only the values that are not on the rm_list */
- for (i = 0; i < el->num_values; i++) {
- valdn = ldb_dn_from_ldb_val(orig_list, ldb, &el->values[i]);
- if (!valdn) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- orig_list->dns[i] = valdn;
- for (j = 0; j < parent->rm_list->num; j++) {
- if (ldb_dn_compare(valdn, parent->rm_list->dns[j]) == 0)
- break;
- }
- if (j < parent->rm_list->num) {
- continue;
- }
- new_list->entries[new_list->num].dn = valdn;
- new_list->num++;
- }
-
- /* now check that we have the right memberof for all direct parents */
- for (i = 0; i < delop->num_parents; i++) {
- found = false;
- for (j = 0; j < orig_list->num; j++) {
- if (new_list->entries[j].pmsg) {
- /* parent has already been paired, try next */
- continue;
- }
- if (new_list->entries[j].dn == NULL) {
- /* we reached the end of current entries, pair new parent */
- new_list->entries[j].dn = delop->parents[i]->dn;
- new_list->entries[j].pmsg = delop->parents[i];
- /* update also the number tracking the entries */
- new_list->num++;
- /* mark we found an entry; */
- found = true;
- break;
- }
- if (ldb_dn_compare(delop->parents[i]->dn,
- new_list->entries[j].dn) == 0) {
- /* pair parent and move to next */
- new_list->entries[j].pmsg = delop->parents[i];
- /* mark we found an entry; */
- found = true;
- break;
- }
- }
- if (!found && j == orig_list->num) {
- /* houston we have a problem here */
- /* it seem we have more parents then allotted slots which is
- * impossible because we have slots for a number of parents equal to
- * the original number minus 1 and we are only potentially removing
- * parents not adding. Return a bogus error, this must be an error
- * somewhere else or the database has been corrupted */
- return LDB_ERR_UNWILLING_TO_PERFORM;
- }
- }
+ /* at the very least we have a number of memberof elements
+ * equal to the number of objects that have this entry as
+ * direct member */
+ new_list->num = delop->num_parents;
/* attach the list to the operation */
delop->anc_ctx->new_list = new_list;
+ delop->anc_ctx->num_direct = new_list->num;
- /* do we have any member in the list at all ? */
+ /* do we have any direct parent at all ? */
if (new_list->num == 0) {
/* no entries at all, entry ended up being orphaned */
/* skip to directly set the new memberof list for this entry */
@@ -1387,6 +1233,17 @@ static int mbof_del_execute_cont(struct mbof_del_operation *delop)
return mbof_del_mod_entry(delop);
}
+ /* fill in the list if we have parents */
+ new_list->dns = talloc_zero_array(new_list,
+ struct ldb_dn *,
+ new_list->num);
+ if (!new_list->dns) {
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ for (i = 0; i < delop->num_parents; i++) {
+ new_list->dns[i] = delop->parents[i]->dn;
+ }
+
/* before proceeding we also need to fetch the ancestors (anew as some may
* have changed by preceeding operations) */
return mbof_del_ancestors(delop);
@@ -1398,7 +1255,7 @@ static int mbof_del_ancestors(struct mbof_del_operation *delop)
struct mbof_del_ctx *del_ctx;
struct mbof_ctx *ctx;
struct ldb_context *ldb;
- struct mbof_array *new_list;
+ struct mbof_dn_array *new_list;
static const char *attrs[] = {"memberof", NULL};
struct ldb_request *search;
int ret;
@@ -1410,7 +1267,7 @@ static int mbof_del_ancestors(struct mbof_del_operation *delop)
new_list = anc_ctx->new_list;
ret = ldb_build_search_req(&search, ldb, anc_ctx,
- new_list->entries[anc_ctx->cur].dn,
+ new_list->dns[anc_ctx->cur],
LDB_SCOPE_BASE, NULL, attrs, NULL,
delop, mbof_del_anc_callback,
ctx->req);
@@ -1431,7 +1288,7 @@ static int mbof_del_anc_callback(struct ldb_request *req,
struct ldb_context *ldb;
struct ldb_message *msg;
const struct ldb_message_element *el;
- struct mbof_array *new_list;
+ struct mbof_dn_array *new_list;
struct ldb_dn *valdn;
int i, j, ret;
@@ -1490,22 +1347,22 @@ static int mbof_del_anc_callback(struct ldb_request *req,
LDB_ERR_OPERATIONS_ERROR);
}
for (j = 0; j < new_list->num; j++) {
- if (ldb_dn_compare(valdn, new_list->entries[j].dn) == 0)
+ if (ldb_dn_compare(valdn, new_list->dns[j]) == 0)
break;
}
if (j < new_list->num) {
talloc_free(valdn);
continue;
}
- new_list->entries = talloc_realloc(new_list,
- new_list->entries,
- struct mbof_array_entry,
- new_list->num + 1);
- if (!new_list->entries) {
+ new_list->dns = talloc_realloc(new_list,
+ new_list->dns,
+ struct ldb_dn *,
+ new_list->num + 1);
+ if (!new_list->dns) {
return ldb_module_done(ctx->req, NULL, NULL,
LDB_ERR_OPERATIONS_ERROR);
}
- new_list->entries[new_list->num].dn = valdn;
+ new_list->dns[new_list->num] = valdn;
new_list->num++;
}
}
@@ -1516,7 +1373,7 @@ static int mbof_del_anc_callback(struct ldb_request *req,
anc_ctx->cur++;
/* check if we need to process any more */
- if (anc_ctx->cur >= new_list->num) {
+ if (anc_ctx->cur == anc_ctx->num_direct) {
/* ok, end of the story, proceed to modify the entry */
ret = mbof_del_mod_entry(delop);
} else {
@@ -1539,7 +1396,7 @@ static int mbof_del_mod_entry(struct mbof_del_operation *delop)
struct mbof_del_ctx *del_ctx;
struct mbof_ctx *ctx;
struct ldb_context *ldb;
- struct mbof_array *new_list;
+ struct mbof_dn_array *new_list;
struct ldb_request *mod_req;
struct ldb_message *msg;
struct ldb_message_element *el;
@@ -1568,9 +1425,9 @@ static int mbof_del_mod_entry(struct mbof_del_operation *delop)
return LDB_ERR_OPERATIONS_ERROR;
}
for (i = 0, j = 0; i < new_list->num; i++) {
- if (ldb_dn_compare(new_list->entries[i].dn, msg->dn) == 0)
+ if (ldb_dn_compare(new_list->dns[i], msg->dn) == 0)
continue;
- val = ldb_dn_get_linearized(new_list->entries[i].dn);
+ val = ldb_dn_get_linearized(new_list->dns[i]);
if (!val) {
return LDB_ERR_OPERATIONS_ERROR;
}
@@ -1655,14 +1512,11 @@ static int mbof_del_progeny(struct mbof_del_operation *delop)
struct mbof_del_ctx *del_ctx;
struct mbof_del_operation *nextop;
const struct ldb_message_element *el;
- struct mbof_dn_array *rm_list;
- struct mbof_dn_array *orig_list;
- struct mbof_array *new_list;
+ struct mbof_dn_array *new_list;
struct ldb_context *ldb;
struct ldb_dn *valdn;
- int i, j, ret;
+ int i, ret;
- orig_list = delop->anc_ctx->orig_list;
new_list = delop->anc_ctx->new_list;
del_ctx = delop->del_ctx;
ctx = del_ctx->ctx;
@@ -1673,35 +1527,6 @@ static int mbof_del_progeny(struct mbof_del_operation *delop)
el = ldb_msg_find_element(delop->entry, "member");
if (el) {
- /* build rm_list for children */
- rm_list = talloc_zero(delop, struct mbof_dn_array);
- if (!rm_list) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- rm_list->dns = talloc_zero_array(rm_list,
- struct ldb_dn *,
- orig_list->num);
- if (!rm_list->dns) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- for (i = 0; i < orig_list->num; i++) {
- for (j = 0; j < new_list->num; j++) {
- if (ldb_dn_compare(orig_list->dns[i],
- new_list->entries[j].dn) == 0)
- break;
- }
- /* present in both, therefoire not deleted */
- if (j < new_list->num) continue;
-
- /* this was removed, add to the rm_list */
- rm_list->dns[rm_list->num] = orig_list->dns[i];
- rm_list->num++;
- }
-
- delop->rm_list = rm_list;
-
for (i = 0; i < el->num_values; i++) {
valdn = ldb_dn_from_ldb_val(delop, ldb, &el->values[i]);
if (!valdn || !ldb_dn_validate(valdn)) {
@@ -2205,11 +2030,6 @@ static int mbof_mod_delete(struct mbof_mod_ctx *mod_ctx,
first->entry = mod_ctx->entry;
first->entry_dn = mod_ctx->entry->dn;
- ret = mbof_build_first_rm_list(ldb, first);
- if (ret != LDB_SUCCESS) {
- return ret;
- }
-
/* prepare del sets */
for (i = 0; i < del->num; i++) {
ret = mbof_append_delop(first, del->dns[i]);