On Thu, Oct 29, 2009 at 09:32:34PM +0000, Simo Sorce wrote:
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.
> +
> + subreq = sysdb_search_entry_send(state, ev, handle, dn,
> LDB_SCOPE_SUBTREE,
> + "distinguishedName=*", NULL);
Please use "(objectclass=*)" as filter to catch all entries.
I would prefer to stay with distinguishedName, because it is
auto-generated and always present.
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.
ah, I thought NULL means nothing, now I pass { NULL }
[..]
> +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.
done
> +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).
done
The rest looks good to me.
Thanks for reviewing.
bye,
Sumit
Simo.
--
Simo Sorce * Red Hat, Inc * New York