Assume mutex locking cannot fail

Locking and unlocking a non-recursive mutex is a simple memory
operation and should not fail on any reasonable platform with correct
usage.  A pthread mutex can return EDEADLK on lock or EPERM on unlock,
or EINVAL if the mutex is uninitialized, but all of these conditions
would reflect serious bugs in the calling code.

Change the k5_mutex_lock and k5_mutex_unlock wrappers to return void
and adjust all call sites.  Propagate this change through
k5_cc_mutex_lock and k5_cc_mutex_unlock as well.

4 files changed, 13 insertions, 31 deletions

diff --git a/src/plugins/kdb/db2/db2_exp.c b/src/plugins/kdb/db2/db2_exp.c
index 4ae3e31d5f..c2bad73787 100644
--- a/src/plugins/kdb/db2/db2_exp.c
+++ b/src/plugins/kdb/db2/db2_exp.c
@@ -58,12 +58,11 @@
 
 k5_mutex_t *krb5_db2_mutex;
 
-#define WRAP(NAME,TYPE,ARGLIST,ARGNAMES,ERROR_RESULT)   \
+#define WRAP(NAME,TYPE,ARGLIST,ARGNAMES)                \
     static TYPE wrap_##NAME ARGLIST                     \
     {                                                   \
         TYPE result;                                    \
-        int code = k5_mutex_lock (krb5_db2_mutex);      \
-        if (code) { return ERROR_RESULT; }              \
+        k5_mutex_lock (krb5_db2_mutex);                 \
         result = NAME ARGNAMES;                         \
         k5_mutex_unlock (krb5_db2_mutex);               \
         return result;                                  \
@@ -77,8 +76,7 @@ k5_mutex_t *krb5_db2_mutex;
 #define WRAP_VOID(NAME,ARGLIST,ARGNAMES)                \
     static void wrap_##NAME ARGLIST                     \
     {                                                   \
-        int code = k5_mutex_lock (krb5_db2_mutex);      \
-        if (code) { return; }                           \
+        k5_mutex_lock (krb5_db2_mutex);                 \
         NAME ARGNAMES;                                  \
         k5_mutex_unlock (krb5_db2_mutex);               \
     }                                                   \
@@ -86,7 +84,7 @@ k5_mutex_t *krb5_db2_mutex;
     static void wrap_##NAME ()
 
 #define WRAP_K(NAME,ARGLIST,ARGNAMES)                   \
-    WRAP(NAME,krb5_error_code,ARGLIST,ARGNAMES,code)
+    WRAP(NAME,krb5_error_code,ARGLIST,ARGNAMES)
 
 WRAP_K (krb5_db2_open,
         ( krb5_context kcontext,
diff --git a/src/plugins/kdb/db2/kdb_db2.c b/src/plugins/kdb/db2/kdb_db2.c
index 62fbef748b..b0cd2a5e97 100644
--- a/src/plugins/kdb/db2/kdb_db2.c
+++ b/src/plugins/kdb/db2/kdb_db2.c
@@ -934,7 +934,7 @@ ctx_iterate(krb5_context context, krb5_db2_context *dbc,
     DBT key, contents;
     krb5_data contdata;
     krb5_db_entry *entry;
-    krb5_error_code retval, retval2;
+    krb5_error_code retval;
     int dbret;
 
     retval = ctx_lock(context, dbc, KRB5_LOCKMODE_SHARED);
@@ -948,21 +948,12 @@ ctx_iterate(krb5_context context, krb5_db2_context *dbc,
         retval = krb5_decode_princ_entry(context, &contdata, &entry);
         if (retval)
             break;
-        retval = k5_mutex_unlock(krb5_db2_mutex);
-        if (retval)
-            break;
+        k5_mutex_unlock(krb5_db2_mutex);
         retval = (*func)(func_arg, entry);
         krb5_dbe_free(context, entry);
-        retval2 = k5_mutex_lock(krb5_db2_mutex);
-        /* Note: If re-locking fails, the wrapper in db2_exp.c will
-           still try to unlock it again.  That would be a bug.  Fix
-           when integrating the locking better.  */
+        k5_mutex_lock(krb5_db2_mutex);
         if (retval)
             break;
-        if (retval2) {
-            retval = retval2;
-            break;
-        }
         dbret = dbc->db->seq(dbc->db, &key, &contents, R_NEXT);
     }
     switch (dbret) {
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 6f53640604..02fbadc421 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap_conn.c
@@ -162,9 +162,7 @@ krb5_ldap_db_init(krb5_context context, krb5_ldap_context *ldap_context)
     ldap_set_option(NULL, LDAP_X_OPT_CONNECT_TIMEOUT, &local_timelimit);
 #endif
 
-    st = HNDL_LOCK(ldap_context);
-    if (st)
-        return st;
+    HNDL_LOCK(ldap_context);
     while (ldap_context->server_info_list[cnt] != NULL) {
         krb5_ldap_server_info *server_info=NULL;
 
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
index 2261e6322e..2188b2d319 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_handle.c
@@ -217,9 +217,7 @@ krb5_ldap_request_handle_from_pool(krb5_ldap_context *ldap_context,
 
     *ldap_server_handle = NULL;
 
-    st = HNDL_LOCK(ldap_context);
-    if (st)
-        return st;
+    HNDL_LOCK(ldap_context);
     if (((*ldap_server_handle)=krb5_get_ldap_handle(ldap_context)) == NULL)
         (*ldap_server_handle)=krb5_retry_get_ldap_handle(ldap_context, &st);
     HNDL_UNLOCK(ldap_context);
@@ -238,9 +236,7 @@ krb5_ldap_request_next_handle_from_pool(krb5_ldap_context *ldap_context,
 {
     krb5_error_code            st=0;
 
-    st = HNDL_LOCK(ldap_context);
-    if (st)
-        return st;
+    HNDL_LOCK(ldap_context);
     (*ldap_server_handle)->server_info->server_status = OFF;
     time(&(*ldap_server_handle)->server_info->downtime);
     krb5_put_ldap_handle(*ldap_server_handle);
@@ -261,10 +257,9 @@ krb5_ldap_put_handle_to_pool(krb5_ldap_context *ldap_context,
                              krb5_ldap_server_handle *ldap_server_handle)
 {
     if (ldap_server_handle != NULL) {
-        if (HNDL_LOCK(ldap_context) == 0) {
-            krb5_put_ldap_handle(ldap_server_handle);
-            HNDL_UNLOCK(ldap_context);
-        }
+        HNDL_LOCK(ldap_context);
+        krb5_put_ldap_handle(ldap_server_handle);
+        HNDL_UNLOCK(ldap_context);
     }
     return;
 }