Modernize some LDAP sources

Bring ldap_misc.c up to date with current practices and make limited
changes to other files.  Of note:

* krb5_decode_krbsecretkey was freeing its bvalues argument; make that
  the caller's responsibility.
* Make is_principal_in_realm and has_modify_increment return
  krb5_boolean, reversing the sense of their results.
* Remove broken code path in decode_tl_data when an integer value has
  a length other than 2 (which should never happen).
* Simplify krb5_ldap_readpassword and make it take filename/name
  parameters instead of an LDAP context.
* Make krb5_ldap_bind (renamed to authenticate) responsible for
  setting a useful error message, so that its caller doesn't assume
  knowledge of the bind parameters.
* Make krb5_ldap_initialize (renamed to initialize_server) responsible
  for updating the handle list, and remove the otherwise unused
  krb5_update_ldap_handle.
* Remove remaining skeletal certificate support, including the unused
  has_sasl_external_mech function.
* Remove unused krb5_get_containerdn and KDB_TL_CONTAINERDN.
* Remove kdb_xdr.h; all of its prototypes were for functions that
  don't exist in the module or were duplicated in other headers.
* Remove krb5_ldap_get_strings and use ldap_get_values directly at
  its call sites; there was no need to copy the result.

15 files changed, 1003 insertions, 1611 deletions

diff --git a/src/plugins/kdb/ldap/libkdb_ldap/deps b/src/plugins/kdb/ldap/libkdb_ldap/deps
index 2c1d63c..10c287d 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/deps
+++ b/src/plugins/kdb/ldap/libkdb_ldap/deps
@@ -224,7 +224,7 @@ kdb_xdr.so kdb_xdr.po $(OUTPRE)kdb_xdr.$(OBJEXT): $(BUILDTOP)/include/autoconf.h
   $(top_srcdir)/include/kdb.h $(top_srcdir)/include/krb5.h \
   $(top_srcdir)/include/krb5/authdata_plugin.h $(top_srcdir)/include/krb5/plugin.h \
   $(top_srcdir)/include/port-sockets.h $(top_srcdir)/include/socket-utils.h \
-  kdb_xdr.c kdb_xdr.h
+  kdb_xdr.c
 ldap_err.so ldap_err.po $(OUTPRE)ldap_err.$(OBJEXT): \
   $(BUILDTOP)/include/krb5/krb5.h $(COM_ERR_DEPS) $(top_srcdir)/include/krb5.h \
   ldap_err.c ldap_err.h
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c
index 8284f81..76243f9 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c
@@ -120,120 +120,66 @@ cleanup:
 }
 
 
-/*
- * Interrogate the root DSE (zero length DN) for an attribute
- * value assertion.
- */
-static int
-has_rootdse_ava(krb5_context context, char *ldap_server, char *attribute,
-                char *value)
+/* Interrogate the root DSE (zero length DN) for an attribute value assertion.
+ * Return true if it is present, false if it is absent or we can't tell. */
+static krb5_boolean
+has_rootdse_ava(krb5_context context, const char *server_name,
+                const char *attribute, const char *value)
 {
-    int               i=0, flag=0, ret=0, retval=0;
-    char              *attrs[2], **values=NULL;
-    LDAP              *ld=NULL;
-    LDAPMessage       *msg=NULL, *res=NULL;
-    struct berval     cred;
+    krb5_boolean result = FALSE;
+    char *attrs[2], **values = NULL;
+    int i, st;
+    LDAP *ld = NULL;
+    LDAPMessage *msg, *res = NULL;
+    struct berval cred;
 
     attrs[0] = attribute;
     attrs[1] = NULL;
 
-    retval = ldap_initialize(&ld, ldap_server);
-    if (retval != LDAP_SUCCESS) {
-        ret = 2; /* Don't know */
+    st = ldap_initialize(&ld, server_name);
+    if (st != LDAP_SUCCESS)
         goto cleanup;
-    }
 
+    /* Bind anonymously. */
     cred.bv_val = "";
     cred.bv_len = 0;
-
-    /* Anonymous bind */
-    retval = ldap_sasl_bind_s(ld, "", NULL, &cred, NULL, NULL, NULL);
-    if (retval != LDAP_SUCCESS) {
-        ret = 2; /* Don't know */
+    st = ldap_sasl_bind_s(ld, "", NULL, &cred, NULL, NULL, NULL);
+    if (st != LDAP_SUCCESS)
         goto cleanup;
-    }
 
-    retval = ldap_search_ext_s(ld, "", LDAP_SCOPE_BASE, NULL, attrs, 0, NULL, NULL, NULL, 0, &res);
-    if (retval != LDAP_SUCCESS) {
-        ret = 2; /* Don't know */
+    st = ldap_search_ext_s(ld, "", LDAP_SCOPE_BASE, NULL, attrs, 0, NULL,
+                           NULL, NULL, 0, &res);
+    if (st != LDAP_SUCCESS)
         goto cleanup;
-    }
 
     msg = ldap_first_message(ld, res);
-    if (msg == NULL) {
-        ret = 2; /* Don't know */
+    if (msg == NULL)
         goto cleanup;
-    }
 
     values = ldap_get_values(ld, msg, attribute);
-    if (values == NULL) {
-        ret = 1; /* Not supported */
+    if (values == NULL)
         goto cleanup;
-    }
 
     for (i = 0; values[i] != NULL; i++) {
         if (strcmp(values[i], value) == 0) {
-            flag = 1;
-            break;
+            result = TRUE;
+            goto cleanup;
         }
     }
 
-    if (flag != 1) {
-        ret = 1; /* Not supported */
-        goto cleanup;
-    }
-
 cleanup:
+    ldap_value_free(values);
+    ldap_msgfree(res);
+    ldap_unbind_ext_s(ld, NULL, NULL);
 
-    if (values != NULL)
-        ldap_value_free(values);
-
-    if (res != NULL)
-        ldap_msgfree(res);
-
-    if (ld != NULL)
-        ldap_unbind_ext_s(ld, NULL, NULL);
-
-    return ret;
-}
-
-#define ERR_MSG1 _("Unable to check if SASL EXTERNAL mechanism is supported by LDAP server. Proceeding anyway ...")
-#define ERR_MSG2 _("SASL EXTERNAL mechanism not supported by LDAP server. Can't perform certificate-based bind.")
-
-/* Function to check if a LDAP server supports the SASL external mechanism
- *Return values:
- *   0 => supports
- *   1 => does not support
- *   2 => don't know
- */
-int
-has_sasl_external_mech(krb5_context context, char *ldap_server)
-{
-    int ret;
-
-    ret = has_rootdse_ava(context, ldap_server,
-                          "supportedSASLMechanisms", "EXTERNAL");
-    switch (ret) {
-    case 1: /* not supported */
-        k5_setmsg(context, 1, "%s", ERR_MSG2);
-        break;
-    case 2: /* don't know */
-        k5_setmsg(context, 1, "%s", ERR_MSG1);
-        break;
-    default:
-        break;
-    }
-
-    return ret;
+    return result;
 }
 
-int
-has_modify_increment(context, ldap_server)
-    krb5_context     context;
-    char             *ldap_server;
+krb5_boolean
+has_modify_increment(krb5_context context, const char *server_name)
 {
-    return has_rootdse_ava(context, ldap_server,
-                           "supportedFeatures", "1.3.6.1.1.14");
+    return has_rootdse_ava(context, server_name, "supportedFeatures",
+                           "1.3.6.1.1.14");
 }
 
 void *
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h
index aa8e7ce..319c701 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h
@@ -147,7 +147,7 @@ extern void prepend_err_str (krb5_context ctx, const char *s, krb5_error_code er
 #define KDB_TL_USERDN             0x03
 #define KDB_TL_KEYINFO            0x04
 #define KDB_TL_MASK               0x05
-#define KDB_TL_CONTAINERDN        0x06
+/* 0x06 was KDB_TL_CONTAINERDN but is no longer used */
 #define KDB_TL_LINKDN             0x07
 
 
@@ -159,12 +159,6 @@ extern void prepend_err_str (krb5_context ctx, const char *s, krb5_error_code er
 #define HNDL_LOCK(lcontext) k5_mutex_lock(&lcontext->hndl_lock)
 #define HNDL_UNLOCK(lcontext) k5_mutex_unlock(&lcontext->hndl_lock)
 
-/* To be used later */
-typedef struct _krb5_ldap_certificates{
-    char *certificate;
-    int  certtype;
-}krb5_ldap_certificates;
-
 /* ldap server info structure */
 
 typedef enum _server_type {PRIMARY, SECONDARY} krb5_ldap_server_type;
@@ -206,7 +200,6 @@ typedef struct _krb5_ldap_context {
     char                          *bind_pwd;
     char                          *service_password_file;
     char                          *root_certificate_file;
-    krb5_ldap_certificates        **certificates;
     krb5_ui_4                     cert_count; /* certificate count */
     k5_mutex_t                    hndl_lock;
     char                          *container_dn;
@@ -270,13 +263,10 @@ krb5_ldap_free_ldap_context(krb5_ldap_context *);
 krb5_error_code
 krb5_ldap_read_startup_information(krb5_context );
 
-int
-has_sasl_external_mech(krb5_context, char *);
-
-int
-has_modify_increment(krb5_context, char *);
+krb5_boolean
+has_modify_increment(krb5_context, const char *);
 
-krb5_error_code
+void
 krb5_ldap_free_server_context_params(krb5_ldap_context *ldap_context);
 
 krb5_error_code
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c
index 3ebfb87..78ea428 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c
@@ -37,45 +37,40 @@
 #include "ldap_service_stash.h"
 #include <kdb5.h>
 
+/* Ensure that we have the parameters we need to authenticate to the LDAP
+ * server.  Read the password if necessary. */
 static krb5_error_code
-krb5_validate_ldap_context(krb5_context context,
-                           krb5_ldap_context *ldap_context)
+validate_context(krb5_context context, krb5_ldap_context *ctx)
 {
-    krb5_error_code             st=0;
-    unsigned char               *password=NULL;
+    krb5_error_code ret;
 
-    if (ldap_context->bind_dn == NULL) {
-        st = EINVAL;
-        k5_setmsg(context, st, _("LDAP bind dn value missing "));
-        goto err_out;
+    if (ctx->bind_dn == NULL) {
+        k5_setmsg(context, EINVAL, _("LDAP bind dn value missing"));
+        return EINVAL;
     }
 
-    if (ldap_context->bind_pwd == NULL && ldap_context->service_password_file == NULL) {
-        st = EINVAL;
-        k5_setmsg(context, st, _("LDAP bind password value missing "));
-        goto err_out;
+    if (ctx->bind_pwd == NULL && ctx->service_password_file == NULL) {
+        k5_setmsg(context, EINVAL, _("LDAP bind password value missing"));
+        return EINVAL;
     }
 
-    if (ldap_context->bind_pwd == NULL &&
-        ldap_context->service_password_file != NULL) {
-        if ((st=krb5_ldap_readpassword(context, ldap_context, &password)) != 0) {
+    if (ctx->bind_pwd == NULL && ctx->service_password_file != NULL) {
+        ret = krb5_ldap_readpassword(context, ctx->service_password_file,
+                                     ctx->bind_dn, &ctx->bind_pwd);
+        if (ret) {
             prepend_err_str(context, _("Error reading password from stash: "),
-                            st, st);
-            goto err_out;
+                            ret, ret);
+            return ret;
         }
-
-        ldap_context->bind_pwd = (char *)password;
     }
 
-    /* NULL password not allowed */
-    if (ldap_context->bind_pwd != NULL && strlen(ldap_context->bind_pwd) == 0) {
-        st = EINVAL;
-        k5_setmsg(context, st, _("Service password length is zero"));
-        goto err_out;
+    /* An empty password is not allowed. */
+    if (*ctx->bind_pwd == '\0') {
+        k5_setmsg(context, EINVAL, _("Service password length is zero"));
+        return EINVAL;
     }
 
-err_out:
-    return st;
+    return 0;
 }
 
 /*
@@ -83,59 +78,60 @@ err_out:
  */
 
 static krb5_error_code
-krb5_ldap_bind(krb5_ldap_context *ldap_context,
-               krb5_ldap_server_handle *ldap_server_handle)
+authenticate(krb5_ldap_context *ctx, krb5_ldap_server_handle *server)
 {
-    struct berval               bv={0, NULL};
-
-    bv.bv_val = ldap_context->bind_pwd;
-    bv.bv_len = strlen(ldap_context->bind_pwd);
-    return ldap_sasl_bind_s(ldap_server_handle->ldap_handle,
-                            ldap_context->bind_dn, NULL, &bv, NULL,
-                            NULL, NULL);
+    int st;
+    struct berval bv;
+
+    bv.bv_val = ctx->bind_pwd;
+    bv.bv_len = strlen(ctx->bind_pwd);
+    st = ldap_sasl_bind_s(server->ldap_handle, ctx->bind_dn, NULL, &bv, NULL,
+                          NULL, NULL);
+    if (st != LDAP_SUCCESS) {
+        k5_setmsg(ctx->kcontext, KRB5_KDB_ACCESS_ERROR,
+                  _("Cannot bind to LDAP server '%s' as '%s': %s"),
+                  server->server_info->server_name, ctx->bind_dn,
+                  ldap_err2string(st));
+        return KRB5_KDB_ACCESS_ERROR;
+    }
+    return 0;
 }
 
 static krb5_error_code
-krb5_ldap_initialize(krb5_ldap_context *ldap_context,
-                     krb5_ldap_server_info *server_info)
+initialize_server(krb5_ldap_context *ldap_context, krb5_ldap_server_info *info)
 {
-    krb5_error_code             st=0;
-    krb5_ldap_server_handle     *ldap_server_handle=NULL;
-
-
-    ldap_server_handle = calloc(1, sizeof(krb5_ldap_server_handle));
-    if (ldap_server_handle == NULL) {
-        st = ENOMEM;
-        goto err_out;
-    }
-
-    /* ldap init */
-    if ((st = ldap_initialize(&ldap_server_handle->ldap_handle, server_info->server_name)) != 0) {
+    krb5_ldap_server_handle *server;
+    krb5_error_code ret;
+    int st;
+
+    server = calloc(1, sizeof(krb5_ldap_server_handle));
+    if (server == NULL)
+        return ENOMEM;
+    server->server_info = info;
+
+    st = ldap_initialize(&server->ldap_handle, info->server_name);
+    if (st) {
+        free(server);
         k5_setmsg(ldap_context->kcontext, KRB5_KDB_ACCESS_ERROR,
                   _("Cannot create LDAP handle for '%s': %s"),
-                  server_info->server_name, ldap_err2string(st));
-        st = KRB5_KDB_ACCESS_ERROR;
-        goto err_out;
+                  info->server_name, ldap_err2string(st));
+        return KRB5_KDB_ACCESS_ERROR;
     }
 
-    if ((st=krb5_ldap_bind(ldap_context, ldap_server_handle)) == 0) {
-        ldap_server_handle->server_info_update_pending = FALSE;
-        server_info->server_status = ON;
-        krb5_update_ldap_handle(ldap_server_handle, server_info);
-    } else {
-        k5_setmsg(ldap_context->kcontext, KRB5_KDB_ACCESS_ERROR,
-                  _("Cannot bind to LDAP server '%s' as '%s': %s"),
-                  server_info->server_name, ldap_context->bind_dn,
-                  ldap_err2string(st));
-        st = KRB5_KDB_ACCESS_ERROR;
-        server_info->server_status = OFF;
-        time(&server_info->downtime);
-        /* ldap_unbind_s(ldap_server_handle->ldap_handle); */
-        free(ldap_server_handle);
+    ret = authenticate(ldap_context, server);
+    if (ret) {
+        info->server_status = OFF;
+        time(&info->downtime);
+        free(server);
+        return ret;
     }
 
-err_out:
-    return st;
+    server->server_info_update_pending = FALSE;
+    server->next = info->ldap_server_handles;
+    info->ldap_server_handles = server;
+    info->num_conns++;
+    info->server_status = ON;
+    return 0;
 }
 
 /*
@@ -143,17 +139,20 @@ err_out:
  */
 
 krb5_error_code
-krb5_ldap_db_init(krb5_context context, krb5_ldap_context *ldap_context)
+krb5_ldap_db_init(krb5_context context, krb5_ldap_context *ctx)
 {
-    krb5_error_code             st=0;
-    int                         cnt=0, version=LDAP_VERSION3;
-    struct timeval              local_timelimit = {10,0};
+    krb5_error_code ret;
+    int i, version = LDAP_VERSION3;
+    unsigned int conns;
+    krb5_ldap_server_info *info;
+    struct timeval local_timelimit = { 10, 0 };
 
-    if ((st=krb5_validate_ldap_context(context, ldap_context)) != 0)
-        return st;
+    ret = validate_context(context, ctx);
+    if (ret)
+        return ret;
 
 #ifdef LDAP_OPT_DEBUG_LEVEL
-    ldap_set_option(NULL, LDAP_OPT_DEBUG_LEVEL, &ldap_context->ldap_debug);
+    ldap_set_option(NULL, LDAP_OPT_DEBUG_LEVEL, &ctx->ldap_debug);
 #endif
     ldap_set_option(NULL, LDAP_OPT_PROTOCOL_VERSION, &version);
 #ifdef LDAP_OPT_NETWORK_TIMEOUT
@@ -162,37 +161,33 @@ krb5_ldap_db_init(krb5_context context, krb5_ldap_context *ldap_context)
     ldap_set_option(NULL, LDAP_X_OPT_CONNECT_TIMEOUT, &local_timelimit);
 #endif
 
-    HNDL_LOCK(ldap_context);
-    while (ldap_context->server_info_list[cnt] != NULL) {
-        krb5_ldap_server_info *server_info=NULL;
-
-        server_info = ldap_context->server_info_list[cnt];
-
-        if (server_info->server_status == NOTSET) {
-            unsigned int conns=0;
-
+    HNDL_LOCK(ctx);
+    for (i = 0; ctx->server_info_list[i] != NULL; i++) {
+        info = ctx->server_info_list[i];
+        if (info->server_status == NOTSET) {
             krb5_clear_error_message(context);
 
 #ifdef LDAP_MOD_INCREMENT
-            server_info->modify_increment =
-                (has_modify_increment(context, server_info->server_name) == 0);
+            info->modify_increment = has_modify_increment(context,
+                                                          info->server_name);
 #else
-            server_info->modify_increment = 0;
-#endif /* LDAP_MOD_INCREMENT */
+            info->modify_increment = 0;
+#endif
 
-            for (conns=0; conns < ldap_context->max_server_conns; ++conns) {
-                if ((st=krb5_ldap_initialize(ldap_context, server_info)) != 0)
+            for (conns = 0; conns < ctx->max_server_conns; conns++) {
+                ret = initialize_server(ctx, info);
+                if (ret)
                     break;
-            } /* for (conn= ... */
+            }
 
-            if (server_info->server_status == ON)
-                break;  /* server init successful, so break */
+            /* If we opened a connection, don't try any more servers. */
+            if (info->server_status == ON)
+                break;
         }
-        ++cnt;
     }
-    HNDL_UNLOCK(ldap_context);
+    HNDL_UNLOCK(ctx);
 
-    return st;
+    return ret;
 }
 
 
@@ -211,7 +206,7 @@ krb5_ldap_db_single_init(krb5_ldap_context *ldap_context)
         server_info = ldap_context->server_info_list[cnt];
         if ((server_info->server_status == NOTSET || server_info->server_status == ON)) {
             if (server_info->num_conns < ldap_context->max_server_conns-1) {
-                st = krb5_ldap_initialize(ldap_context, server_info);
+                st = initialize_server(ldap_context, server_info);
                 if (st == LDAP_SUCCESS)
                     goto cleanup;
             }
@@ -224,7 +219,7 @@ krb5_ldap_db_single_init(krb5_ldap_context *ldap_context)
     cnt = 0;
     while (ldap_context->server_info_list[cnt] != NULL) {
         server_info = ldap_context->server_info_list[cnt];
-        st = krb5_ldap_initialize(ldap_context, server_info);
+        st = initialize_server(ldap_context, server_info);
         if (st == LDAP_SUCCESS)
             goto cleanup;
         ++cnt;
@@ -237,12 +232,15 @@ krb5_error_code
 krb5_ldap_rebind(krb5_ldap_context *ldap_context,
                  krb5_ldap_server_handle **ldap_server_handle)
 {
-    krb5_ldap_server_handle     *handle = *ldap_server_handle;
+    krb5_ldap_server_handle *handle = *ldap_server_handle;
 
     ldap_unbind_ext_s(handle->ldap_handle, NULL, NULL);
-    if ((ldap_initialize(&handle->ldap_handle, handle->server_info->server_name) != LDAP_SUCCESS)
-        || (krb5_ldap_bind(ldap_context, handle) != LDAP_SUCCESS))
-        return krb5_ldap_request_next_handle_from_pool(ldap_context, ldap_server_handle);
+    if (ldap_initialize(&handle->ldap_handle,
+                        handle->server_info->server_name) != LDAP_SUCCESS ||
+        authenticate(ldap_context, handle) != 0) {
+        return krb5_ldap_request_next_handle_from_pool(ldap_context,
+                                                       ldap_server_handle);
+    }
     return LDAP_SUCCESS;
 }
 
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c b/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
index 4cac857..ae51905 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
@@ -28,7 +28,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <errno.h>
-#include "kdb_xdr.h"
+#include <kdb.h>
 
 #define safe_realloc(p,n) ((p)?(realloc(p,n)):(malloc(n)))
 
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.h b/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.h
deleted file mode 100644
index 7e45710..0000000
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
-#ifndef _KDB2_XDR_H
-#define _KDB2_XDR_H
-
-#include "kdb.h"
-
-krb5_error_code
-krb5_encode_princ_dbkey(krb5_context context,
-                        krb5_data *key,
-                        krb5_const_principal principal);
-
-krb5_error_code
-krb5_decode_princ_contents(krb5_context context,
-                           krb5_data *content,
-                           krb5_db_entry *entry);
-
-void
-krb5_dbe_free_contents(krb5_context context,
-                       krb5_db_entry *entry);
-
-krb5_error_code
-krb5_encode_princ_contents(krb5_context context,
-                           krb5_data *content,
-                           krb5_db_entry *entry);
-
-
-void
-krb5_free_princ_dbkey(krb5_context context,
-                      krb5_data *key);
-
-void
-krb5_free_princ_contents(krb5_context context,
-                         krb5_data *contents);
-
-#endif
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
index 616a7e2..77d8f81 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
@@ -163,27 +163,6 @@ krb5_put_ldap_handle(krb5_ldap_server_handle *ldap_server_handle)
 }
 
 /*
- * Add a new ldap server handle structure to the server info structure.
- * This function name can be changed to krb5_insert_ldap_handle.
- * Do not lock the mutex here. The caller should lock it
- */
-
-krb5_error_code
-krb5_update_ldap_handle(krb5_ldap_server_handle *ldap_server_handle,
-                        krb5_ldap_server_info *server_info)
-{
-
-    if (ldap_server_handle == NULL || server_info == NULL)
-        return 0;
-
-    ldap_server_handle->next = server_info->ldap_server_handles;
-    server_info->ldap_server_handles = ldap_server_handle;
-    server_info->num_conns++;
-    ldap_server_handle->server_info = server_info;
-    return 0;
-}
-
-/*
  * Free up all the ldap server handles of the server info.
  * This function is called when the ldap server returns LDAP_SERVER_DOWN.
  */
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.h b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.h
index 73ea3d8..f56dead3 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.h
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.h
@@ -32,9 +32,6 @@
 #define _LDAP_HANDLE_H_
 
 krb5_error_code
-krb5_update_ldap_handle(krb5_ldap_server_handle *, krb5_ldap_server_info *);
-
-krb5_error_code
 krb5_ldap_request_handle_from_pool(krb5_ldap_context *, krb5_ldap_server_handle **);
 
 krb5_error_code
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_krbcontainer.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_krbcontainer.c
index 4ef7f2e..7668ee5 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_krbcontainer.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_krbcontainer.c
@@ -40,7 +40,6 @@ krb5_error_code
 krb5_ldap_read_krbcontainer_dn(krb5_context context, char **container_dn)
 {
     krb5_error_code                 st=0;
-    LDAP                            *ld=NULL;
     char                            *dn=NULL;
     kdb5_dal_handle                 *dal_handle=NULL;
     krb5_ldap_context               *ldap_context=NULL;
@@ -48,7 +47,6 @@ krb5_ldap_read_krbcontainer_dn(krb5_context context, char **container_dn)
 
     *container_dn = NULL;
     SETUP_CONTEXT();
-    GET_HANDLE();
 
     /* read kerberos containter location from [dbmodules] section of krb5.conf file */
     if (ldap_context->conf_section) {
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c
index 779121f..6c1ac5d 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c
@@ -42,130 +42,96 @@
 #include <ctype.h>
 
 #ifdef NEED_STRPTIME_PROTO
-extern char *strptime (const char *, const char *, struct tm *);
+extern char *strptime(const char *, const char *, struct tm *);
 #endif
 
-static krb5_error_code
-remove_overlapping_subtrees(char **listin, char **listop, int *subtcount,
-                            int sscope);
-
-/* Linux (GNU Libc) provides a length-limited variant of strdup.
-   But all the world's not Linux.  */
-#undef strndup
-#define strndup my_strndup
+static void remove_overlapping_subtrees(char **listin, int *subtcount,
+                                        int sscope);
 
-static char *
-my_strndup(const char *input, size_t limit)
+/* Set an extended error message about being unable to read name. */
+static krb5_error_code
+attr_read_error(krb5_context ctx, krb5_error_code code, const char *name)
 {
-    size_t len = strlen(input);
-    char *result;
-    if (len > limit) {
-        result = malloc(1 + limit);
-        if (result != NULL) {
-            memcpy(result, input, limit);
-            result[limit] = 0;
-        }
-        return result;
-    } else
-        return strdup(input);
+    k5_setmsg(ctx, code, _("Error reading '%s' attribute: %s"), name,
+              error_message(code));
+    return code;
 }
 
-/* Get integer or string values from the config section, falling back
-   to the default section, then to hard-coded values.  */
-static errcode_t
+/* Get integer or string values from the config section, falling back to the
+ * default section, then to hard-coded values. */
+static krb5_error_code
 prof_get_integer_def(krb5_context ctx, const char *conf_section,
                      const char *name, int dfl, krb5_ui_4 *out)
 {
-    errcode_t err;
-    int out_temp = 0;
-
-    err = profile_get_integer (ctx->profile,
-                               KDB_MODULE_SECTION, conf_section, name,
-                               0, &out_temp);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
-    if (out_temp != 0) {
-        *out = out_temp;
+    krb5_error_code ret;
+    int val;
+
+    ret = profile_get_integer(ctx->profile, KDB_MODULE_SECTION, conf_section,
+                              name, 0, &val);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
+    if (val != 0) {
+        *out = val;
         return 0;
     }
-    err = profile_get_integer (ctx->profile,
-                               KDB_MODULE_DEF_SECTION, name, 0,
-                               dfl, &out_temp);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
-    *out = out_temp;
+    ret = profile_get_integer(ctx->profile, KDB_MODULE_DEF_SECTION, name, NULL,
+                              dfl, &val);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
+    *out = val;
     return 0;
 }
 
-/* Get integer or string values from the config section, falling back
-   to the default section, then to hard-coded values.  */
-static errcode_t
+/* Get integer or string values from the config section, falling back to the
+ * default section, then to hard-coded values. */
+static krb5_error_code
 prof_get_boolean_def(krb5_context ctx, const char *conf_section,
                      const char *name, krb5_boolean dfl, krb5_boolean *out)
 {
-    errcode_t err;
-    int out_temp = 0;
-
-    err = profile_get_boolean(ctx->profile, KDB_MODULE_SECTION, conf_section,
-                              name, -1, &out_temp);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
-    if (out_temp != -1) {
-        *out = out_temp;
+    krb5_error_code ret;
+    int val = 0;
+
+    ret = profile_get_boolean(ctx->profile, KDB_MODULE_SECTION, conf_section,
+                              name, -1, &val);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
+    if (val != -1) {
+        *out = val;
         return 0;
     }
-    err = profile_get_boolean(ctx->profile, KDB_MODULE_DEF_SECTION, name, 0,
-                              dfl, &out_temp);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
-    *out = out_temp;
+    ret = profile_get_boolean(ctx->profile, KDB_MODULE_DEF_SECTION, name, NULL,
+                              dfl, &val);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
+    *out = val;
     return 0;
 }
 
-/* We don't have non-null defaults in any of our calls, so don't
-   bother with the extra argument.  */
-static errcode_t
+/* We don't have non-null defaults in any of our calls, so don't bother with
+ * the extra argument. */
+static krb5_error_code
 prof_get_string_def(krb5_context ctx, const char *conf_section,
                     const char *name, char **out)
 {
-    errcode_t err;
-
-    err = profile_get_string (ctx->profile,
-                              KDB_MODULE_SECTION, conf_section, name,
-                              0, out);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
-    if (*out != 0)
+    krb5_error_code ret;
+
+    ret = profile_get_string(ctx->profile, KDB_MODULE_SECTION, conf_section,
+                             name, NULL, out);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
+    if (*out != NULL)
         return 0;
-    err = profile_get_string (ctx->profile,
-                              KDB_MODULE_DEF_SECTION, name, 0,
-                              0, out);
-    if (err) {
-        k5_setmsg(ctx, err, _("Error reading '%s' attribute: %s"), name,
-                  error_message(err));
-        return err;
-    }
+    ret = profile_get_string(ctx->profile, KDB_MODULE_DEF_SECTION, name, NULL,
+                             NULL, out);
+    if (ret)
+        return attr_read_error(ctx, ret, name);
     return 0;
 }
 
 static krb5_error_code
 get_db_opt(const char *input, char **opt_out, char **val_out)
 {
+    krb5_error_code ret;
     const char *pos;
     char *opt, *val = NULL;
     size_t len;
@@ -181,7 +147,9 @@ get_db_opt(const char *input, char **opt_out, char **val_out)
         /* Ignore trailing spaces. */
         while (len > 0 && isspace((unsigned char)input[len - 1]))
             len--;
-        opt = strndup(input, len);
+        opt = k5memdup0(input, len, &ret);
+        if (opt == NULL)
+            return ret;
 
         pos++;                  /* Move past '='. */
         while (isspace(*pos))   /* Ignore leading spaces. */
@@ -202,17 +170,17 @@ get_db_opt(const char *input, char **opt_out, char **val_out)
 static krb5_error_code
 add_server_entry(krb5_context context, const char *name)
 {
-    krb5_ldap_context *lctx = context->dal_handle->db_context;
+    krb5_ldap_context *ctx = context->dal_handle->db_context;
     krb5_ldap_server_info **sp, **list, *server;
     size_t count = 0;
 
     /* Allocate list space for the new entry and null terminator. */
-    for (sp = lctx->server_info_list; sp != NULL && *sp != NULL; sp++)
+    for (sp = ctx->server_info_list; sp != NULL && *sp != NULL; sp++)
         count++;
-    list = realloc(lctx->server_info_list, (count + 2) * sizeof(*list));
+    list = realloc(ctx->server_info_list, (count + 2) * sizeof(*list));
     if (list == NULL)
         return ENOMEM;
-    lctx->server_info_list = list;
+    ctx->server_info_list = list;
 
     server = calloc(1, sizeof(krb5_ldap_server_info));
     if (server == NULL)
@@ -232,59 +200,58 @@ krb5_error_code
 krb5_ldap_parse_db_params(krb5_context context, char **db_args)
 {
     char *opt = NULL, *val = NULL;
-    krb5_error_code status = 0;
-    krb5_ldap_context *lctx = context->dal_handle->db_context;
+    krb5_error_code ret = 0;
+    krb5_ldap_context *ctx = context->dal_handle->db_context;
 
     if (db_args == NULL)
         return 0;
     for (; *db_args != NULL; db_args++) {
-        status = get_db_opt(*db_args, &opt, &val);
-        if (status)
+        ret = get_db_opt(*db_args, &opt, &val);
+        if (ret)
             goto cleanup;
 
         /* Check for options which don't require values. */
         if (!strcmp(opt, "temporary")) {
             /* "temporary" is passed by kdb5_util load without -update,
              * which we don't support. */
-            status = EINVAL;
-            k5_setmsg(context, status,
-                      _("KDB module requires -update argument"));
+            ret = EINVAL;
+            k5_setmsg(context, ret, _("KDB module requires -update argument"));
             goto cleanup;
         }
 
         if (val == NULL) {
-            status = EINVAL;
-            k5_setmsg(context, status, _("'%s' value missing"), opt);
+            ret = EINVAL;
+            k5_setmsg(context, ret, _("'%s' value missing"), opt);
             goto cleanup;
         }
 
         /* Check for options which do require arguments. */
         if (!strcmp(opt, "binddn")) {
-            free(lctx->bind_dn);
-            lctx->bind_dn = strdup(val);
-            if (lctx->bind_dn == NULL) {
-                status = ENOMEM;
+            free(ctx->bind_dn);
+            ctx->bind_dn = strdup(val);
+            if (ctx->bind_dn == NULL) {
+                ret = ENOMEM;
                 goto cleanup;
             }
         } else if (!strcmp(opt, "nconns")) {
-            lctx->max_server_conns = atoi(val) ? atoi(val) :
+            ctx->max_server_conns = atoi(val) ? atoi(val) :
                 DEFAULT_CONNS_PER_SERVER;
         } else if (!strcmp(opt, "bindpwd")) {
-            free(lctx->bind_pwd);
-            lctx->bind_pwd = strdup(val);
-            if (lctx->bind_pwd == NULL) {
-                status = ENOMEM;
+            free(ctx->bind_pwd);
+            ctx->bind_pwd = strdup(val);
+            if (ctx->bind_pwd == NULL) {
+                ret = ENOMEM;
                 goto cleanup;
             }
         } else if (!strcmp(opt, "host")) {
-            status = add_server_entry(context, val);
-            if (status)
+            ret = add_server_entry(context, val);
+            if (ret)
                 goto cleanup;
         } else if (!strcmp(opt, "debug")) {
-            lctx->ldap_debug = atoi(val);
+            ctx->ldap_debug = atoi(val);
         } else {
-            status = EINVAL;
-            k5_setmsg(context, status, _("unknown option '%s'"), opt);
+            ret = EINVAL;
+            k5_setmsg(context, ret, _("unknown option '%s'"), opt);
             goto cleanup;
         }
 
@@ -296,7 +263,14 @@ krb5_ldap_parse_db_params(krb5_context context, char **db_args)
 cleanup:
     free(opt);
     free(val);
-    return status;
+    return ret;
+}
+
+/* Pick kdc_var or kadmind_var depending on the server type. */
+static inline const char *
+choose_var(int srv_type, const char *kdc_var, const char *kadmind_var)
+{
+    return (srv_type == KRB5_KDB_SRV_TYPE_KDC) ? kdc_var : kadmind_var;
 }
 
 /*
@@ -308,264 +282,151 @@ krb5_error_code
 krb5_ldap_read_server_params(krb5_context context, char *conf_section,
                              int srv_type)
 {
-    char                        *tempval=NULL, *save_ptr=NULL, *item=NULL;
-    const char                  *delims="\t\n\f\v\r ,";
-    krb5_error_code             st=0;
-    kdb5_dal_handle             *dal_handle=NULL;
-    krb5_ldap_context           *ldap_context=NULL;
-
-    dal_handle = context->dal_handle;
-    ldap_context = (krb5_ldap_context *) dal_handle->db_context;
+    char *servers, *save_ptr, *item;
+    const char *delims = "\t\n\f\v\r ,", *name;
+    krb5_error_code ret = 0;
+    kdb5_dal_handle *dal_handle = context->dal_handle;
+    krb5_ldap_context *ldap_context = dal_handle->db_context;
 
     /* copy the conf_section into ldap_context for later use */
-    if (conf_section) {
-        ldap_context->conf_section = strdup (conf_section);
-        if (ldap_context->conf_section == NULL) {
-            st = ENOMEM;
-            goto cleanup;
-        }
+    if (conf_section != NULL) {
+        ldap_context->conf_section = strdup(conf_section);
+        if (ldap_context->conf_section == NULL)
+            return ENOMEM;
     }
 
-    /* initialize the mutexs and condition variable */
-    /* this portion logically doesn't fit here should be moved appropriately */
+    /* This mutex is used in the LDAP connection pool. */
+    if (k5_mutex_init(&(ldap_context->hndl_lock)) != 0)
+        return KRB5_KDB_SERVER_INTERNAL_ERR;
 
-    /* this mutex is used in ldap reconnection pool */
-    if (k5_mutex_init(&(ldap_context->hndl_lock)) != 0) {
-        st = KRB5_KDB_SERVER_INTERNAL_ERR;
-#if 0
-        st = -1;
-        krb5_ldap_dal_err_funcp(context, krb5_err_have_str, st,
-                                "k5_mutex_init failed");
-#endif
-        goto cleanup;
-    }
-
-    /*
-     * If max_server_conns is not set read it from database module
-     * section of conf file this parameter defines maximum ldap
-     * connections per ldap server.
-     */
+    /* Read the maximum number of LDAP connections per server. */
     if (ldap_context->max_server_conns == 0) {
-        st = prof_get_integer_def (context, conf_section,
+        ret = prof_get_integer_def(context, conf_section,
                                    KRB5_CONF_LDAP_CONNS_PER_SERVER,
                                    DEFAULT_CONNS_PER_SERVER,
                                    &ldap_context->max_server_conns);
-        if (st)
-            goto cleanup;
+        if (ret)
+            return ret;
     }
 
     if (ldap_context->max_server_conns < 2) {
-        st = EINVAL;
-        k5_setmsg(context, st,
+        k5_setmsg(context, EINVAL,
                   _("Minimum connections required per server is 2"));
-        goto cleanup;
+        return EINVAL;
     }
 
-    /*
-     * If the bind dn is not set read it from the database module
-     * section of conf file this paramter is populated by one of the
-     * KDC, ADMIN or PASSWD dn to be used to connect to LDAP
-     * server.  The srv_type decides which dn to read.
-     */
+    /* Read the DN used to connect to the LDAP server. */
     if (ldap_context->bind_dn == NULL) {
-        char *name = 0;
-        if (srv_type == KRB5_KDB_SRV_TYPE_KDC)
-            name = KRB5_CONF_LDAP_KDC_DN;
-        else if (srv_type == KRB5_KDB_SRV_TYPE_ADMIN)
-            name = KRB5_CONF_LDAP_KADMIND_DN;
-
-        if (name) {
-            st = prof_get_string_def (context, conf_section, name,
-                                      &ldap_context->bind_dn);
-            if (st)
-                goto cleanup;
-        }
+        name = choose_var(srv_type, KRB5_CONF_LDAP_KDC_DN,
+                          KRB5_CONF_LDAP_KADMIND_DN);
+        ret = prof_get_string_def(context, conf_section, name,
+                                  &ldap_context->bind_dn);
+        if (ret)
+            return ret;
     }
 
-    /*
-     * Read service_password_file parameter from database module
-     * section of conf file this file contains stashed passwords of
-     * the KDC, ADMIN and PASSWD dns.
-     */
+    /* Read the filename containing stashed DN passwords. */
     if (ldap_context->service_password_file == NULL) {
-        st = prof_get_string_def (context, conf_section,
+        ret = prof_get_string_def(context, conf_section,
                                   KRB5_CONF_LDAP_SERVICE_PASSWORD_FILE,
                                   &ldap_context->service_password_file);
-        if (st)
-            goto cleanup;
+        if (ret)
+            return ret;
     }
 
-    /*
-     * If the ldap server parameter is not set read the list of ldap
-     * servers from the database module section of the conf file.
-     */
-
+    /* Read the LDAP server URL list. */
     if (ldap_context->server_info_list == NULL) {
-        if ((st=profile_get_string(context->profile, KDB_MODULE_SECTION, conf_section,
-                                   KRB5_CONF_LDAP_SERVERS, NULL, &tempval)) != 0) {
-            k5_setmsg(context, st,
-                      _("Error reading 'ldap_servers' attribute"));
-            goto cleanup;
-        }
-
-        if (tempval == NULL) {
-            st = add_server_entry(context, "ldapi://");
-            if (st)
-                goto cleanup;
+        ret = profile_get_string(context->profile, KDB_MODULE_SECTION,
+                                 conf_section, KRB5_CONF_LDAP_SERVERS, NULL,
+                                 &servers);
+        if (ret)
+            return attr_read_error(context, ret, KRB5_CONF_LDAP_SERVERS);
+
+        if (servers == NULL) {
+            ret = add_server_entry(context, "ldapi://");
+            if (ret)
+                return ret;
         } else {
-            item = strtok_r(tempval, delims, &save_ptr);
+            item = strtok_r(servers, delims, &save_ptr);
             while (item != NULL) {
-                st = add_server_entry(context, item);
-                if (st)
-                    goto cleanup;
-                item = strtok_r(NULL,delims,&save_ptr);
+                ret = add_server_entry(context, item);
+                if (ret) {
+                    profile_release_string(servers);
+                    return ret;
+                }
+                item = strtok_r(NULL, delims, &save_ptr);
             }
-            profile_release_string(tempval);
+            profile_release_string(servers);
         }
     }
 
-    if ((st = prof_get_boolean_def(context, conf_section,
-                                   KRB5_CONF_DISABLE_LAST_SUCCESS, FALSE,
-                                   &ldap_context->disable_last_success)))
-        goto cleanup;
-
-    if ((st = prof_get_boolean_def(context, conf_section,
-                                   KRB5_CONF_DISABLE_LOCKOUT, FALSE,
-                                   &ldap_context->disable_lockout)))
-        goto cleanup;
+    ret = prof_get_boolean_def(context, conf_section,
+                               KRB5_CONF_DISABLE_LAST_SUCCESS, FALSE,
+                               &ldap_context->disable_last_success);
+    if (ret)
+        return ret;
 
-cleanup:
-    return(st);
+    return prof_get_boolean_def(context, conf_section,
+                                KRB5_CONF_DISABLE_LOCKOUT, FALSE,
+                                &ldap_context->disable_lockout);
 }
 
-/*
- * This function frees the krb5_ldap_context structure members.
- */
-
-krb5_error_code
-krb5_ldap_free_server_context_params(krb5_ldap_context *ldap_context)
+void
+krb5_ldap_free_server_context_params(krb5_ldap_context *ctx)
 {
-    int                         i=0;
-    krb5_ldap_server_handle     *ldap_server_handle=NULL, *next_ldap_server_handle=NULL;
-
-    if (ldap_context == NULL)
-        return 0;
-
-    /* Free all ldap servers list and the ldap handles associated with
-       the ldap server.  */
-    if (ldap_context->server_info_list) {
-        while (ldap_context->server_info_list[i]) {
-            if (ldap_context->server_info_list[i]->server_name) {
-                free (ldap_context->server_info_list[i]->server_name);
-            }
-            if (ldap_context->server_info_list[i]->ldap_server_handles) {
-                ldap_server_handle = ldap_context->server_info_list[i]->ldap_server_handles;
-                while (ldap_server_handle) {
-                    ldap_unbind_ext_s(ldap_server_handle->ldap_handle, NULL, NULL);
-                    ldap_server_handle->ldap_handle = NULL;
-                    next_ldap_server_handle = ldap_server_handle->next;
-                    free(ldap_server_handle);
-                    ldap_server_handle = next_ldap_server_handle;
-                }
-            }
-            free(ldap_context->server_info_list[i]);
-            i++;
+    int i;
+    krb5_ldap_server_info **list;
+    krb5_ldap_server_handle *h, *next;
+
+    if (ctx == NULL)
+        return;
+
+    list = ctx->server_info_list;
+    for (i = 0; list != NULL && list[i] != NULL; i++) {
+        free(list[i]->server_name);
+        for (h = list[i]->ldap_server_handles; h != NULL; h = next) {
+            next = h->next;
+            ldap_unbind_ext_s(h->ldap_handle, NULL, NULL);
+            free(h);
         }
-        free(ldap_context->server_info_list);
-    }
-
-    if (ldap_context->conf_section != NULL) {
-        free(ldap_context->conf_section);
-        ldap_context->conf_section = NULL;
+        free(list[i]);
     }
+    free(list);
+    ctx->server_info_list = NULL;
 
-    if (ldap_context->bind_dn != NULL) {
-        free(ldap_context->bind_dn);
-        ldap_context->bind_dn = NULL;
-    }
-
-    if (ldap_context->bind_pwd != NULL) {
-        memset(ldap_context->bind_pwd, 0, strlen(ldap_context->bind_pwd));
-        free(ldap_context->bind_pwd);
-        ldap_context->bind_pwd = NULL;
-    }
-
-    if (ldap_context->service_password_file != NULL) {
-        free(ldap_context->service_password_file);
-        ldap_context->service_password_file = NULL;
-    }
-
-    if (ldap_context->certificates) {
-        i=0;
-        while (ldap_context->certificates[i] != NULL) {
-            free(ldap_context->certificates[i]->certificate);
-            free(ldap_context->certificates[i]);
-            ++i;
-        }
-        free(ldap_context->certificates);
-    }
-
-    return(0);
+    free(ctx->conf_section);
+    free(ctx->bind_dn);
+    zapfreestr(ctx->bind_pwd);
+    free(ctx->service_password_file);
+    ctx->conf_section = ctx->bind_dn = ctx->bind_pwd = NULL;
+    ctx->service_password_file = NULL;
 }
 
-krb5_error_code
-krb5_ldap_free_server_params(krb5_ldap_context *ldap_context)
+void
+krb5_ldap_free_server_params(krb5_ldap_context *ctx)
 {
-    if (ldap_context == NULL)
-        return 0;
-
-    krb5_ldap_free_server_context_params(ldap_context);
-
-    k5_mutex_destroy(&ldap_context->hndl_lock);
-    free(ldap_context);
-    return(0);
+    if (ctx == NULL)
+        return;
+    krb5_ldap_free_server_context_params(ctx);
+    k5_mutex_destroy(&ctx->hndl_lock);
+    free(ctx);
 }
 
-/*
- * check to see if the principal belongs to the default realm.
- * The default realm is present in the krb5_ldap_context structure.
- * The principal has a realm portion. This realm portion is compared with the default realm
- * to check whether the principal belong to the default realm.
- * Return 0 if principal belongs to default realm else 1.
- */
-
-krb5_error_code
+/* Return true if princ is in the default realm of ldap_context or is a
+ * cross-realm TGS principal for that realm. */
+krb5_boolean
 is_principal_in_realm(krb5_ldap_context *ldap_context,
-                      krb5_const_principal searchfor)
+                      krb5_const_principal princ)
 {
-    char                        *defrealm=NULL;
-
-#define FIND_MAX(a,b) ((a) > (b) ? (a) : (b))
-
-    defrealm = ldap_context->lrparams->realm_name;
+    const char *realm = ldap_context->lrparams->realm_name;
 
-    /*
-     * Care should be taken for inter-realm principals as the default
-     * realm can exist in the realm part of the principal name or can
-     * also exist in the second portion of the name part.  However, if
-     * the default realm exist in the second part of the principal
-     * portion, then the first portion of the principal name SHOULD be
-     * "krbtgt".  All this check is done in the immediate block.
-     */
-    if (searchfor->length == 2) {
-        if (data_eq_string(searchfor->data[0], "krbtgt") &&
-            data_eq_string(searchfor->data[1], defrealm))
-            return 0;
-    }
-
-    /* first check the length, if they are not equal, then they are not same */
-    if (strlen(defrealm) != searchfor->realm.length)
-        return 1;
-
-    /* if the length is equal, check for the contents */
-    if (strncmp(defrealm, searchfor->realm.data,
-                searchfor->realm.length) != 0)
-        return 1;
-    /* if we are here, then the realm portions match, return 0 */
-    return 0;
+    if (princ->length == 2 &&
+        data_eq_string(princ->data[0], "krbtgt") &&
+        data_eq_string(princ->data[1], realm))
+        return TRUE;
+    return data_eq_string(princ->realm, realm);
 }
 
-
 /*
  * Deduce the subtree information from the context. A realm can have
  * multiple subtrees.
@@ -590,12 +451,10 @@ krb5_error_code
 krb5_get_subtree_info(krb5_ldap_context *ldap_context, char ***subtreearr,
                       unsigned int *ntree)
 {
-    int                         st=0, i=0, subtreecount=0;
-    int                         ncount=0, search_scope=0;
-    char                        **subtree=NULL, *realm_cont_dn=NULL;
-    char                        **subtarr=NULL;
-    char                        *containerref=NULL;
-    char                        **newsubtree=NULL;
+    krb5_error_code ret;
+    int subtreecount, count = 0, search_scope;
+    char **subtree, *realm_cont_dn, *containerref;
+    char **subtarr = NULL;
 
     containerref = ldap_context->lrparams->containerref;
     subtree = ldap_context->lrparams->subtree;
@@ -603,292 +462,208 @@ krb5_get_subtree_info(krb5_ldap_context *ldap_context, char ***subtreearr,
     subtreecount = ldap_context->lrparams->subtreecount;
     search_scope = ldap_context->lrparams->search_scope;
 
-    subtarr = (char **) malloc(sizeof(char *) * (subtreecount + 1 /*realm dn*/ + 1 /*containerref*/ + 1));
-    if (subtarr == NULL) {
-        st = ENOMEM;
+    /* Leave space for realm DN, containerref, and null terminator. */
+    subtarr = k5calloc(subtreecount + 3, sizeof(char *), &ret);
+    if (subtarr == NULL)
         goto cleanup;
-    }
-    memset(subtarr, 0, (sizeof(char *) * (subtreecount+1+1+1)));
 
-    /* get the complete subtree list */
-    for (i=0; i<subtreecount && subtree[i]!=NULL; i++) {
-        subtarr[i] = strdup(subtree[i]);
-        if (subtarr[i] == NULL) {
-            st = ENOMEM;
+    /* Get the complete subtree list. */
+    while (count < subtreecount && subtree[count] != NULL) {
+        subtarr[count] = strdup(subtree[count]);
+        if (subtarr[count++] == NULL) {
+            ret = ENOMEM;
             goto cleanup;
         }
     }
 
-    subtarr[i] = strdup(realm_cont_dn);
-    if (subtarr[i++] == NULL) {
-        st = ENOMEM;
+    subtarr[count] = strdup(realm_cont_dn);
+    if (subtarr[count++] == NULL) {
+        ret = ENOMEM;
         goto cleanup;
     }
 
     if (containerref != NULL) {
-        subtarr[i] = strdup(containerref);
-        if (subtarr[i++] == NULL) {
-            st = ENOMEM;
+        subtarr[count] = strdup(containerref);
+        if (subtarr[count++] == NULL) {
+            ret = ENOMEM;
             goto cleanup;
         }
     }
 
-    ncount = i;
-    newsubtree = (char **) malloc(sizeof(char *) * (ncount + 1));
-    if (newsubtree == NULL) {
-        st = ENOMEM;
-        goto cleanup;
-    }
-    memset(newsubtree, 0, (sizeof(char *) * (ncount+1)));
-    if ((st = remove_overlapping_subtrees(subtarr, newsubtree, &ncount,
-                                          search_scope)) != 0) {
-        goto cleanup;
-    }
-
-    *ntree = ncount;
-    *subtreearr = newsubtree;
+    remove_overlapping_subtrees(subtarr, &count, search_scope);
+    *ntree = count;
+    *subtreearr = subtarr;
+    subtarr = NULL;
+    count = 0;
 
 cleanup:
-    if (subtarr != NULL) {
-        for (i=0; subtarr[i] != NULL; i++)
-            free(subtarr[i]);
-        free(subtarr);
-    }
-
-    if (st != 0) {
-        if (newsubtree != NULL) {
-            for (i=0; newsubtree[i] != NULL; i++)
-                free(newsubtree[i]);
-            free(newsubtree);
-        }
-    }
-    return st;
+    while (count > 0)
+        free(subtarr[--count]);
+    free(subtarr);
+    return ret;
 }
 
-/*
- * This function appends the content with a type into the tl_data
- * structure.  Based on the type the length of the content is either
- * pre-defined or computed from the content.  Returns 0 in case of
- * success and 1 if the type associated with the content is undefined.
- */
+/* Reallocate tl and return a pointer to the new space, or NULL on failure. */
+static unsigned char *
+expand_tl_data(krb5_tl_data *tl, uint16_t len)
+{
+    unsigned char *newptr;
+
+    if (len > UINT16_MAX - tl->tl_data_length)
+        return NULL;
+    newptr = realloc(tl->tl_data_contents, tl->tl_data_length + len);
+    if (newptr == NULL)
+        return NULL;
+    tl->tl_data_contents = newptr;
+    tl->tl_data_length += len;
+    return tl->tl_data_contents + tl->tl_data_length - len;
+}
 
+/* Append a one-byte type, a two-byte length, and a value to a KDB_TL_USER_INFO
+ * tl_data item.  The length is inferred from type and value. */
 krb5_error_code
-store_tl_data(krb5_tl_data *tl_data, int tl_type, void *value)
+store_tl_data(krb5_tl_data *tl, int type, void *value)
 {
-    unsigned int                currlen=0, tldatalen=0;
-    unsigned char               *curr=NULL;
-    void                        *reallocptr=NULL;
+    unsigned char *ptr;
+    int ival;
+    char *str;
+    size_t len;
 
-    tl_data->tl_data_type = KDB_TL_USER_INFO;
-    switch (tl_type) {
+    tl->tl_data_type = KDB_TL_USER_INFO;
+    switch (type) {
     case KDB_TL_PRINCCOUNT:
     case KDB_TL_PRINCTYPE:
     case KDB_TL_MASK:
-    {
-        int *iptr = (int *)value;
-        int ivalue = *iptr;
-
-        currlen = tl_data->tl_data_length;
-        tl_data->tl_data_length += 1 + 2 + 2;
-        /* allocate required memory */
-        reallocptr = tl_data->tl_data_contents;
-        tl_data->tl_data_contents = realloc(tl_data->tl_data_contents,
-                                            tl_data->tl_data_length);
-        if (tl_data->tl_data_contents == NULL) {
-            if (reallocptr)
-                free (reallocptr);
+        ival = *(int *)value;
+        if (ival > UINT16_MAX)
+            return EINVAL;
+        ptr = expand_tl_data(tl, 5);
+        if (ptr == NULL)
             return ENOMEM;
-        }
-        curr = (tl_data->tl_data_contents + currlen);
-
-        /* store the tl_type value */
-        memset(curr, tl_type, 1);
-        curr += 1;
-        /* store the content length */
-        tldatalen = 2;
-        STORE16_INT(curr, tldatalen);
-        curr += 2;
-        /* store the content */
-        STORE16_INT(curr, ivalue);
-        curr += 2;
+        *ptr = type;
+        store_16_be(2, ptr + 1);
+        store_16_be(ival, ptr + 3);
         break;
-    }
 
     case KDB_TL_USERDN:
     case KDB_TL_LINKDN:
-    {
-        char *cptr = (char *)value;
-
-        currlen = tl_data->tl_data_length;
-        tl_data->tl_data_length += 1 + 2 + strlen(cptr);
-        /* allocate required memory */
-        reallocptr = tl_data->tl_data_contents;
-        tl_data->tl_data_contents = realloc(tl_data->tl_data_contents,
-                                            tl_data->tl_data_length);
-        if (tl_data->tl_data_contents == NULL) {
-            if (reallocptr)
-                free (reallocptr);
+        str = value;
+        len = strlen(str);
+        if (len > UINT16_MAX - 3)
             return ENOMEM;
-        }
-        curr = (tl_data->tl_data_contents + currlen);
-
-        /* store the tl_type value */
-        memset(curr, tl_type, 1);
-        curr += 1;
-        /* store the content length */
-        tldatalen = strlen(cptr);
-        STORE16_INT(curr, tldatalen);
-        curr += 2;
-        /* store the content */
-        memcpy(curr, cptr, tldatalen);
-        curr += tldatalen;
+        ptr = expand_tl_data(tl, 3 + len);
+        if (ptr == NULL)
+            return ENOMEM;
+        *ptr = type;
+        store_16_be(len, ptr + 1);
+        memcpy(ptr + 3, str, len);
         break;
-    }
 
     default:
-        return 1;
-
+        return EINVAL;
     }
     return 0;
 }
 
-/*
- * This function scans the tl_data structure to get the value of a
- * type defined by the tl_type (second parameter).  The tl_data
- * structure has all the data in the tl_data_contents member.  The
- * format of the tl_data_contents is as follows.  The first byte
- * defines the type of the content that follows.  The next 2 bytes
- * define the size n (in terms of bytes) of the content that
- * follows.  The next n bytes define the content itself.
- */
-
-krb5_error_code
-decode_tl_data(krb5_tl_data *tl_data, int tl_type, void **data)
+/* Scan tl for a value of the given type and return it in allocated memory.
+ * For KDB_TL_LINKDN, return a list of all values found. */
+static krb5_error_code
+decode_tl_data(krb5_tl_data *tl, int type, void **data_out)
 {
-    int                         subtype=0, i=0, limit=10;
-    unsigned int                sublen=0;
-    unsigned char               *curr=NULL;
-    int                         *intptr=NULL;
-    long                        *longptr=NULL;
-    char                        *DN=NULL, **DNarr=NULL;
-    krb5_error_code             st=-1;
-
-    *data = NULL;
-
-    curr = tl_data->tl_data_contents;
-    while (curr < (tl_data->tl_data_contents + tl_data->tl_data_length)) {
-
-        /* get the type of the content */
-        subtype = (int) curr[0];
-        /* forward by 1 byte*/
-        curr += 1;
-
-        if (subtype == tl_type) {
-            switch (subtype) {
-
-            case KDB_TL_PRINCCOUNT:
-            case KDB_TL_PRINCTYPE:
-            case KDB_TL_MASK:
-                /* get the length of the content */
-                UNSTORE16_INT(curr, sublen);
-                /* forward by 2 bytes */
-                curr += 2;
-                /* get the actual content */
-                if (sublen == 2) {
-                    /* intptr = malloc(sublen);   */
-                    intptr = malloc(sizeof(krb5_int32));
-                    if (intptr == NULL)
-                        return ENOMEM;
-                    memset(intptr, 0, sublen);
-                    UNSTORE16_INT(curr, (*intptr));
-                    *data = intptr;
-                } else {
-                    longptr = malloc(sublen);
-                    if (longptr == NULL)
-                        return ENOMEM;
-                    memset(longptr, 0, sublen);
-                    UNSTORE32_INT(curr, (*longptr));
-                    *data = longptr;
-                }
-                curr += sublen;
-                st = 0;
-                return st;
-                break;
-
-            case KDB_TL_CONTAINERDN:
-            case KDB_TL_USERDN:
-                /* get the length of the content */
-                UNSTORE16_INT(curr, sublen);
-                /* forward by 2 bytes */
-                curr += 2;
-                DN = k5memdup0(curr, sublen, &st);
-                if (DN == NULL)
-                    return st;
-                *data = DN;
-                curr += sublen;
-                st = 0;
-                return st;
-                break;
-
-            case KDB_TL_LINKDN:
-                if (DNarr == NULL) {
-                    DNarr = calloc(limit, sizeof(char *));
-                    if (DNarr == NULL)
-                        return ENOMEM;
-                }
-                if (i == limit-1) {
-                    limit *= 2;
-                    DNarr = realloc(DNarr, sizeof(char *) * (limit));
-                    if (DNarr == NULL)
-                        return ENOMEM;
-                }
+    krb5_error_code ret;
+    const unsigned char *ptr, *end;
+    uint16_t len;
+    size_t linkcount = 0, i;
+    char **dnlist = NULL, **newlist;
+    int *intptr;
+
+    *data_out = NULL;
+
+    /* Find the first matching subfield or return ENOENT.  For KDB_TL_LINKDN,
+     * keep iterating after finding a match as it may be repeated. */
+    ptr = tl->tl_data_contents;
+    end = ptr + tl->tl_data_length;
+    for (;;) {
+        if (end - ptr < 3)
+            break;
+        len = load_16_be(ptr + 1);
+        if (len > (end - ptr) - 3)
+            break;
+        if (*ptr != type) {
+            ptr += 3 + len;
+            continue;
+        }
+        ptr += 3;
+
+        switch (type) {
+        case KDB_TL_PRINCCOUNT:
+        case KDB_TL_PRINCTYPE:
+        case KDB_TL_MASK:
+            if (len != 2)
+                return EINVAL;
+            intptr = malloc(sizeof(int));
+            if (intptr == NULL)
+                return ENOMEM;
+            *intptr = load_16_be(ptr);
+            *data_out = intptr;
+            return 0;
 
-                /* get the length of the content */
-                UNSTORE16_INT(curr, sublen);
-                /* forward by 2 bytes */
-                curr += 2;
-                DNarr[i] = k5memdup0(curr, sublen, &st);
-                if (DNarr[i] == NULL)
-                    return st;
-                ++i;
-                curr += sublen;
-                *data = DNarr;
-                st=0;
-                break;
-            }
-        } else {
-            /* move to the current content block */
-            UNSTORE16_INT(curr, sublen);
-            curr += 2 + sublen;
+        case KDB_TL_USERDN:
+            *data_out = k5memdup0(ptr, len, &ret);
+            return ret;
+
+        case KDB_TL_LINKDN:
+            newlist = realloc(dnlist, (linkcount + 2) * sizeof(char *));
+            if (newlist == NULL)
+                goto oom;
+            dnlist = newlist;
+            dnlist[linkcount] = k5memdup0(ptr, len, &ret);
+            if (dnlist[linkcount] == NULL)
+                goto oom;
+            dnlist[linkcount + 1] = NULL;
+            linkcount++;
+            break;
         }
+
+        ptr += len;
     }
-    return st;
+
+    if (type != KDB_TL_LINKDN || dnlist == NULL)
+        return ENOENT;
+    *data_out = dnlist;
+    return 0;
+
+oom:
+    for (i = 0; i < linkcount; i++)
+        free(dnlist[i]);
+    free(dnlist);
+    return ENOMEM;
 }
 
 /*
  * wrapper routines for decode_tl_data
  */
 static krb5_error_code
-krb5_get_int_from_tl_data(krb5_context context, krb5_db_entry *entries,
-                          int type, int *intval)
+get_int_from_tl_data(krb5_context context, krb5_db_entry *entry, int type,
+                     int *intval)
 {
-    krb5_error_code             st=0;
-    krb5_tl_data                tl_data;
-    void                        *voidptr=NULL;
-    int                         *intptr=NULL;
+    krb5_error_code ret;
+    krb5_tl_data tl_data;
+    void *ptr;
+    int *intptr;
 
     tl_data.tl_data_type = KDB_TL_USER_INFO;
-    if (((st=krb5_dbe_lookup_tl_data(context, entries, &tl_data)) != 0) || tl_data.tl_data_length == 0)
-        goto cleanup;
+    ret = krb5_dbe_lookup_tl_data(context, entry, &tl_data);
+    if (ret || tl_data.tl_data_length == 0)
+        return ret;
 
-    if (decode_tl_data(&tl_data, type, &voidptr) == 0) {
-        intptr = (int *) voidptr;
+    if (decode_tl_data(&tl_data, type, &ptr) == 0) {
+        intptr = ptr;
         *intval = *intptr;
         free(intptr);
     }
 
-cleanup:
-    return st;
+    return 0;
 }
 
 /*
@@ -896,171 +671,132 @@ cleanup:
  * object (user, policy ...).
  */
 krb5_error_code
-krb5_get_attributes_mask(krb5_context context, krb5_db_entry *entries,
+krb5_get_attributes_mask(krb5_context context, krb5_db_entry *entry,
                          int *mask)
 {
-    return krb5_get_int_from_tl_data(context, entries, KDB_TL_MASK, mask);
+    return get_int_from_tl_data(context, entry, KDB_TL_MASK, mask);
 }
 
 krb5_error_code
-krb5_get_princ_type(krb5_context context, krb5_db_entry *entries, int *ptype)
+krb5_get_princ_type(krb5_context context, krb5_db_entry *entry, int *ptype)
 {
-    return krb5_get_int_from_tl_data(context, entries, KDB_TL_PRINCTYPE, ptype);
+    return get_int_from_tl_data(context, entry, KDB_TL_PRINCTYPE, ptype);
 }
 
 krb5_error_code
-krb5_get_princ_count(krb5_context context, krb5_db_entry *entries, int *pcount)
+krb5_get_princ_count(krb5_context context, krb5_db_entry *entry, int *pcount)
 {
-    return krb5_get_int_from_tl_data(context, entries, KDB_TL_PRINCCOUNT, pcount);
+    return get_int_from_tl_data(context, entry, KDB_TL_PRINCCOUNT, pcount);
 }
 
 krb5_error_code
-krb5_get_linkdn(krb5_context context, krb5_db_entry *entries, char ***link_dn)
+krb5_get_linkdn(krb5_context context, krb5_db_entry *entry, char ***link_dn)
 {
-    krb5_error_code             st=0;
-    krb5_tl_data                tl_data;
-    void                        *voidptr=NULL;
+    krb5_error_code ret;
+    krb5_tl_data tl_data;
+    void *ptr;
 
     *link_dn = NULL;
     tl_data.tl_data_type = KDB_TL_USER_INFO;
-    if (((st=krb5_dbe_lookup_tl_data(context, entries, &tl_data)) != 0) || tl_data.tl_data_length == 0)
-        goto cleanup;
+    ret = krb5_dbe_lookup_tl_data(context, entry, &tl_data);
+    if (ret || tl_data.tl_data_length == 0)
+        return ret;
 
-    if (decode_tl_data(&tl_data, KDB_TL_LINKDN, &voidptr) == 0) {
-        *link_dn = (char **) voidptr;
-    }
+    if (decode_tl_data(&tl_data, KDB_TL_LINKDN, &ptr) == 0)
+        *link_dn = ptr;
 
-cleanup:
-    return st;
+    return 0;
 }
 
 static krb5_error_code
-krb5_get_str_from_tl_data(krb5_context context, krb5_db_entry *entries,
-                          int type, char **strval)
+get_str_from_tl_data(krb5_context context, krb5_db_entry *entry, int type,
+                     char **strval)
 {
-    krb5_error_code             st=0;
-    krb5_tl_data                tl_data;
-    void                        *voidptr=NULL;
+    krb5_error_code ret;
+    krb5_tl_data tl_data;
+    void *ptr;
 
-    if (type != KDB_TL_USERDN && type != KDB_TL_CONTAINERDN) {
-        st = EINVAL;
-        goto cleanup;
-    }
+    if (type != KDB_TL_USERDN)
+        return EINVAL;
 
     tl_data.tl_data_type = KDB_TL_USER_INFO;
-    if (((st=krb5_dbe_lookup_tl_data(context, entries, &tl_data)) != 0) || tl_data.tl_data_length == 0)
-        goto cleanup;
+    ret = krb5_dbe_lookup_tl_data(context, entry, &tl_data);
+    if (ret || tl_data.tl_data_length == 0)
+        return ret;
 
-    if (decode_tl_data(&tl_data, type, &voidptr) == 0) {
-        *strval = (char *) voidptr;
-    }
+    if (decode_tl_data(&tl_data, type, &ptr) == 0)
+        *strval = ptr;
 
-cleanup:
-    return st;
+    return 0;
 }
 
 krb5_error_code
-krb5_get_userdn(krb5_context context, krb5_db_entry *entries, char **userdn)
+krb5_get_userdn(krb5_context context, krb5_db_entry *entry, char **userdn)
 {
     *userdn = NULL;
-    return krb5_get_str_from_tl_data(context, entries, KDB_TL_USERDN, userdn);
-}
-
-krb5_error_code
-krb5_get_containerdn(krb5_context context, krb5_db_entry *entries,
-                     char **containerdn)
-{
-    *containerdn = NULL;
-    return krb5_get_str_from_tl_data(context, entries, KDB_TL_CONTAINERDN, containerdn);
+    return get_str_from_tl_data(context, entry, KDB_TL_USERDN, userdn);
 }
 
 /*
- * This function reads the attribute values (if the attribute is
- * non-null) from the dn.  The read attribute values is compared
- * aganist the attrvalues passed to the function and a bit mask is set
- * for all the matching attributes (attributes existing in both list).
- * The bit to be set is selected such that the index of the attribute
- * in the attrvalues parameter is the position of the bit.  For ex:
- * the first element in the attrvalues is present in both list shall
- * set the LSB of the bit mask.
- *
- * In case if either the attribute or the attrvalues parameter to the
- * function is NULL, then the existence of the object is considered
- * and appropriate status is returned back.
+ * If attribute or attrvalues is NULL, just check for the existence of dn.
+ * Otherwise, read values for attribute from dn; then set the bit 1<<n in mask
+ * for each attrvalues[n] which is present in the values read.
  */
-
 krb5_error_code
 checkattributevalue(LDAP *ld, char *dn, char *attribute, char **attrvalues,
                     int *mask)
 {
-    int                         st=0, one=1;
-    char                        **values=NULL, *attributes[2] = {NULL};
-    LDAPMessage                 *result=NULL, *entry=NULL;
+    krb5_error_code ret;
+    int one = 1, i, j;
+    char **values = NULL, *attributes[2] = { NULL };
+    LDAPMessage *result = NULL, *entry;
 
-    if (strlen(dn) == 0) {
-        st = set_ldap_error(0, LDAP_NO_SUCH_OBJECT, OP_SEARCH);
-        return st;
-    }
+    if (strlen(dn) == 0)
+        return set_ldap_error(0, LDAP_NO_SUCH_OBJECT, OP_SEARCH);
 
     attributes[0] = attribute;
 
-    /* read the attribute values from the dn */
-    if ((st = ldap_search_ext_s(ld,
-                                dn,
-                                LDAP_SCOPE_BASE,
-                                0,
-                                attributes,
-                                0,
-                                NULL,
-                                NULL,
-                                &timelimit,
-                                LDAP_NO_LIMIT,
-                                &result)) != LDAP_SUCCESS) {
-        st = set_ldap_error(0, st, OP_SEARCH);
-        goto cleanup;
+    /* Read values for attribute from the dn, or check for its existence. */
+    ret = ldap_search_ext_s(ld, dn, LDAP_SCOPE_BASE, 0, attributes, 0, NULL,
+                            NULL, &timelimit, LDAP_NO_LIMIT, &result);
+    if (ret != LDAP_SUCCESS) {
+        ldap_msgfree(result);
+        return set_ldap_error(0, ret, OP_SEARCH);
     }
 
-    /*
-     * If the attribute/attrvalues is NULL, then check for the
-     * existence of the object alone.
-     */
+    /* Don't touch *mask if we are only checking for existence. */
     if (attribute == NULL || attrvalues == NULL)
-        goto cleanup;
+        goto done;
 
-    /* reset the bit mask */
     *mask = 0;
 
-    if ((entry=ldap_first_entry(ld, result)) != NULL) {
-        /* read the attribute values */
-        if ((values=ldap_get_values(ld, entry, attribute)) != NULL) {
-            int i,j;
-
-            /*
-             * Compare the read attribute values with the attrvalues
-             * array and set the appropriate bit mask.
-             */
-            for (j=0; attrvalues[j]; ++j) {
-                for (i=0; values[i]; ++i) {
-                    if (strcasecmp(values[i], attrvalues[j]) == 0) {
-                        *mask |= (one<<j);
-                        break;
-                    }
-                }
+    entry = ldap_first_entry(ld, result);
+    if (entry == NULL)
+        goto done;
+    values = ldap_get_values(ld, entry, attribute);
+    if (values == NULL)
+        goto done;
+
+    /* Set bits in mask for each matching value we read. */
+    for (i = 0; attrvalues[i]; i++) {
+        for (j = 0; values[j]; j++) {
+            if (strcasecmp(attrvalues[i], values[j]) == 0) {
+                *mask |= (one << i);
+                break;
             }
-            ldap_value_free(values);
         }
     }
 
-cleanup:
+done:
     ldap_msgfree(result);
-    return st;
+    ldap_value_free(values);
+    return 0;
 }
 
-
 static krb5_error_code
 getepochtime(char *strtime, krb5_timestamp *epochtime)
 {
-    struct tm           tme;
+    struct tm tme;
 
     memset(&tme, 0, sizeof(tme));
     if (strptime(strtime,"%Y%m%d%H%M%SZ", &tme) == NULL) {
@@ -1071,251 +807,194 @@ getepochtime(char *strtime, krb5_timestamp *epochtime)
     return 0;
 }
 
-/*
- * krb5_ldap_get_value() - get the integer value of the attribute
- * Returns, 0 if the attribute is present, 1 if the attribute is missing.
- * The retval is 0 if the attribute is missing.
- */
-
+/* Get the integer value of attribute from int.  If it is not found, return
+ * ENOENT and set *val_out to 0. */
 krb5_error_code
-krb5_ldap_get_value(LDAP *ld, LDAPMessage *ent, char *attribute, int *retval)
+krb5_ldap_get_value(LDAP *ld, LDAPMessage *ent, char *attribute, int *val_out)
 {
-    char                           **values=NULL;
-
-    *retval = 0;
-    values=ldap_get_values(ld, ent, attribute);
-    if (values != NULL) {
-        if (values[0] != NULL)
-            *retval = atoi(values[0]);
-        ldap_value_free(values);
-        return 0;
-    }
-    return 1;
+    char **values;
+
+    *val_out = 0;
+    values = ldap_get_values(ld, ent, attribute);
+    if (values == NULL)
+        return ENOENT;
+    if (values[0] != NULL)
+        *val_out = atoi(values[0]);
+    ldap_value_free(values);
+    return 0;
 }
 
-/*
- * krb5_ldap_get_string() - Returns the first string of the
- * attribute.  Intended to
- *
- *
- */
+/* Return the first string value of attribute in ent. */
 krb5_error_code
 krb5_ldap_get_string(LDAP *ld, LDAPMessage *ent, char *attribute,
-                     char **retstr, krb5_boolean *attr_present)
+                     char **str_out, krb5_boolean *attr_present)
 {
-    char                           **values=NULL;
-    krb5_error_code                st=0;
+    char **values;
+    krb5_error_code ret = 0;
 
-    *retstr = NULL;
+    *str_out = NULL;
     if (attr_present != NULL)
         *attr_present = FALSE;
 
-    values=ldap_get_values(ld, ent, attribute);
-    if (values != NULL) {
-        if (values[0] != NULL) {
-            if (attr_present!= NULL)
-                *attr_present = TRUE;
-            *retstr = strdup(values[0]);
-            if (*retstr == NULL)
-                st = ENOMEM;
-        }
-        ldap_value_free(values);
-    }
-    return st;
-}
-
-/*
- * krb5_ldap_get_strings() - Returns all the values
- * of the attribute.
- */
-krb5_error_code
-krb5_ldap_get_strings(LDAP *ld, LDAPMessage *ent, char *attribute,
-                      char ***retarr, krb5_boolean *attr_present)
-{
-    char                        **values=NULL;
-    krb5_error_code             st=0;
-    unsigned int                i=0, count=0;
-
-    *retarr = NULL;
-    if (attr_present != NULL)
-        *attr_present = FALSE;
-
-    values=ldap_get_values(ld, ent, attribute);
-    if (values != NULL) {
+    values = ldap_get_values(ld, ent, attribute);
+    if (values == NULL)
+        return 0;
+    if (values[0] != NULL) {
         if (attr_present != NULL)
             *attr_present = TRUE;
-
-        count = ldap_count_values(values);
-        *retarr  = (char **) calloc(count+1, sizeof(char *));
-        if (*retarr == NULL) {
-            st = ENOMEM;
-            return st;
-        }
-        for (i=0; i< count; ++i) {
-            (*retarr)[i] = strdup(values[i]);
-            if ((*retarr)[i] == NULL) {
-                st = ENOMEM;
-                goto cleanup;
-            }
-        }
-        ldap_value_free(values);
+        *str_out = strdup(values[0]);
+        if (*str_out == NULL)
+            ret = ENOMEM;
     }
-
-cleanup:
-    if (st != 0) {
-        if (*retarr != NULL) {
-            for (i=0; i< count; ++i)
-                if ((*retarr)[i] != NULL)
-                    free ((*retarr)[i]);
-            free (*retarr);
-        }
-    }
-    return st;
+    ldap_value_free(values);
+    return ret;
 }
 
-krb5_error_code
-krb5_ldap_get_time(LDAP *ld, LDAPMessage *ent, char *attribute,
-                   krb5_timestamp *rettime, krb5_boolean *attr_present)
+static krb5_error_code
+get_time(LDAP *ld, LDAPMessage *ent, char *attribute, krb5_timestamp *time_out,
+         krb5_boolean *attr_present)
 {
-    char                         **values=NULL;
-    krb5_error_code              st=0;
+    char **values = NULL;
+    krb5_error_code ret = 0;
 
-    *rettime = 0;
+    *time_out = 0;
     *attr_present = FALSE;
 
-    values=ldap_get_values(ld, ent, attribute);
-    if (values != NULL) {
-        if (values[0] != NULL) {
-            *attr_present = TRUE;
-            st = getepochtime(values[0], rettime);
-        }
-        ldap_value_free(values);
+    values = ldap_get_values(ld, ent, attribute);
+    if (values == NULL)
+        return 0;
+    if (values[0] != NULL) {
+        *attr_present = TRUE;
+        ret = getepochtime(values[0], time_out);
     }
-    return st;
+    ldap_value_free(values);
+    return ret;
 }
 
-/*
- * Function to allocate, set the values of LDAPMod structure. The
- * LDAPMod structure is then added to the array at the ind
- */
-
-krb5_error_code
-krb5_add_member(LDAPMod ***mods, int *count)
+/* Add an entry to *list_inout and return it in *mod_out. */
+static krb5_error_code
+alloc_mod(LDAPMod ***list_inout, LDAPMod **mod_out)
 {
-    int i=0;
-    LDAPMod **lmods=NULL;
+    size_t count;
+    LDAPMod **mods = *list_inout;
 
-    if ((*mods) != NULL) {
-        for (;(*mods)[i] != NULL; ++i)
-            ;
-    }
-    lmods = (LDAPMod **) realloc((*mods), (2+i) * sizeof(LDAPMod *));
-    if (lmods == NULL)
+    *mod_out = NULL;
+
+    for (count = 0; mods != NULL && mods[count] != NULL; count++);
+    mods = realloc(mods, (count + 2) * sizeof(*mods));
+    if (mods == NULL)
         return ENOMEM;
+    *list_inout = mods;
 
-    *mods = lmods;
-    (*mods)[i+1] = NULL;
-    (*mods)[i] = (LDAPMod *) calloc(1, sizeof (LDAPMod));
-    if ((*mods)[i] == NULL)
+    mods[count] = calloc(1, sizeof(LDAPMod));
+    if (mods[count] == NULL)
         return ENOMEM;
-    *count = i;
+    mods[count + 1] = NULL;
+    *mod_out = mods[count];
     return 0;
 }
 
 krb5_error_code
-krb5_add_str_mem_ldap_mod(LDAPMod ***mods, char *attribute, int op,
+krb5_add_str_mem_ldap_mod(LDAPMod ***list, char *attribute, int op,
                           char **values)
 {
-    int i=0, j=0;
-    krb5_error_code   st=0;
+    krb5_error_code ret;
+    LDAPMod *mod;
+    size_t count, i;
 
-    if ((st=krb5_add_member(mods, &i)) != 0)
-        return st;
+    ret = alloc_mod(list, &mod);
+    if (ret)
+        return ret;
 
-    (*mods)[i]->mod_type = strdup(attribute);
-    if ((*mods)[i]->mod_type == NULL)
+    mod->mod_type = strdup(attribute);
+    if (mod->mod_type == NULL)
         return ENOMEM;
-    (*mods)[i]->mod_op = op;
+    mod->mod_op = op;
+
+    mod->mod_values = NULL;
+    if (values == NULL)
+        return 0;
 
-    (*mods)[i]->mod_values = NULL;
+    for (count = 0; values[count] != NULL; count++);
+    mod->mod_values = calloc(count + 1, sizeof(char *));
+    if (mod->mod_values == NULL)
+        return ENOMEM;
 
-    if (values != NULL) {
-        for (j=0; values[j] != NULL; ++j)
-            ;
-        (*mods)[i]->mod_values = malloc (sizeof(char *) * (j+1));
-        if ((*mods)[i]->mod_values == NULL)
+    for (i = 0; i < count; i++) {
+        mod->mod_values[i] = strdup(values[i]);
+        if (mod->mod_values[i] == NULL)
             return ENOMEM;
-
-        for (j=0; values[j] != NULL; ++j) {
-            (*mods)[i]->mod_values[j] = strdup(values[j]);
-            if ((*mods)[i]->mod_values[j] == NULL)
-                return ENOMEM;
-        }
-        (*mods)[i]->mod_values[j] = NULL;
     }
+    mod->mod_values[i] = NULL;
     return 0;
 }
 
 krb5_error_code
-krb5_add_ber_mem_ldap_mod(LDAPMod ***mods, char *attribute, int op,
+krb5_add_ber_mem_ldap_mod(LDAPMod ***list, char *attribute, int op,
                           struct berval **ber_values)
 {
-    int i=0, j=0;
-    krb5_error_code   st=0;
+    krb5_error_code ret;
+    LDAPMod *mod;
+    size_t count, i;
 
-    if ((st=krb5_add_member(mods, &i)) != 0)
-        return st;
+    ret = alloc_mod(list, &mod);
+    if (ret)
+        return ret;
 
-    (*mods)[i]->mod_type = strdup(attribute);
-    if ((*mods)[i]->mod_type == NULL)
+    mod->mod_type = strdup(attribute);
+    if (mod->mod_type == NULL)
         return ENOMEM;
-    (*mods)[i]->mod_op = op;
+    mod->mod_op = op;
 
-    for (j=0; ber_values[j] != NULL; ++j)
-        ;
-    (*mods)[i]->mod_bvalues = malloc (sizeof(struct berval *) * (j+1));
-    if ((*mods)[i]->mod_bvalues == NULL)
+    for (count = 0; ber_values[count] != NULL; count++);
+    mod->mod_bvalues = calloc(count + 1, sizeof(struct berval *));
+    if (mod->mod_bvalues == NULL)
         return ENOMEM;
 
-    for (j=0; ber_values[j] != NULL; ++j) {
-        (*mods)[i]->mod_bvalues[j] = calloc(1, sizeof(struct berval));
-        if ((*mods)[i]->mod_bvalues[j] == NULL)
+    for (i = 0; i < count; i++) {
+        mod->mod_bvalues[i] = calloc(1, sizeof(struct berval));
+        if (mod->mod_bvalues[i] == NULL)
             return ENOMEM;
 
-        (*mods)[i]->mod_bvalues[j]->bv_len = ber_values[j]->bv_len;
-        (*mods)[i]->mod_bvalues[j]->bv_val =
-            k5memdup(ber_values[j]->bv_val, ber_values[j]->bv_len, &st);
-        if ((*mods)[i]->mod_bvalues[j]->bv_val == NULL)
-            return ENOMEM;
+        mod->mod_bvalues[i]->bv_len = ber_values[i]->bv_len;
+        mod->mod_bvalues[i]->bv_val = k5memdup(ber_values[i]->bv_val,
+                                               ber_values[i]->bv_len, &ret);
+        if (mod->mod_bvalues[i]->bv_val == NULL)
+            return ret;
     }
-    (*mods)[i]->mod_bvalues[j] = NULL;
+    mod->mod_bvalues[i] = NULL;
     return 0;
 }
 
 static inline char *
-format_d (int val)
+format_d(int val)
 {
-    char tmpbuf[2+3*sizeof(val)];
+    char tmpbuf[3 * sizeof(val) + 2];
+
     snprintf(tmpbuf, sizeof(tmpbuf), "%d", val);
     return strdup(tmpbuf);
 }
 
 krb5_error_code
-krb5_add_int_mem_ldap_mod(LDAPMod ***mods, char *attribute, int op, int value)
+krb5_add_int_mem_ldap_mod(LDAPMod ***list, char *attribute, int op, int value)
 {
-    int i=0;
-    krb5_error_code      st=0;
+    krb5_error_code ret;
+    LDAPMod *mod;
 
-    if ((st=krb5_add_member(mods, &i)) != 0)
-        return st;
+    ret = alloc_mod(list, &mod);
+    if (ret)
+        return ret;
 
-    (*mods)[i]->mod_type = strdup(attribute);
-    if ((*mods)[i]->mod_type == NULL)
+    mod->mod_type = strdup(attribute);
+    if (mod->mod_type == NULL)
         return ENOMEM;
 
-    (*mods)[i]->mod_op = op;
-    (*mods)[i]->mod_values = calloc (2, sizeof(char *));
-    if (((*mods)[i]->mod_values[0] = format_d(value)) == NULL)
+    mod->mod_op = op;
+    mod->mod_values = calloc(2, sizeof(char *));
+    if (mod->mod_values == NULL)
+        return ENOMEM;
+    mod->mod_values[0] = format_d(value);
+    if (mod->mod_values[0] == NULL)
         return ENOMEM;
     return 0;
 }
@@ -1324,6 +1003,7 @@ krb5_error_code
 krb5_ldap_lock(krb5_context kcontext, int mode)
 {
     krb5_error_code status = KRB5_PLUGIN_OP_NOTSUPP;
+
     k5_setmsg(kcontext, status, "LDAP %s", error_message(status));
     return status;
 }
@@ -1332,13 +1012,14 @@ krb5_error_code
 krb5_ldap_unlock(krb5_context kcontext)
 {
     krb5_error_code status = KRB5_PLUGIN_OP_NOTSUPP;
+
     k5_setmsg(kcontext, status, "LDAP %s", error_message(status));
     return status;
 }
 
 
 /*
- * Get the number of times an object has been referred to in a realm. this is
+ * Get the number of times an object has been referred to in a realm.  This is
  * needed to find out if deleting the attribute will cause dangling links.
  *
  * An LDAP handle may be optionally specified to prevent race condition - there
@@ -1348,16 +1029,14 @@ krb5_error_code
 krb5_ldap_get_reference_count(krb5_context context, char *dn, char *refattr,
                               int *count, LDAP *ld)
 {
-    int                         st = 0, tempst = 0, gothandle = 0;
-    unsigned int                i, ntrees;
-    char                        *refcntattr[2];
-    char                        *filter = NULL;
-    char                        **subtree = NULL, *ptr = NULL;
-    kdb5_dal_handle             *dal_handle = NULL;
-    krb5_ldap_context           *ldap_context = NULL;
-    krb5_ldap_server_handle     *ldap_server_handle = NULL;
-    LDAPMessage                 *result = NULL;
-
+    int n, st, tempst, gothandle = 0;
+    unsigned int i, ntrees = 0;
+    char *refcntattr[2];
+    char *filter = NULL, *corrected = NULL, **subtree = NULL;
+    kdb5_dal_handle *dal_handle = NULL;
+    krb5_ldap_context *ldap_context = NULL;
+    krb5_ldap_server_handle *ldap_server_handle = NULL;
+    LDAPMessage *result = NULL;
 
     if (dn == NULL || refattr == NULL) {
         st = EINVAL;
@@ -1370,38 +1049,35 @@ krb5_ldap_get_reference_count(krb5_context context, char *dn, char *refattr,
         gothandle = 1;
     }
 
-    refcntattr [0] = refattr;
-    refcntattr [1] = NULL;
+    refcntattr[0] = refattr;
+    refcntattr[1] = NULL;
 
-    ptr = ldap_filter_correct (dn);
-    if (ptr == NULL) {
+    corrected = ldap_filter_correct(dn);
+    if (corrected == NULL) {
         st = ENOMEM;
         goto cleanup;
     }
 
-    if (asprintf (&filter, "%s=%s", refattr, ptr) < 0) {
+    if (asprintf(&filter, "%s=%s", refattr, corrected) < 0) {
         filter = NULL;
         st = ENOMEM;
         goto cleanup;
     }
 
-    if ((st = krb5_get_subtree_info(ldap_context, &subtree, &ntrees)) != 0)
+    st = krb5_get_subtree_info(ldap_context, &subtree, &ntrees);
+    if (st)
         goto cleanup;
 
     for (i = 0, *count = 0; i < ntrees; i++) {
-        int n;
-
-        LDAP_SEARCH(subtree[i],
-                    LDAP_SCOPE_SUBTREE,
-                    filter,
-                    refcntattr);
-        n = ldap_count_entries (ld, result);
+        LDAP_SEARCH(subtree[i], LDAP_SCOPE_SUBTREE, filter, refcntattr);
+        n = ldap_count_entries(ld, result);
         if (n == -1) {
             int ret, errcode = 0;
-            ret = ldap_parse_result (ld, result, &errcode, NULL, NULL, NULL, NULL, 0);
+            ret = ldap_parse_result(ld, result, &errcode, NULL, NULL, NULL,
+                                    NULL, 0);
             if (ret != LDAP_SUCCESS)
                 errcode = ret;
-            st = translate_ldap_error (errcode, OP_SEARCH);
+            st = translate_ldap_error(errcode, OP_SEARCH);
             goto cleanup;
         }
 
@@ -1412,81 +1088,67 @@ krb5_ldap_get_reference_count(krb5_context context, char *dn, char *refattr,
     }
 
 cleanup:
-    if (filter != NULL)
-        free (filter);
-
-    if (result != NULL)
-        ldap_msgfree (result);
-
-    if (subtree != NULL) {
-        for (i = 0; i < ntrees; i++)
-            free (subtree[i]);
-        free (subtree);
-    }
-
-    if (ptr != NULL)
-        free (ptr);
-
-    if (gothandle == 1)
+    free(filter);
+    ldap_msgfree(result);
+    for (i = 0; i < ntrees; i++)
+        free(subtree[i]);
+    free(subtree);
+    free(corrected);
+    if (gothandle)
         krb5_ldap_put_handle_to_pool(ldap_context, ldap_server_handle);
-
     return st;
 }
 
-/*
- * For now, policy objects are expected to be directly under the realm
- * container.
- */
+/* Extract a name from policy_dn, which must be directly under the realm
+ * container. */
 krb5_error_code
-krb5_ldap_policydn_to_name(krb5_context context, char *policy_dn, char **name)
+krb5_ldap_policydn_to_name(krb5_context context, const char *policy_dn,
+                           char **name_out)
 {
-    int len1, len2;
-    krb5_error_code             st = 0;
-    kdb5_dal_handle             *dal_handle=NULL;
-    krb5_ldap_context           *ldap_context=NULL;
+    size_t len1, len2, plen;
+    krb5_error_code ret;
+    kdb5_dal_handle *dal_handle;
+    krb5_ldap_context *ldap_context;
+    const char *realmdn;
 
+    *name_out = NULL;
     SETUP_CONTEXT();
 
-    if (ldap_context->lrparams->realmdn == NULL) {
-        st = EINVAL;
-        goto cleanup;
-    }
-
-    len1 = strlen (ldap_context->lrparams->realmdn);
-    len2 = strlen (policy_dn);
-    if (len1 == 0 || len2 == 0 || len1 > len2) {
-        st = EINVAL;
-        goto cleanup;
-    }
+    realmdn = ldap_context->lrparams->realmdn;
+    if (realmdn == NULL)
+        return EINVAL;
 
-    if (strcmp (ldap_context->lrparams->realmdn, policy_dn + (len2 - len1)) != 0) {
-        st = EINVAL;
-        goto cleanup;
-    }
+    /* policyn_dn should be "cn=<policyname>,<realmdn>". */
+    len1 = strlen(realmdn);
+    len2 = strlen(policy_dn);
+    if (len1 == 0 || len2 == 0 || len1 + 1 >= len2)
+        return EINVAL;
+    plen = len2 - len1 - 1;
+    if (policy_dn[plen] != ',' || strcmp(realmdn, policy_dn + plen + 1) != 0)
+        return EINVAL;
 
 #if defined HAVE_LDAP_STR2DN
     {
         char *rdn;
         LDAPDN dn;
-        rdn = strndup(policy_dn, len2 - len1 - 1); /* 1 character for ',' */
 
-        st = ldap_str2dn(rdn, &dn, LDAP_DN_FORMAT_LDAPV3 | LDAP_DN_PEDANTIC);
+        rdn = k5memdup0(policy_dn, plen, &ret);
+        if (rdn == NULL)
+            return ret;
+        ret = ldap_str2dn(rdn, &dn, LDAP_DN_FORMAT_LDAPV3 | LDAP_DN_PEDANTIC);
         free(rdn);
-        if (st != 0) {
-            st = EINVAL;
-            goto cleanup;
-        }
-        if (dn[0] == NULL || dn[1] != NULL)
-            st = EINVAL;
-        else if (strcasecmp (dn[0][0]->la_attr.bv_val, "cn") != 0)
-            st = EINVAL;
-        else {
-            *name = strndup(dn[0][0]->la_value.bv_val, dn[0][0]->la_value.bv_len);
-            if (*name == NULL)
-                st = EINVAL;
+        if (ret)
+            return EINVAL;
+        if (dn[0] == NULL || dn[1] != NULL ||
+            dn[0][0]->la_attr.bv_len != 2 ||
+            strncasecmp(dn[0][0]->la_attr.bv_val, "cn", 2) != 0) {
+            ret = EINVAL;
+        } else {
+            *name_out = k5memdup0(dn[0][0]->la_value.bv_val,
+                                  dn[0][0]->la_value.bv_len, &ret);
         }
-
         ldap_dnfree(dn);
+        return ret;
     }
 #elif defined HAVE_LDAP_EXPLODE_DN
     {
@@ -1494,140 +1156,93 @@ krb5_ldap_policydn_to_name(krb5_context context, char *policy_dn, char **name)
 
         /* 1 = return DN components without type prefix */
         parsed_dn = ldap_explode_dn(policy_dn, 1);
-        if (parsed_dn == NULL) {
-            st = EINVAL;
-        } else {
-            *name = strdup(parsed_dn[0]);
-            if (*name == NULL)
-                st = EINVAL;
-
-            ldap_value_free(parsed_dn);
-        }
+        if (parsed_dn == NULL)
+            return EINVAL;
+        *name_out = strdup(parsed_dn[0]);
+        if (*name_out == NULL)
+            return ENOMEM;
+        ldap_value_free(parsed_dn);
+        return 0;
     }
 #else
-    st = EINVAL;
+    return EINVAL;
 #endif
-
-cleanup:
-    return st;
 }
 
+/* Compute the policy DN for the given policy name. */
 krb5_error_code
 krb5_ldap_name_to_policydn(krb5_context context, char *name, char **policy_dn)
 {
-    int                         len;
-    char                        *ptr = NULL;
-    krb5_error_code             st = 0;
-    kdb5_dal_handle             *dal_handle=NULL;
-    krb5_ldap_context           *ldap_context=NULL;
+    int st;
+    char *corrected;
+    kdb5_dal_handle *dal_handle;
+    krb5_ldap_context *ldap_context;
 
     *policy_dn = NULL;
 
-    /* validate the input parameters */
-    if (name == NULL) {
-        st = EINVAL;
-        goto cleanup;
-    }
-
     /* Used for removing policy reference from an object */
     if (name[0] == '\0') {
-        if ((*policy_dn = strdup ("")) == NULL)
-            st = ENOMEM;
-        goto cleanup;
+        *policy_dn = strdup("");
+        return (*policy_dn == NULL) ? ENOMEM : 0;
     }
 
     SETUP_CONTEXT();
 
-    if (ldap_context->lrparams->realmdn == NULL) {
-        st = EINVAL;
-        goto cleanup;
-    }
-    len = strlen (ldap_context->lrparams->realmdn);
-
-    ptr = ldap_filter_correct (name);
-    if (ptr == NULL) {
-        st = ENOMEM;
-        goto cleanup;
-    }
-    len += strlen (ptr);
+    if (ldap_context->lrparams->realmdn == NULL)
+        return EINVAL;
 
-    len += sizeof ("cn=") + 3;
+    corrected = ldap_filter_correct(name);
+    if (corrected == NULL)
+        return ENOMEM;
 
-    *policy_dn = (char *) malloc (len);
-    if (*policy_dn == NULL) {
-        st = ENOMEM;
-        goto cleanup;
+    st = asprintf(policy_dn, "cn=%s,%s", corrected,
+                  ldap_context->lrparams->realmdn);
+    free(corrected);
+    if (st == -1) {
+        *policy_dn = NULL;
+        return ENOMEM;
     }
+    return 0;
+}
 
-    sprintf (*policy_dn, "cn=%s,%s", ptr, ldap_context->lrparams->realmdn);
-
-cleanup:
-    if (ptr != NULL)
-        free (ptr);
-    return st;
+/* Return true if dn1 is a subtree of dn2. */
+static inline krb5_boolean
+is_subtree(const char *dn1, size_t len1, const char *dn2, size_t len2)
+{
+    return len1 > len2 && dn1[len1 - len2 - 1] == ',' &&
+        strcasecmp(dn1 + (len1 - len2), dn2) == 0;
 }
 
-/* remove overlapping and repeated subtree entries from the list of subtrees */
-static krb5_error_code
-remove_overlapping_subtrees(char **listin, char **listop, int *subtcount,
-                            int sscope)
+/* Remove overlapping and repeated subtree entries from the list of subtrees.
+ * If sscope is not 2 (sub), only remove repeated entries. */
+static void
+remove_overlapping_subtrees(char **list, int *subtcount, int sscope)
 {
-    int     slen=0, k=0, j=0, lendiff=0;
-    int     count = *subtcount;
-    char    **subtree = listop;
-
-    slen = count-1;
-    for (k=0; k<=slen && listin[k]!=NULL ; k++) {
-        for (j=k+1; j<=slen && listin[j]!=NULL ;j++) {
-            lendiff = strlen(listin[k]) - strlen(listin[j]);
-            if (sscope == 2) {
-                if ((lendiff > 0) && (strcasecmp((listin[k])+lendiff, listin[j])==0)) {
-                    if (k != slen) {
-                        free(listin[k]);
-                        listin[k] = listin[slen];
-                        listin[slen] = NULL;
-                    } else {
-                        free(listin[k]);
-                        listin[k] = NULL;
-                    }
-                    slen-=1;
-                    k-=1;
-                    break;
-                } else if ((lendiff < 0) && (strcasecmp((listin[j])+abs(lendiff), listin[k])==0)) {
-                    if (j != slen) {
-                        free(listin[j]);
-                        listin[j] = listin[slen];
-                        listin[slen]=NULL;
-                    } else {
-                        free(listin[j]);
-                        listin[j] = NULL;
-                    }
-                    slen-=1;
-                    j-=1;
-                }
-            }
-            if ((lendiff == 0) && (strcasecmp(listin[j], listin[k])==0)) {
-                if (j != slen) {
-                    free(listin[j]);
-                    listin[j] = listin[slen];
-                    listin[slen]=NULL;
-                } else {
-                    free(listin[j]);
-                    listin[j] = NULL;
-                }
-                slen -=1;
-                j-=1;
+    size_t ilen, jlen;
+    int i, j;
+    int count = *subtcount;
+
+    for (i = 0; i < count && list[i] != NULL; i++) {
+        ilen = strlen(list[i]);
+        for (j = i + 1; j < count && list[j] != NULL; j++) {
+            jlen = strlen(list[j]);
+            /* Remove list[j] if it is identical to list[i] or a subtree of it.
+             * Remove list[i] if it is a subtree of list[j]. */
+            if ((ilen == jlen && strcasecmp(list[j], list[i]) == 0) ||
+                (sscope == 2 && is_subtree(list[j], jlen, list[i], ilen))) {
+                free(list[j]);
+                list[j--] = list[count - 1];
+                list[--count] = NULL;
+            } else if (sscope == 2 &&
+                       is_subtree(list[i], ilen, list[j], jlen)) {
+                free(list[i]);
+                list[i--] = list[count - 1];
+                list[--count] = NULL;
+                break;
             }
         }
     }
-    *subtcount=slen+1;
-    for (k=0; k<*subtcount && listin[k]!=NULL; k++) {
-        subtree[k] = strdup(listin[k]);
-        if (subtree[k] == NULL) {
-            return ENOMEM;
-        }
-    }
-    return 0;
+    *subtcount = count;
 }
 
 /*
@@ -1639,306 +1254,257 @@ populate_krb5_db_entry(krb5_context context, krb5_ldap_context *ldap_context,
                        LDAP *ld, LDAPMessage *ent, krb5_const_principal princ,
                        krb5_db_entry *entry)
 {
-    krb5_error_code st = 0;
-    unsigned int    mask = 0;
-    int             val;
-    krb5_boolean    attr_present = FALSE;
-    char            **values = NULL, *policydn = NULL, *pwdpolicydn = NULL;
-    char            *polname = NULL, *tktpolname = NULL;
-    struct berval   **bvalues = NULL;
-    krb5_tl_data    userinfo_tl_data = {0};
-    char            **link_references = NULL;
-    char *DN = NULL;
-
-    if (princ == NULL) {
-        /* XXX WAF probably should just extract princ from ldap result */
-        st = EINVAL;
+    krb5_error_code ret;
+    unsigned int mask = 0;
+    int val, i, pcount, objtype;
+    krb5_boolean attr_present;
+    krb5_kvno mkvno = 0;
+    krb5_timestamp lastpwdchange, unlock_time;
+    char *policydn = NULL, *pwdpolicydn = NULL, *polname = NULL, *user = NULL;
+    char *tktpolname = NULL, *dn = NULL, **link_references = NULL;
+    char **pnvalues = NULL, **ocvalues = NULL, **a2d2 = NULL;
+    struct berval **ber_key_data = NULL, **ber_tl_data = NULL;
+    krb5_tl_data userinfo_tl_data = { NULL }, **endp, *tl;
+
+    ret = krb5_copy_principal(context, princ, &entry->princ);
+    if (ret)
         goto cleanup;
-    } else {
-        if ((st=krb5_copy_principal(context, princ, &(entry->princ))) != 0)
-            goto cleanup;
-    }
-    /* get the associated directory user information */
-    if ((values = ldap_get_values(ld, ent, "krbprincipalname")) != NULL) {
-        int i, pcount=0, kerberos_principal_object_type=0;
-        char *user;
 
-        if ((st=krb5_unparse_name(context, princ, &user)) != 0)
+    /* get the associated directory user information */
+    pnvalues = ldap_get_values(ld, ent, "krbprincipalname");
+    if (pnvalues != NULL) {
+        ret = krb5_unparse_name(context, princ, &user);
+        if (ret)
             goto cleanup;
 
-        for (i=0; values[i] != NULL; ++i) {
-            if (strcasecmp(values[i], user) == 0) {
-                pcount = ldap_count_values(values);
+        pcount = 0;
+        for (i = 0; pnvalues[i] != NULL; i++) {
+            if (strcasecmp(pnvalues[i], user) == 0) {
+                pcount = ldap_count_values(pnvalues);
                 break;
             }
         }
-        ldap_value_free(values);
-        free(user);
 
-        if ((DN = ldap_get_dn(ld, ent)) == NULL) {
-            ldap_get_option(ld, LDAP_OPT_RESULT_CODE, &st);
-            st = set_ldap_error(context, st, 0);
+        dn = ldap_get_dn(ld, ent);
+        if (dn == NULL) {
+            ldap_get_option(ld, LDAP_OPT_RESULT_CODE, &ret);
+            ret = set_ldap_error(context, ret, 0);
             goto cleanup;
         }
 
-        if ((values=ldap_get_values(ld, ent, "objectclass")) != NULL) {
-            for (i=0; values[i] != NULL; ++i)
-                if (strcasecmp(values[i], "krbprincipal") == 0) {
-                    kerberos_principal_object_type = KDB_STANDALONE_PRINCIPAL_OBJECT;
-                    if ((st=store_tl_data(&userinfo_tl_data, KDB_TL_PRINCTYPE,
-                                          &kerberos_principal_object_type)) != 0)
+        ocvalues = ldap_get_values(ld, ent, "objectclass");
+        if (ocvalues != NULL) {
+            for (i = 0; ocvalues[i] != NULL; i++) {
+                if (strcasecmp(ocvalues[i], "krbprincipal") == 0) {
+                    objtype = KDB_STANDALONE_PRINCIPAL_OBJECT;
+                    ret = store_tl_data(&userinfo_tl_data, KDB_TL_PRINCTYPE,
+                                        &objtype);
+                    if (ret)
                         goto cleanup;
                     break;
                 }
-            ldap_value_free(values);
+            }
         }
 
-        /* add principalcount, DN and principaltype user information to tl_data */
-        if (((st=store_tl_data(&userinfo_tl_data, KDB_TL_PRINCCOUNT, &pcount)) != 0) ||
-            ((st=store_tl_data(&userinfo_tl_data, KDB_TL_USERDN, DN)) != 0))
+        /* Add principalcount, DN and principaltype user information to
+         * tl_data */
+        ret = store_tl_data(&userinfo_tl_data, KDB_TL_PRINCCOUNT, &pcount);
+        if (ret)
+            goto cleanup;
+        ret = store_tl_data(&userinfo_tl_data, KDB_TL_USERDN, dn);
+        if (ret)
             goto cleanup;
     }
 
-    /* read all the kerberos attributes */
-
-    /* KRBLASTSUCCESSFULAUTH */
-    if ((st=krb5_ldap_get_time(ld, ent, "krbLastSuccessfulAuth",
-                               &(entry->last_success), &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbLastSuccessfulAuth", &entry->last_success,
+                   &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE)
+    if (attr_present)
         mask |= KDB_LAST_SUCCESS_ATTR;
 
-    /* KRBLASTFAILEDAUTH */
-    if ((st=krb5_ldap_get_time(ld, ent, "krbLastFailedAuth",
-                               &(entry->last_failed), &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbLastFailedAuth", &entry->last_failed,
+                   &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE)
+    if (attr_present)
         mask |= KDB_LAST_FAILED_ATTR;
 
-    /* KRBLOGINFAILEDCOUNT */
     if (krb5_ldap_get_value(ld, ent, "krbLoginFailedCount", &val) == 0) {
         entry->fail_auth_count = val;
         mask |= KDB_FAIL_AUTH_COUNT_ATTR;
     }
 
-    /* KRBMAXTICKETLIFE */
-    if (krb5_ldap_get_value(ld, ent, "krbmaxticketlife", &(entry->max_life)) == 0)
+    if (krb5_ldap_get_value(ld, ent, "krbmaxticketlife",
+                            &entry->max_life) == 0)
         mask |= KDB_MAX_LIFE_ATTR;
 
-    /* KRBMAXRENEWABLEAGE */
     if (krb5_ldap_get_value(ld, ent, "krbmaxrenewableage",
-                            &(entry->max_renewable_life)) == 0)
+                            &entry->max_renewable_life) == 0)
         mask |= KDB_MAX_RLIFE_ATTR;
 
-    /* KRBTICKETFLAGS */
-    if (krb5_ldap_get_value(ld, ent, "krbticketflags", &(entry->attributes)) == 0)
+    if (krb5_ldap_get_value(ld, ent, "krbticketflags",
+                            &entry->attributes) == 0)
         mask |= KDB_TKT_FLAGS_ATTR;
 
-    /* PRINCIPAL EXPIRATION TIME */
-    if ((st=krb5_ldap_get_time(ld, ent, "krbprincipalexpiration", &(entry->expiration),
-                               &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbprincipalexpiration", &entry->expiration,
+                   &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE)
+    if (attr_present)
         mask |= KDB_PRINC_EXPIRE_TIME_ATTR;
 
-    /* PASSWORD EXPIRATION TIME */
-    if ((st=krb5_ldap_get_time(ld, ent, "krbpasswordexpiration", &(entry->pw_expiration),
-                               &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbpasswordexpiration", &entry->pw_expiration,
+                   &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE)
+    if (attr_present)
         mask |= KDB_PWD_EXPIRE_TIME_ATTR;
 
-    /* KRBPOLICYREFERENCE */
-
-    if ((st=krb5_ldap_get_string(ld, ent, "krbticketpolicyreference", &policydn,
-                                 &attr_present)) != 0)
+    ret = krb5_ldap_get_string(ld, ent, "krbticketpolicyreference", &policydn,
+                               &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE) {
+    if (attr_present) {
         mask |= KDB_POL_REF_ATTR;
-        /* Ensure that the policy is inside the realm container */
-        if ((st = krb5_ldap_policydn_to_name (context, policydn, &tktpolname)) != 0)
+        /* Ensure that the policy is inside the realm container. */
+        ret = krb5_ldap_policydn_to_name(context, policydn, &tktpolname);
+        if (ret)
             goto cleanup;
     }
 
-    /* KRBPWDPOLICYREFERENCE */
-    if ((st=krb5_ldap_get_string(ld, ent, "krbpwdpolicyreference", &pwdpolicydn,
-                                 &attr_present)) != 0)
+    ret = krb5_ldap_get_string(ld, ent, "krbpwdpolicyreference", &pwdpolicydn,
+                               &attr_present);
+    if (ret)
         goto cleanup;
-    if (attr_present == TRUE) {
+    if (attr_present) {
         mask |= KDB_PWD_POL_REF_ATTR;
 
-        /* Ensure that the policy is inside the realm container */
-        if ((st = krb5_ldap_policydn_to_name (context, pwdpolicydn, &polname)) != 0)
+        /* Ensure that the policy is inside the realm container. */
+        ret = krb5_ldap_policydn_to_name(context, pwdpolicydn, &polname);
+        if (ret)
             goto cleanup;
 
-        if ((st = krb5_update_tl_kadm_data(context, entry, polname)) != 0)
+        ret = krb5_update_tl_kadm_data(context, entry, polname);
+        if (ret)
             goto cleanup;
     }
 
-    /* KRBSECRETKEY */
-    if ((bvalues=ldap_get_values_len(ld, ent, "krbprincipalkey")) != NULL) {
-        krb5_kvno mkvno = 0;
-
+    ber_key_data = ldap_get_values_len(ld, ent, "krbprincipalkey");
+    if (ber_key_data != NULL) {
         mask |= KDB_SECRET_KEY_ATTR;
-        if ((st=krb5_decode_krbsecretkey(context, entry, bvalues, &userinfo_tl_data, &mkvno)) != 0)
+        ret = krb5_decode_krbsecretkey(context, entry, ber_key_data,
+                                       &userinfo_tl_data, &mkvno);
+        if (ret)
             goto cleanup;
         if (mkvno != 0) {
-            /* don't add the tl data if mkvno == 0 */
-            if ((st=krb5_dbe_update_mkvno(context, entry, mkvno)) != 0)
+            ret = krb5_dbe_update_mkvno(context, entry, mkvno);
+            if (ret)
                 goto cleanup;
         }
     }
 
-    /* LAST PASSWORD CHANGE */
-    {
-        krb5_timestamp lstpwdchng=0;
-        if ((st=krb5_ldap_get_time(ld, ent, "krbLastPwdChange",
-                                   &lstpwdchng, &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbLastPwdChange", &lastpwdchange, &attr_present);
+    if (ret)
+        goto cleanup;
+    if (attr_present) {
+        ret = krb5_dbe_update_last_pwd_change(context, entry, lastpwdchange);
+        if (ret)
             goto cleanup;
-        if (attr_present == TRUE) {
-            if ((st=krb5_dbe_update_last_pwd_change(context, entry,
-                                                    lstpwdchng)))
-                goto cleanup;
-            mask |= KDB_LAST_PWD_CHANGE_ATTR;
-        }
+        mask |= KDB_LAST_PWD_CHANGE_ATTR;
     }
 
-    /* LAST ADMIN UNLOCK */
-    {
-        krb5_timestamp unlock_time=0;
-        if ((st=krb5_ldap_get_time(ld, ent, "krbLastAdminUnlock",
-                                   &unlock_time, &attr_present)) != 0)
+    ret = get_time(ld, ent, "krbLastAdminUnlock", &unlock_time, &attr_present);
+    if (ret)
+        goto cleanup;
+    if (attr_present) {
+        ret = krb5_dbe_update_last_admin_unlock(context, entry, unlock_time);
+        if (ret)
             goto cleanup;
-        if (attr_present == TRUE) {
-            if ((st=krb5_dbe_update_last_admin_unlock(context, entry,
-                                                      unlock_time)))
-                goto cleanup;
-            mask |= KDB_LAST_ADMIN_UNLOCK_ATTR;
-        }
+        mask |= KDB_LAST_ADMIN_UNLOCK_ATTR;
     }
 
-    /* ALLOWED TO DELEGATE TO */
-    {
-        char **a2d2 = NULL;
-        int i;
-        krb5_tl_data **tlp;
-
-        st = krb5_ldap_get_strings(ld, ent, "krbAllowedToDelegateTo",
-                                   &a2d2, &attr_present);
-        if (st != 0)
-            goto cleanup;
-
-        if (attr_present == TRUE) {
-            for (tlp = &entry->tl_data; *tlp; tlp = &(*tlp)->tl_data_next)
-                ;
-            for (i = 0; a2d2[i] != NULL; i++) {
-                krb5_tl_data *tl = k5alloc(sizeof(*tl), &st);
-                if (st != 0) {
-                    ldap_value_free(a2d2);
-                    goto cleanup;
-                }
-                tl->tl_data_type = KRB5_TL_CONSTRAINED_DELEGATION_ACL;
-                tl->tl_data_length = strlen(a2d2[i]);
-                tl->tl_data_contents = (krb5_octet *)strdup(a2d2[i]);
-                if (tl->tl_data_contents == NULL) {
-                    st = ENOMEM;
-                    ldap_value_free(a2d2);
-                    free(tl);
-                    goto cleanup;
-                }
-                tl->tl_data_next = NULL;
-                *tlp = tl;
-                tlp = &tl->tl_data_next;
+    a2d2 = ldap_get_values(ld, ent, "krbAllowedToDelegateTo");
+    if (a2d2 != NULL) {
+        for (endp = &entry->tl_data; *endp; endp = &(*endp)->tl_data_next);
+        for (i = 0; a2d2[i] != NULL; i++) {
+            tl = k5alloc(sizeof(*tl), &ret);
+            if (tl == NULL)
+                goto cleanup;
+            tl->tl_data_type = KRB5_TL_CONSTRAINED_DELEGATION_ACL;
+            tl->tl_data_length = strlen(a2d2[i]);
+            tl->tl_data_contents = (unsigned char *)strdup(a2d2[i]);
+            if (tl->tl_data_contents == NULL) {
+                ret = ENOMEM;
+                free(tl);
+                goto cleanup;
             }
-            ldap_value_free(a2d2);
+            tl->tl_data_next = NULL;
+            *endp = tl;
+            endp = &tl->tl_data_next;
         }
     }
 
-    /* KRBOBJECTREFERENCES */
-    {
-        int i=0;
-
-        if ((st = krb5_ldap_get_strings(ld, ent, "krbobjectreferences",
-                                        &link_references, &attr_present)) != 0)
-            goto cleanup;
-        if (link_references != NULL) {
-            for (i=0; link_references[i] != NULL; ++i) {
-                if ((st = store_tl_data(&userinfo_tl_data, KDB_TL_LINKDN,
-                                        link_references[i])) != 0)
-                    goto cleanup;
-            }
+    link_references = ldap_get_values(ld, ent, "krbobjectreferences");
+    if (link_references != NULL) {
+        for (i = 0; link_references[i] != NULL; i++) {
+            ret = store_tl_data(&userinfo_tl_data, KDB_TL_LINKDN,
+                                link_references[i]);
+            if (ret)
+                goto cleanup;
         }
     }
 
-    /* Set tl_data */
-    {
-        int i;
-        struct berval **ber_tl_data = NULL;
-        krb5_tl_data *ptr = NULL;
-
-        if ((ber_tl_data = ldap_get_values_len (ld, ent, "krbExtraData")) != NULL) {
-            for (i = 0; ber_tl_data[i] != NULL; i++) {
-                if ((st = berval2tl_data (ber_tl_data[i] , &ptr)) != 0)
-                    break;
-                st = krb5_dbe_update_tl_data(context, entry, ptr);
-                free(ptr->tl_data_contents);
-                free(ptr);
-                if (st != 0)
-                    break;
-            }
-            ldap_value_free_len (ber_tl_data);
-            if (st != 0)
+    ber_tl_data = ldap_get_values_len(ld, ent, "krbExtraData");
+    if (ber_tl_data != NULL) {
+        for (i = 0; ber_tl_data[i] != NULL; i++) {
+            ret = berval2tl_data(ber_tl_data[i], &tl);
+            if (ret)
+                goto cleanup;
+            ret = krb5_dbe_update_tl_data(context, entry, tl);
+            free(tl->tl_data_contents);
+            free(tl);
+            if (ret)
                 goto cleanup;
-            mask |= KDB_EXTRA_DATA_ATTR;
         }
+        mask |= KDB_EXTRA_DATA_ATTR;
     }
 
-    /* update the mask of attributes present on the directory object to the tl_data */
-    if ((st=store_tl_data(&userinfo_tl_data, KDB_TL_MASK, &mask)) != 0)
+    /* Update the mask of attributes present on the directory object to the
+     * tl_data. */
+    ret = store_tl_data(&userinfo_tl_data, KDB_TL_MASK, &mask);
+    if (ret)
         goto cleanup;
-    if ((st=krb5_dbe_update_tl_data(context, entry, &userinfo_tl_data)) != 0)
+    ret = krb5_dbe_update_tl_data(context, entry, &userinfo_tl_data);
+    if (ret)
         goto cleanup;
 
-    if ((st=krb5_read_tkt_policy (context, ldap_context, entry, tktpolname)) !=0)
+    ret = krb5_read_tkt_policy(context, ldap_context, entry, tktpolname);
+    if (ret)
         goto cleanup;
 
-    /* XXX so krb5_encode_princ_contents() will be happy */
+    /* For compatibility with DB2 principals. */
     entry->len = KRB5_KDB_V1_BASE_LENGTH;
 
 cleanup:
-
-    if (DN != NULL)
-        ldap_memfree(DN);
-
-    if (userinfo_tl_data.tl_data_contents != NULL)
-        free(userinfo_tl_data.tl_data_contents);
-
-    if (pwdpolicydn != NULL)
-        free(pwdpolicydn);
-
-    if (polname != NULL)
-        free(polname);
-
-    if (tktpolname != NULL)
-        free (tktpolname);
-
-    if (policydn != NULL)
-        free(policydn);
-
-    if (link_references) {
-        int i;
-        for (i=0; link_references[i] != NULL; ++i)
-            free (link_references[i]);
-        free (link_references);
-    }
-
-    return (st);
+    ldap_memfree(dn);
+    ldap_value_free_len(ber_key_data);
+    ldap_value_free_len(ber_tl_data);
+    ldap_value_free(pnvalues);
+    ldap_value_free(ocvalues);
+    ldap_value_free(link_references);
+    ldap_value_free(a2d2);
+    free(userinfo_tl_data.tl_data_contents);
+    free(pwdpolicydn);
+    free(polname);
+    free(tktpolname);
+    free(policydn);
+    krb5_free_unparsed_name(context, user);
+    return ret;
 }
 
-/*
- * Solaris libldap does not provide the following functions which are in
- * OpenLDAP.
- */
+/* Solaris libldap does not provide the following functions which are in
+ * OpenLDAP. */
 #ifndef HAVE_LDAP_INITIALIZE
 int
 ldap_initialize(LDAP **ldp, char *url)
@@ -1947,31 +1513,26 @@ ldap_initialize(LDAP **ldp, char *url)
     LDAP *ld = NULL;
     LDAPURLDesc *ludp = NULL;
 
-    /* For now, we don't use any DN that may be provided.  And on
-       Solaris (based on Mozilla's LDAP client code), we need the
-       _nodn form to parse "ldap://host" without a trailing slash.
-
-       Also, this version won't handle an input string which contains
-       multiple URLs, unlike the OpenLDAP ldap_initialize.  See
-       https://bugzilla.mozilla.org/show_bug.cgi?id=353336#c1 .  */
+    /*
+     * For now, we don't use any DN that may be provided.  And on Solaris
+     * (based on Mozilla's LDAP client code), we need the _nodn form to parse
+     * "ldap://host" without a trailing slash.
+     *
+     * Also, this version won't handle an input string which contains multiple
+     * URLs, unlike the OpenLDAP ldap_initialize.  See
+     * https://bugzilla.mozilla.org/show_bug.cgi?id=353336#c1 .
+     */
 #ifdef HAVE_LDAP_URL_PARSE_NODN
     rc = ldap_url_parse_nodn(url, &ludp);
 #else
     rc = ldap_url_parse(url, &ludp);
 #endif
     if (rc == 0) {
-
         ld = ldap_init(ludp->lud_host, ludp->lud_port);
-        if (ld != NULL) {
+        if (ld != NULL)
             *ldp = ld;
-#if 0
-            printf("lud_host %s lud_port %d\n", ludp->lud_host,
-                   ludp->lud_port);
-#endif
-        }
         else
             rc = KRB5_KDB_ACCESS_ERROR;
-
         ldap_free_urldesc(ludp);
     }
     return rc;
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h
index c3bb6c8..fb4f3ce 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.h
@@ -38,7 +38,7 @@
 
 /* misc functions */
 
-krb5_error_code
+krb5_boolean
 is_principal_in_realm(krb5_ldap_context *, krb5_const_principal);
 
 krb5_error_code
@@ -60,18 +60,9 @@ krb5_error_code
 krb5_get_userdn(krb5_context, krb5_db_entry *, char **);
 
 krb5_error_code
-krb5_get_containerdn(krb5_context, krb5_db_entry *, char **);
-
-krb5_error_code
 store_tl_data(krb5_tl_data *, int, void *);
 
 krb5_error_code
-decode_tl_data(krb5_tl_data *, int, void **);
-
-krb5_error_code
-is_principal_in_realm(krb5_ldap_context *, krb5_const_principal);
-
-krb5_error_code
 krb5_get_subtree_info(krb5_ldap_context *, char ***, unsigned int *);
 
 krb5_error_code
@@ -80,7 +71,7 @@ krb5_ldap_parse_db_params(krb5_context, char **);
 krb5_error_code
 krb5_ldap_read_server_params(krb5_context , char *, int);
 
-krb5_error_code
+void
 krb5_ldap_free_server_params(krb5_ldap_context *);
 
 krb5_error_code
@@ -93,15 +84,6 @@ krb5_error_code
 krb5_ldap_get_string(LDAP *, LDAPMessage *, char *, char **, krb5_boolean *);
 
 krb5_error_code
-krb5_ldap_get_strings(LDAP *, LDAPMessage *, char *, char ***, krb5_boolean *);
-
-krb5_error_code
-krb5_ldap_get_time(LDAP *, LDAPMessage *, char *, krb5_timestamp *, krb5_boolean *);
-
-krb5_error_code
-krb5_add_member(LDAPMod ***, int *);
-
-krb5_error_code
 krb5_add_str_mem_ldap_mod(LDAPMod  ***, char *, int, char **);
 
 krb5_error_code
@@ -117,7 +99,7 @@ krb5_error_code
 krb5_ldap_get_reference_count (krb5_context, char *, char *, int *, LDAP *);
 
 krb5_error_code
-krb5_ldap_policydn_to_name (krb5_context, char *, char **);
+krb5_ldap_policydn_to_name (krb5_context, const char *, char **);
 
 krb5_error_code
 krb5_ldap_name_to_policydn (krb5_context, char *, char **);
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c
index 44bf339..3f3efdc 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c
@@ -189,7 +189,7 @@ krb5_ldap_iterate(krb5_context context, char *match_expr,
                         continue;
                     if (krb5_parse_name(context, princ_name, &principal) != 0)
                         continue;
-                    if (is_principal_in_realm(ldap_context, principal) == 0) {
+                    if (is_principal_in_realm(ldap_context, principal)) {
                         if ((st = populate_krb5_db_entry(context, ldap_context, ld, ent, principal,
                                                          &entry)) != 0)
                             goto cleanup;
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index 06881f0..ce851ea 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -103,7 +103,7 @@ krb5_ldap_get_principal(krb5_context context, krb5_const_principal searchfor,
 
     CHECK_LDAP_HANDLE(ldap_context);
 
-    if (is_principal_in_realm(ldap_context, searchfor) != 0) {
+    if (!is_principal_in_realm(ldap_context, searchfor)) {
         st = KRB5_KDB_NOENTRY;
         k5_setmsg(context, st, _("Principal does not belong to realm"));
         goto cleanup;
@@ -536,7 +536,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
     /* get ldap handle */
     GET_HANDLE();
 
-    if (is_principal_in_realm(ldap_context, entry->princ) != 0) {
+    if (!is_principal_in_realm(ldap_context, entry->princ)) {
         st = EINVAL;
         k5_setmsg(context, st,
                   _("Principal does not belong to the default realm"));
@@ -1411,7 +1411,6 @@ krb5_decode_krbsecretkey(krb5_context context, krb5_db_entry *entries,
     entries->key_data = key_data;
 
 cleanup:
-    ldap_value_free_len(bvalues);
     free (user);
     return st;
 }
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.c
index 94ad9c7..87a2118 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.c
@@ -35,8 +35,7 @@
 
 /* Decode a password of the form {HEX}<hexstring>. */
 static krb5_error_code
-dec_password(krb5_context context, const char *str,
-             unsigned char **password_out)
+dec_password(krb5_context context, const char *str, char **password_out)
 {
     size_t len;
     const unsigned char *p;
@@ -72,85 +71,62 @@ dec_password(krb5_context context, const char *str,
     }
     *q = '\0';
 
-    *password_out = password;
+    *password_out = (char *)password;
     return 0;
 }
 
 krb5_error_code
-krb5_ldap_readpassword(krb5_context context, krb5_ldap_context *ldap_context,
-                       unsigned char **password)
+krb5_ldap_readpassword(krb5_context context, const char *filename,
+                       const char *name, char **password_out)
 {
-    int                         entryfound=0;
-    krb5_error_code             st=0;
-    char                        line[RECORDLEN]="0", *start=NULL, *file=NULL;
-    FILE                        *fptr=NULL;
-
-    *password = NULL;
-
-    if (ldap_context->service_password_file)
-        file = ldap_context->service_password_file;
-
-    fptr = fopen(file, "r");
-    if (fptr == NULL) {
-        st = errno;
-        k5_setmsg(context, st, _("Cannot open LDAP password file '%s': %s"),
-                  file, error_message(st));
-        goto rp_exit;
+    krb5_error_code ret;
+    char line[RECORDLEN], *end;
+    const char *start, *sep, *val = NULL;
+    int namelen = strlen(name);
+    FILE *fp;
+
+    *password_out = NULL;
+
+    fp = fopen(filename, "r");
+    if (fp == NULL) {
+        ret = errno;
+        k5_setmsg(context, ret, _("Cannot open LDAP password file '%s': %s"),
+                  filename, error_message(ret));
+        return ret;
     }
-    set_cloexec_file(fptr);
+    set_cloexec_file(fp);
 
-    /* get the record from the file */
-    while (fgets(line, RECORDLEN, fptr)!= NULL) {
-        char tmp[RECORDLEN];
+    while (fgets(line, RECORDLEN, fp) != NULL) {
+        /* Remove trailing newline. */
+        end = line + strlen(line);
+        if (end > line && end[-1] == '\n')
+            end[-1] = '\0';
 
-        tmp[0] = '\0';
-        /* Handle leading white-spaces */
+        /* Skip past leading whitespace. */
         for (start = line; isspace(*start); ++start);
 
-        /* Handle comment lines */
+        /* Ignore comment lines */
         if (*start == '!' || *start == '#')
             continue;
-        sscanf(line, "%*[ \t]%[^#]", tmp);
-        if (tmp[0] == '\0')
-            sscanf(line, "%[^#]", tmp);
-        if (strcasecmp(tmp, ldap_context->bind_dn) == 0) {
-            entryfound = 1; /* service_dn record found !!! */
+
+        sep = strchr(start, '#');
+        if (sep != NULL && sep - start == namelen &&
+            strncasecmp(start, name, namelen) == 0) {
+            val = sep + 1;
             break;
         }
     }
-    fclose (fptr);
+    fclose(fp);
 
-    if (entryfound == 0)  {
-        st = KRB5_KDB_SERVER_INTERNAL_ERR;
-        k5_setmsg(context, st,
+    if (val == NULL) {
+        k5_setmsg(context, KRB5_KDB_SERVER_INTERNAL_ERR,
                   _("Bind DN entry '%s' missing in LDAP password file '%s'"),
-                  ldap_context->bind_dn, file);
-        goto rp_exit;
+                  name, filename);
+        return KRB5_KDB_SERVER_INTERNAL_ERR;
     }
-    /* replace the \n with \0 */
-    start = strchr(line, '\n');
-    if (start)
-        *start = '\0';
-
-    start = strchr(line, '#');
-    if (start == NULL) {
-        /* password field missing */
-        st = KRB5_KDB_SERVER_INTERNAL_ERR;
-        k5_setmsg(context, st, _("Stash file entry corrupt"));
-        goto rp_exit;
-    }
-    ++ start;
 
     /* Extract the plain password information. */
-    st = dec_password(context, start, password);
-
-rp_exit:
-    if (st) {
-        if (*password)
-            free (*password);
-        *password = NULL;
-    }
-    return st;
+    return dec_password(context, val, password_out);
 }
 
 /* Encodes a sequence of bytes in hexadecimal */
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.h b/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.h
index 31511fc..dbf6244 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.h
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_service_stash.h
@@ -34,7 +34,8 @@
 #define RECORDLEN 1024
 
 krb5_error_code
-krb5_ldap_readpassword(krb5_context, krb5_ldap_context *, unsigned char **);
+krb5_ldap_readpassword(krb5_context context, const char *filename,
+                       const char *name, char **password_out);
 
 int
 tohex(krb5_data, krb5_data *);