[LDB] Simplify ldb_wait()

simo simo at samba.org
Sun Nov 11 17:33:30 GMT 2007


Why have you changed the ldb_wait() prototype from getting an handle to
get a full request?
It seem that the new ldab_wait() never uses req at all except as in
req->handle.

It seem to me an unnecessary ABI change for ldb.

Also it is not clear to me why instead of changing ldb_wait() you
haven't simply addedd an helper function to do what these modules where
doing all the time, it seem to me that such a change would achive
simplification without changing the API. What do we gain from the new
API (and most importantly ABI change) you propose?


Simo.


On Tue, 2007-11-06 at 14:51 +1100, Andrew Bartlett wrote:
> In trying to chase down bugs in the linked_attribute handling, I'm
> increasingly noticing how much redundent code is in the ldb_wait()
> handling for 'async' ldb.
> 
> For example, almost all modules need to implement stanza like:
> 
> static int rename_wait_all(struct ldb_handle *handle) {
> 
>        int ret;
> 
>        while (handle->state != LDB_ASYNC_DONE) {
>                ret = rename_wait(handle);
>                if (ret != LDB_SUCCESS) {
>                        return ret;
>                }
>        }
> 
>        return handle->status;
> }
> 
> static int rdn_name_wait(struct ldb_handle *handle, enum ldb_wait_type
> type)
> {
>        if (type == LDB_WAIT_ALL) {
>                return rename_wait_all(handle);
>        } else {
>                return rename_wait(handle);
>        }
> }
> 
> I've instead moved this logic to ldb_wait(), with a new operation
> pointer .wait_type for the (few) cases where we don't want this
> behaviour. 
> 
> I've also made ldb_wait take a struct ldb_request *, not a struct
> ldb_handle *, as in every case one was simply dereferenced to the other
> in the caller. 
> 
> The diffstat shows the outcome pretty well:
> 
> cldap_server/rootdse.c                     |    2 -
>  dsdb/samdb/cracknames.c                    |    2 -
>  dsdb/samdb/ldb_modules/entryUUID.c         |    4 +-
>  dsdb/samdb/ldb_modules/linked_attributes.c |   29 +-----------------
>  dsdb/samdb/ldb_modules/local_password.c    |   35 +++-------------------
>  dsdb/samdb/ldb_modules/objectclass.c       |   39 +++++-------------------
>  dsdb/samdb/ldb_modules/partition.c         |   28 +----------------
>  dsdb/samdb/ldb_modules/password_hash.c     |   37 ++++-------------------
>  dsdb/samdb/ldb_modules/repl_meta_data.c    |   46 +++++++----------------------
>  dsdb/samdb/ldb_modules/schema.c            |   29 +-----------------
>  dsdb/samdb/ldb_modules/subtree_rename.c    |   27 +----------------
>  ldap_server/ldap_backend.c                 |    2 -
>  lib/ldb/common/ldb.c                       |   26 ++++++++++++----
>  lib/ldb/include/ldb.h                      |    2 -
>  lib/ldb/include/ldb_private.h              |    3 +
>  lib/ldb/ldb_ildap/ldb_ildap.c              |    4 +-
>  lib/ldb/ldb_ldap/ldb_ldap.c                |    4 +-
>  lib/ldb/ldb_map/ldb_map.c                  |   32 +-------------------
>  lib/ldb/ldb_tdb/ldb_tdb.c                  |    2 -
>  lib/ldb/modules/asq.c                      |   29 +-----------------
>  lib/ldb/modules/paged_results.c            |    8 ++---
>  lib/ldb/modules/paged_searches.c           |   29 +-----------------
>  lib/ldb/modules/rdn_name.c                 |   29 +-----------------
>  lib/ldb/modules/sort.c                     |    6 +--
>  lib/ldb/nssldb/ldb-nss.c                   |    2 -
>  lib/ldb/tools/ldbsearch.c                  |    2 -
>  scripting/ejs/smbcalls_ldb.c               |    2 -
>  torture/ldap/schema.c                      |    2 -
>  28 files changed, 99 insertions(+), 363 deletions(-)
> 
> Thoughts?
> 
> Andrew Bartlett
> 
-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Senior Software Engineer at Red Hat Inc. <ssorce at redhat.com>



More information about the samba-technical mailing list