[SSSD] [PATCH] add sysdb_delete_recursive request to sysdb API

Simo Sorce ssorce at redhat.com
Thu Oct 29 21:32:34 UTC 2009


On Thu, 2009-10-29 at 19:40 +0100, Sumit Bose wrote:
> On Thu, Oct 29, 2009 at 01:39:21PM +0100, Sumit Bose wrote:
> > Hi,
> > 
> > this patch adds a recursive delete request to the sysdb API. It has
> the
> > same interface as sysdb_delete_entry, but does not delete the entry,
> but
> > its children.
> > 
> > bye,
> > Sumit
> 
> This is a new version of the patch which tries to delete the entry AND
> all its children. It searches all objects with a subtree search, sorts
> the result so that the ones with the most components come first and
> finally loops over the results and deletes them.

Comments inline.


> >From 0f087f921f5f3e26557049a25822c6183efcad91 Mon Sep 17 00:00:00
> 2001
> From: Sumit Bose <sbose at redhat.com>
> Date: Thu, 29 Oct 2009 12:57:57 +0100
> Subject: [PATCH] add sysdb_delete_recursive request to sysdb API
> 
> ---
>  server/db/sysdb.h          |    8 +++
>  server/db/sysdb_ops.c      |  145
> ++++++++++++++++++++++++++++++++++++++++++++
>  server/tests/sysdb-tests.c |  111 ++++++++++++++++++++++++++++++++-
>  3 files changed, 260 insertions(+), 4 deletions(-)
> 
> diff --git a/server/db/sysdb.h b/server/db/sysdb.h
> index 00a3378..fcb8e5a 100644
> --- a/server/db/sysdb.h
> +++ b/server/db/sysdb.h
> @@ -311,6 +311,14 @@ struct tevent_req
> *sysdb_delete_entry_send(TALLOC_CTX *mem_ctx,
>                                             bool ignore_not_found);
>  int sysdb_delete_entry_recv(struct tevent_req *req);
>  
> +
> +struct tevent_req *sysdb_delete_recursive_send(TALLOC_CTX *mem_ctx,
> +                                               struct tevent_context
> *ev,
> +                                               struct sysdb_handle
> *handle,
> +                                               struct ldb_dn *dn,
> +                                               bool
> ignore_not_found);
> +int sysdb_delete_recursive_recv(struct tevent_req *req);
> +
>  /* Search Entry */
>  struct tevent_req *sysdb_search_entry_send(TALLOC_CTX *mem_ctx,
>                                             struct tevent_context *ev,
> diff --git a/server/db/sysdb_ops.c b/server/db/sysdb_ops.c
> index acff5e5..882ef45 100644
> --- a/server/db/sysdb_ops.c
> +++ b/server/db/sysdb_ops.c
> @@ -300,6 +300,151 @@ int sysdb_delete_entry_recv(struct tevent_req
> *req)
>  }
>  
>  
> +/*
> =Remove-Subentries-From-Sysdb=============================================== */
> +
> +struct sysdb_delete_recursive_state {
> +    struct tevent_context *ev;
> +    struct sysdb_handle *handle;
> +
> +    bool ignore_not_found;
> +
> +    struct ldb_reply *ldbreply;
> +    size_t msgs_count;
> +    struct ldb_message **msgs;
> +    size_t current_item;
> +};
> +
> +static void sysdb_delete_recursive_loop(struct tevent_req *subreq);
> +
> +struct tevent_req *sysdb_delete_recursive_send(TALLOC_CTX *mem_ctx,
> +                                               struct tevent_context
> *ev,
> +                                               struct sysdb_handle
> *handle,
> +                                               struct ldb_dn *dn,
> +                                               bool ignore_not_found)
> +{
> +    struct tevent_req *req, *subreq;
> +    struct sysdb_delete_recursive_state *state;
> +    int ret;
> +
> +    req = tevent_req_create(mem_ctx, &state,
> +                            struct sysdb_delete_recursive_state);
> +    if (!req) return NULL;
> +
> +    state->ev = ev;
> +    state->handle = handle;
> +    state->ignore_not_found = ignore_not_found;
> +    state->ldbreply = NULL;
> +    state->msgs_count = 0;
> +    state->msgs = NULL;
> +    state->current_item = 0;
> +
> +    subreq = sysdb_search_entry_send(state, ev, handle, dn,
> LDB_SCOPE_SUBTREE,
> +                                     "distinguishedName=*", NULL);

Please use "(objectclass=*)" as filter to catch all entries.

Also please set attrs. Passing NULL, means you will retrieve all
attributes wasting a lot of memory unnecessarily. You are interested
only in the entries msg->dn, so you probably do not want any attribute
returned at all.

[..]

> +static int compare_ldb_dn_comp_num(const void *m1, const void *m2)
> +{
> +    struct ldb_message *msg1 = talloc_get_type(*(const void **) m1,
> +                                               struct ldb_message);
> +    struct ldb_message *msg2 = talloc_get_type(*(const void **) m2,
> +                                               struct ldb_message);
> +
> +    return ldb_dn_get_comp_num(msg2->dn) -
> ldb_dn_get_comp_num(msg1->dn);
> +}

Please move this function in sysdb.c, it's a generic function that can
be used by multiple functions and here just interrupts reading the
program flow.

> +static void sysdb_delete_recursive_loop(struct tevent_req *subreq)
[...]

I think you should split the this function into a function that receives
the results of sysdb_search_entry_recv() and then another one that sets
the loop. If necessary use the trick I used in sdap_cli_connect to do
continuation functions (see the sdap_cli_*_step functions).

The rest looks good to me.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list