From ae3f1f0589fc0dd45ef5fee4f8bcd244903681ae Mon Sep 17 00:00:00 2001
From: Russell Jancewicz <russell.jancewicz@gmail.com>
Date: Mon, 27 Apr 2015 11:33:49 -0400
Subject: [PATCH] restructure kadmin.c to correctly handle errors and avoid
 memory leaks

cleanup errors on kadmin object

new error model for principal objects

remove remaining instances of old error handling.

restore direct error numbers (lower errno values are possible); cleanup memory leaks

track internal lock state; only unlock on dealloc if lock is held.

krb5_init leaks memory - same result running kadmin.local: unsupported command

when resolving the client_name from a credential cache we are responsible for freeing allocated memory

valgrind supp file for python leaks.

restructure assignment so no memory is allocated until argumets are parsed.
---
 PyKAdminErrors.c                | 182 +++++++++++++-----------------
 PyKAdminErrors.h                |  12 +-
 PyKAdminIterator.c              |  30 ++---
 PyKAdminObject.c                | 123 ++++++++++++++++----
 PyKAdminObject.h                |   2 +
 PyKAdminPrincipalObject.c       | 117 ++++++++++++-------
 kadmin.c                        | 176 +++++++++++++++++++++--------
 pykadmin.h                      |   4 +
 test/bootstrap_kdb.py           |   2 +-
 test/krb5.supp                  |  10 ++
 test/unittests.py               |  42 +++++--
 test/valgrind-python-leaks.supp | 193 ++++++++++++++++++++++++++++++++
 12 files changed, 650 insertions(+), 243 deletions(-)
 create mode 100644 test/krb5.supp
 create mode 100644 test/valgrind-python-leaks.supp

diff --git a/PyKAdminErrors.c b/PyKAdminErrors.c
index a7940a7..33a97b6 100644
--- a/PyKAdminErrors.c
+++ b/PyKAdminErrors.c
@@ -4,10 +4,13 @@
 static PyObject *_pykadmin_error_base;
 static PyObject *_pykadmin_errors;
 
-//static PyObject *_pykadmin_kadm_errors; 
+//static PyObject *_pykadmin_kadm_errors;
 //static PyObject *_pykadmin_krb5_errors;
 
-/* 
+/*
+
+    k5-int.h
+
     Initialize Error Classes
 
     kadmin.KAdminError(exceptions.Exception)
@@ -24,7 +27,7 @@ int PyKAdminError_init_kdb(PyObject *module, PyObject *base);
 
 
 PyObject *PyKAdminError_init(PyObject *module) {
-    
+
     static const char kBASE_ERROR[] = "KAdminError";
     static const char kKADM_ERROR[] = "AdminError";
     static const char kKRB5_ERROR[] = "KerberosError";
@@ -66,19 +69,16 @@ PyObject *PyKAdminError_init(PyObject *module) {
             PyKAdminError_kdb = PyErr_NewException(cname, _pykadmin_error_base, NULL);
 
             if (PyKAdminError_kadm) {
-                //Py_INCREF(PyKAdminError_kadm);
                 PyModule_AddObject(module, kKADM_ERROR, PyKAdminError_kadm);
                 PyKAdminError_init_kadm(module, PyKAdminError_kadm);
             }
 
              if (PyKAdminError_krb5) {
-                //Py_INCREF(PyKAdminError_krb5);
                 PyModule_AddObject(module, kKRB5_ERROR, PyKAdminError_krb5);
                 PyKAdminError_init_krb5(module, PyKAdminError_krb5);
             }
 
              if (PyKAdminError_kdb) {
-                //Py_INCREF(PyKAdminError_krb5);
                 PyModule_AddObject(module, kKDB_ERROR, PyKAdminError_kdb);
                 PyKAdminError_init_kdb(module, PyKAdminError_kdb);
             }
@@ -110,8 +110,8 @@ static void _PyKAdminError_raise_exception(PyObject *storage, PyObject *error, c
             error_tuple = PyDict_GetItem(storage, error);
 
             if (error_tuple && (PyTuple_GET_SIZE(error_tuple) == 2)) {
-                error_object = PyTuple_GetItem(error_tuple, 0); 
-                error_string = PyTuple_GetItem(error_tuple, 1); 
+                error_object = PyTuple_GetItem(error_tuple, 0);
+                error_string = PyTuple_GetItem(error_tuple, 1);
             }
 
         }
@@ -130,39 +130,47 @@ static void _PyKAdminError_raise_exception(PyObject *storage, PyObject *error, c
 
     }
 
+    Py_DECREF(error);
     Py_XDECREF(error_dict);
 
 }
 
 void PyKAdminError_raise_error(long value, char *caller) {
+
     PyObject *error  = PyLong_FromLong((long)value);
     _PyKAdminError_raise_exception(_pykadmin_errors, error, caller);
 }
 
 /*
+void PyKAdminError_raise_kadm_error(kadm5_ret_t value, char *caller) {
 
-void PyKAdminError_raise_error(krb5_error_code code, char *caller) {
-    PyObject *error  = PyLong_FromLong((long)code);
+    PyObject *error  = PyLong_FromLong((long)(value));
     _PyKAdminError_raise_exception(_pykadmin_errors, error, caller);
 }
 
-void PyKAdminError_raise_error(kadm5_ret_t retval, char *caller) {
-    PyObject *error  = PyLong_FromLong((long)retval);
+void PyKAdminError_raise_krb5_error(krb5_error_code value, char *caller) {
+
+    PyObject *error  = PyLong_FromLong((long)(value));
     _PyKAdminError_raise_exception(_pykadmin_errors, error, caller);
 }
-*/
 
+void PyKAdminError_raise_kdb_error(krb5_error_code value, char *caller) {
+
+    PyObject *error  = PyLong_FromLong((long)(value));
+    _PyKAdminError_raise_exception(_pykadmin_errors, error, caller);
+
+*/
 
 static int PyKAdminErrors_new_exception(PyObject *module, PyObject *base, PyObject *storage, PyObject *error, char *name, char *cname, char *message) {
 
-    int result = 0; 
+    int result = 0;
     PyObject *exception = NULL;
     PyObject *tuple     = NULL;
 
     if (module && base && storage && error && name && cname && message) {
 
         exception = PyErr_NewException(cname, base, NULL);
-        
+
         if (exception) {
 
             result = PyModule_AddObject(module, name, exception);
@@ -181,8 +189,8 @@ static int PyKAdminErrors_new_exception(PyObject *module, PyObject *base, PyObje
 }
 
 static int _pykadminerror_error_insert(PyObject *module, PyObject *base, krb5_error_code code, char *name, char *message) {
-    
-    int result       = 0; 
+
+    int result       = 0;
     char *cname      = NULL;
     size_t length    = strlen(kMODULE_NAME) + strlen(name) + 0xF;
     PyObject *error  = PyLong_FromLong((long)code);
@@ -191,7 +199,7 @@ static int _pykadminerror_error_insert(PyObject *module, PyObject *base, krb5_er
 
         cname = malloc(length);
 
-        if (cname) { 
+        if (cname) {
             snprintf(cname, length, "%s.%s", kMODULE_NAME, name);
             result = PyKAdminErrors_new_exception(module, base, _pykadmin_errors, error, name, cname, message);
             free(cname);
@@ -201,90 +209,46 @@ static int _pykadminerror_error_insert(PyObject *module, PyObject *base, krb5_er
     return result;
 }
 
-/*
-static int _pykadminerror_error_insert(PyObject *module, PyObject *base, krb5_error_code code, char *name, char *message) {
-    
-    int result       = 0; 
-    char *cname      = NULL;
-    size_t length    = strlen(kMODULE_NAME) + strlen(name) + 0xF;
-    PyObject *error  = PyLong_FromLong(code);
-
-    if (error) {
-
-        cname = malloc(length);
-
-        if (cname) { 
-            snprintf(cname, length, "%s.%s", kMODULE_NAME, name);
-            result = PyKAdminErrors_new_exception(module, base, _pykadmin_krb5_errors, error, name, cname, message);
-            free(cname);
-        }
-    }
-
-    return result;
-}
-
-
-static int _pykadminerror_error_insert(PyObject *module, PyObject *base, kadm5_ret_t retval, char *name, char *message) {
-    
-    int result       = 0; 
-    char *cname      = NULL;
-    size_t length    = strlen(kMODULE_NAME) + strlen(name) + 0xF;
-    PyObject *error  = PyLong_FromUnsignedLong(retval);
-
-    if (error) {
-
-        cname = malloc(length);
-
-        if (cname) { 
-            snprintf(cname, length, "%s.%s", kMODULE_NAME, name);
-            result = PyKAdminErrors_new_exception(module, base, _pykadmin_kadm_errors, error, name, cname, message);
-            free(cname);
-        }
-    }
-
-    return result;
-}
-*/
-
-
 int PyKAdminError_init_krb5(PyObject *module, PyObject *base) {
 
-    int result = 0; 
+    int result = 0;
     //_pykadmin_krb5_errors = PyDict_New();
 
     if (_pykadmin_errors) {
 
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NONE,                                 "KDCNoneError",               "No error");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NAME_EXP,                             "KDCClientExpiredError",      "Client's entry in database has expired");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_EXP,                          "KDCServerExpireError",       "Server's entry in database has expired");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_BAD_PVNO,                             "KDCProtocolVersionError",    "Requested protocol version not supported");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_C_OLD_MAST_KVNO,                      "KDCClientOldMasterKeyError", "Client's key is encrypted in an old master key");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_S_OLD_MAST_KVNO,                      "KDCServerOldMasterKeyError", "Server's key is encrypted in an old master key");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN,                  "KDCClientNotFoundError",     "Client not found in Kerberos database");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN,                  "KDCServerNotFoundError",     "Server not found in Kerberos database");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PRINCIPAL_NOT_UNIQUE,                 "KDCPrincipalUniqueError",    "Principal has multiple entries in Kerberos database");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NULL_KEY,                             "KDCNullKeyError",            "Client or server has a null key");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CANNOT_POSTDATE,                      "KDCCannotPostdateError",     "Ticket is ineligible for postdating");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NEVER_VALID,                          "KDCNeverValidError",         "Requested effective lifetime is negative or too short");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_POLICY,                               "KDCPolicyError",             "KDC policy rejects request");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_BADOPTION,                            "KDCOptionError",             "KDC can't fulfill requested option");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_ETYPE_NOSUPP,                         "KDCEncryptionSupportError",  "KDC has no support for encryption type");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SUMTYPE_NOSUPP,                       "KDCChecksumSupportError",    "KDC has no support for checksum type");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PADATA_TYPE_NOSUPP,                   "KDCPADataSupportError",      "KDC has no support for padata type");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_TRTYPE_NOSUPP,                        "KDCTypeSupportError",        "KDC has no support for transited type");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CLIENT_REVOKED,                       "KDCClientRevokedError",      "Clients credentials have been revoked");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_REVOKED,                      "KDCServerRevokedError",      "Credentials for server have been revoked");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_TGT_REVOKED,                          "KDCTGTRevokedError",         "TGT has been revoked");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CLIENT_NOTYET,                        "KDCClientNotYetValidError",  "Client not yet valid - try again later");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_NOTYET,                       "KDCServerNotYetValidError",  "Server not yet valid - try again later");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_KEY_EXP,                              "KDCPasswordExpiredError",    "Password has expired");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PREAUTH_FAILED,                       "KDCPreauthFailedError",      "Preauthentication failed");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PREAUTH_REQUIRED,                     "KDCPreauthRequiredError",    "Additional pre-authentication required");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVER_NOMATCH,                       "KDCServerMatchError",        "Requested server and ticket don't match");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_MUST_USE_USER2USER,                   "KDCRequireUser2UserError",   "Server principal valid for user2user only");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PATH_NOT_ACCEPTED,                    "KDCPathError",               "KDC policy rejects transited path");
-        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SVC_UNAVAILABLE,                      "KDCServiceUnavailableError", "A service is not available that is required to process the request");
-        
+        /* Errors defined by /usr/include/krb5/krb5.h */
+
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NONE,                                 "KRB5KDCNoneError",               "No error");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NAME_EXP,                             "KRB5KDCClientExpiredError",      "Client's entry in database has expired");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_EXP,                          "KRB5KDCServerExpireError",       "Server's entry in database has expired");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_BAD_PVNO,                             "KRB5KDCProtocolVersionError",    "Requested protocol version not supported");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_C_OLD_MAST_KVNO,                      "KRB5KDCClientOldMasterKeyError", "Client's key is encrypted in an old master key");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_S_OLD_MAST_KVNO,                      "KRB5KDCServerOldMasterKeyError", "Server's key is encrypted in an old master key");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN,                  "KRB5KDCClientNotFoundError",     "Client not found in Kerberos database");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN,                  "KRB5KDCServerNotFoundError",     "Server not found in Kerberos database");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PRINCIPAL_NOT_UNIQUE,                 "KRB5KDCPrincipalUniqueError",    "Principal has multiple entries in Kerberos database");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NULL_KEY,                             "KRB5KDCNullKeyError",            "Client or server has a null key");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CANNOT_POSTDATE,                      "KRB5KDCCannotPostdateError",     "Ticket is ineligible for postdating");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_NEVER_VALID,                          "KRB5KDCNeverValidError",         "Requested effective lifetime is negative or too short");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_POLICY,                               "KRB5KDCPolicyError",             "KDC policy rejects request");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_BADOPTION,                            "KRB5KDCOptionError",             "KDC can't fulfill requested option");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_ETYPE_NOSUPP,                         "KRB5KDCEncryptionSupportError",  "KDC has no support for encryption type");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SUMTYPE_NOSUPP,                       "KRB5KDCChecksumSupportError",    "KDC has no support for checksum type");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PADATA_TYPE_NOSUPP,                   "KRB5KDCPADataSupportError",      "KDC has no support for padata type");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_TRTYPE_NOSUPP,                        "KRB5KDCTypeSupportError",        "KDC has no support for transited type");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CLIENT_REVOKED,                       "KRB5KDCClientRevokedError",      "Clients credentials have been revoked");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_REVOKED,                      "KRB5KDCServerRevokedError",      "Credentials for server have been revoked");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_TGT_REVOKED,                          "KRB5KDCTGTRevokedError",         "TGT has been revoked");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_CLIENT_NOTYET,                        "KRB5KDCClientNotYetValidError",  "Client not yet valid - try again later");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVICE_NOTYET,                       "KRB5KDCServerNotYetValidError",  "Server not yet valid - try again later");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_KEY_EXP,                              "KRB5KDCPasswordExpiredError",    "Password has expired");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PREAUTH_FAILED,                       "KRB5KDCPreauthFailedError",      "Preauthentication failed");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PREAUTH_REQUIRED,                     "KRB5KDCPreauthRequiredError",    "Additional pre-authentication required");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SERVER_NOMATCH,                       "KRB5KDCServerMatchError",        "Requested server and ticket don't match");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_MUST_USE_USER2USER,                   "KRB5KDCRequireUser2UserError",   "Server principal valid for user2user only");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_PATH_NOT_ACCEPTED,                    "KRB5KDCPathError",               "KDC policy rejects transited path");
+        _pykadminerror_error_insert(module, base, KRB5KDC_ERR_SVC_UNAVAILABLE,                      "KRB5KDCServiceUnavailableError", "A service is not available that is required to process the request");
+
         // think AP stands for authentication or application protocol ? not sure
         _pykadminerror_error_insert(module, base, KRB5KRB_AP_ERR_BAD_INTEGRITY,                     "APIntegrityError",         "Decrypt integrity check failed");
         _pykadminerror_error_insert(module, base, KRB5KRB_AP_ERR_TKT_EXPIRED,                       "APTicketExpiredError",     "Ticket expired");
@@ -422,7 +386,7 @@ int PyKAdminError_init_krb5(PyObject *module, PyObject *base) {
         _pykadminerror_error_insert(module, base, KRB5_BAD_ENCTYPE,                                 "EncryptionTypeError", "Bad encryption type");
         _pykadminerror_error_insert(module, base, KRB5_BAD_KEYSIZE,                                 "KeySizeError",        "Key size is incompatible with encryption type");
         _pykadminerror_error_insert(module, base, KRB5_BAD_MSIZE,                                   "MessageSizeError",    "Message size is incompatible with encryption type");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_CC_TYPE_EXISTS,                              "CCTypeExistsError", "Credentials cache type is already registered.");
         _pykadminerror_error_insert(module, base, KRB5_KT_TYPE_EXISTS,                              "KTTypeExistsError", "Key table type is already registered.");
 
@@ -445,21 +409,21 @@ int PyKAdminError_init_krb5(PyObject *module, PyObject *base) {
         _pykadminerror_error_insert(module, base, KRB5_PREAUTH_BAD_TYPE,                            "PreauthTypeError",                "Unsupported preauthentication type");
         _pykadminerror_error_insert(module, base, KRB5_PREAUTH_NO_KEY,                              "PreauthKeyError",                 "Required preauthentication key not supplied");
         _pykadminerror_error_insert(module, base, KRB5_PREAUTH_FAILED,                              "PreauthGenericError",             "Generic preauthentication failure");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_RCACHE_BADVNO,                               "RCVserionNumberError", "Unsupported replay cache format version number");
         _pykadminerror_error_insert(module, base, KRB5_CCACHE_BADVNO,                               "CCVserionNumberError", "Unsupported credentials cache format version number");
         _pykadminerror_error_insert(module, base, KRB5_KEYTAB_BADVNO,                               "KTVersionNumberError", "Unsupported key table format version number");
 
         _pykadminerror_error_insert(module, base, KRB5_PROG_ATYPE_NOSUPP,                           "ProgramAddressTypeError", "Program lacks support for address type");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_RC_REQUIRED,                                 "RCRequiredError", "Message replay detection requires rcache parameter");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_ERR_BAD_HOSTNAME,                            "HostnameError",               "Hostname cannot be canonicalized");
         _pykadminerror_error_insert(module, base, KRB5_ERR_HOST_REALM_UNKNOWN,                      "HostRealmUnknownError",       "Cannot determine realm for host");
         _pykadminerror_error_insert(module, base, KRB5_SNAME_UNSUPP_NAMETYPE,                       "ServiceNameUnsupportedError", "Conversion to service principal undefined for name type");
-        
+
         _pykadminerror_error_insert(module, base, KRB5KRB_AP_ERR_V4_REPLY,                          "APV4ReplyError", "Initial Ticket response appears to be Version 4 error");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_REALM_CANT_RESOLVE,                          "RealmResolveError",             "Cannot resolve network address for KDC in requested realm");
         _pykadminerror_error_insert(module, base, KRB5_TKT_NOT_FORWARDABLE,                         "TicketNotForwardableError",     "Requesting ticket can't get forwardable tickets");
         _pykadminerror_error_insert(module, base, KRB5_FWD_BAD_PRINCIPAL,                           "ForwardPrincipalError",         "Bad principal name while trying to forward credentials");
@@ -479,7 +443,7 @@ int PyKAdminError_init_krb5(PyObject *module, PyObject *base) {
         _pykadminerror_error_insert(module, base, KRB5_NOPERM_ETYPE,                                "EncryptionTypeError",           "Encryption type not permitted");
         _pykadminerror_error_insert(module, base, KRB5_CONFIG_ETYPE_NOSUPP,                         "ConfigEncryptionTypeError",     "No supported encryption types (config file error?)");
         _pykadminerror_error_insert(module, base, KRB5_OBSOLETE_FN,                                 "ObsoleteFunctionError",         "Program called an obsolete, deleted function");
-        
+
         _pykadminerror_error_insert(module, base, KRB5_EAI_FAIL,                                    "EAIGenericError",         "unknown getaddrinfo failure");
         _pykadminerror_error_insert(module, base, KRB5_EAI_NODATA,                                  "EAINoDataError",          "no data available for host/domain name");
         _pykadminerror_error_insert(module, base, KRB5_EAI_NONAME,                                  "EAINoNameError",          "host/domain name not found");
@@ -499,8 +463,8 @@ int PyKAdminError_init_krb5(PyObject *module, PyObject *base) {
         _pykadminerror_error_insert(module, base, KRB5_LOCAL_ADDR_REQUIRED,                         "LocalAddressRequiredError",  "Auth context must contain local address");
         _pykadminerror_error_insert(module, base, KRB5_REMOTE_ADDR_REQUIRED,                        "RemoteAddressRequiredError", "Auth context must contain remote address");
         _pykadminerror_error_insert(module, base, KRB5_TRACE_NOSUPP,                                "TraceSupportError",          "Tracing unsupported");
-    
-        result = 1;   
+
+        result = 1;
     }
 
     return result;
@@ -515,7 +479,7 @@ int PyKAdminError_init_kadm(PyObject *module, PyObject *base) {
     //_pykadmin_kadm_errors = PyDict_New();
 
     if (_pykadmin_errors) {
- 
+
         _pykadminerror_error_insert(module, base, KADM5_FAILURE,                  "FailureError",                 "Operation failed for unspecified reason");
         _pykadminerror_error_insert(module, base, KADM5_AUTH_GET,                 "AuthGetError",                 "Operation requires ``get'' privilege");
         _pykadminerror_error_insert(module, base, KADM5_AUTH_ADD,                 "AuthAddError",                 "Operation requires ``add'' privilege");
@@ -578,7 +542,7 @@ int PyKAdminError_init_kadm(PyObject *module, PyObject *base) {
 #       ifdef KADM5_PASS_Q_GENERIC
             _pykadminerror_error_insert(module, base, KADM5_PASS_Q_GENERIC,           "PasswordGenericError",         "Database synchronization failed");
 #       endif
-    
+
         result = 1;
     }
 
@@ -587,6 +551,10 @@ int PyKAdminError_init_kadm(PyObject *module, PyObject *base) {
 }
 
 
+
+
+
+
 int PyKAdminError_init_kdb(PyObject *module, PyObject *base) {
 
     int result = 0;
diff --git a/PyKAdminErrors.h b/PyKAdminErrors.h
index ffaeb6d..6fb6fb9 100644
--- a/PyKAdminErrors.h
+++ b/PyKAdminErrors.h
@@ -12,16 +12,14 @@
 
 #include "pykadmin.h"
 
-#define PyKAdmin_RETURN_ERROR(value, caller) { PyKAdminError_raise_error((long)value, caller); return NULL; }
-
-//#define PyKAdmin_RETURN_KADM5_ERROR(retval, caller) { PyKAdminError_raise_kadm_error(retval, caller); return NULL; }
-//#define PyKAdmin_RETURN_KRB5_ERROR(code, caller) { PyKAdminError_raise_krb5_error(code, caller); return NULL; }
-
 PyObject *PyKAdminError_init(PyObject *module);
 
-//void PyKAdminError_raise_kadm_error(kadm5_ret_t retval, char *caller);
-//void PyKAdminError_raise_krb5_error(krb5_error_code code, char *caller);
 void PyKAdminError_raise_error(long code, char *caller);
 
+/*
+void PyKAdminError_raise_kadm_error(kadm5_ret_t retval, char *caller);
+void PyKAdminError_raise_krb5_error(krb5_error_code code, char *caller);
+void PyKAdminError_raise_kdb_error(krb5_error_code retval, char *caller);
+*/
 
 #endif
diff --git a/PyKAdminIterator.c b/PyKAdminIterator.c
index 23efd6e..a4884b7 100644
--- a/PyKAdminIterator.c
+++ b/PyKAdminIterator.c
@@ -85,17 +85,18 @@ PyKAdminIterator *PyKAdminIterator_principal_iterator(PyKAdminObject *kadmin, ch
 
     if (iter) {
 
-            iter->count = 0x0; 
-            iter->index = 0x0;
+        iter->count = 0x0; 
+        iter->index = 0x0;
 
-            iter->kadmin = kadmin;
-            Py_INCREF(kadmin);
+        iter->kadmin = kadmin;
+        Py_INCREF(kadmin);
 
-            retval = kadm5_get_principals(kadmin->server_handle, match, &iter->names, &iter->count);
-            if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_get_principals"); }
+        retval = kadm5_get_principals(kadmin->server_handle, match, &iter->names, &iter->count);
+        if (retval != KADM5_OK) { 
+            PyKAdminError_raise_error(retval, "kadm5_get_principals");
+        }
     }
 
-    Py_XINCREF(iter);
     return iter;
 }
 
@@ -107,17 +108,18 @@ PyKAdminIterator *PyKAdminIterator_policy_iterator(PyKAdminObject *kadmin, char
 
     if (iter) {
 
-            iter->count = 0x0; 
-            iter->index = 0x0;
+        iter->count = 0x0; 
+        iter->index = 0x0;
 
-            iter->kadmin = kadmin;
-            Py_INCREF(kadmin);
+        iter->kadmin = kadmin;
+        Py_INCREF(kadmin);
 
-            retval = kadm5_get_policies(kadmin->server_handle, match, &iter->names, &iter->count);
-            if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_get_policies"); }
+        retval = kadm5_get_policies(kadmin->server_handle, match, &iter->names, &iter->count);
+        if (retval != KADM5_OK) { 
+            PyKAdminError_raise_error(retval, "kadm5_get_policies"); 
+        }
     }
 
-    Py_XINCREF(iter);
     return iter;
 }
 
diff --git a/PyKAdminObject.c b/PyKAdminObject.c
index e70d4ae..413d382 100644
--- a/PyKAdminObject.c
+++ b/PyKAdminObject.c
@@ -12,11 +12,14 @@ static void PyKAdminObject_dealloc(PyKAdminObject *self) {
     kadm5_ret_t retval;
 
     if (self) {
-        krb5_db_unlock(self->context);
+
+        if (self->locked)
+            krb5_db_unlock(self->context);
 
         if (self->server_handle) {
             retval = kadm5_destroy(self->server_handle);
-            if (retval) {}
+            if (retval != KADM5_OK) {
+            }
             self->server_handle = NULL;
         }
         
@@ -44,17 +47,29 @@ static PyObject *PyKAdminObject_new(PyTypeObject *type, PyObject *args, PyObject
     if (self) {
 
         retval = kadm5_init_krb5_context(&self->context);
-        if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_init_krb5_context"); }
+        if (retval != KADM5_OK) { 
+            PyKAdminError_raise_error(retval, "kadm5_init_krb5_context");
+            Py_TYPE(self)->tp_free((PyObject *)self);
+            self = NULL;
+            goto cleanup;
+        }
 
         self->server_handle = NULL;
 
         // attempt to load the default realm 
         code = krb5_get_default_realm(self->context, &self->realm);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_get_default_realm"); }
+
+        /*if (code) { 
+            PyKAdminError_raise_krb5_error(code, "krb5_get_default_realm");
+            goto cleanup;
+        }*/
 
         self->_storage = PyDict_New();
+        self->locked = 0;
     }
 
+cleanup:
+    
     return (PyObject *)self;    
 
 }
@@ -81,13 +96,21 @@ static PyObject *PyKAdminObject_principal_exists(PyKAdminObject *self, PyObject
     if (self->server_handle) {
 
         code = krb5_parse_name(self->context, client_name, &princ);
-        if (code) { PyKAdmin_RETURN_ERROR(retval, "krb5_parse_name"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_parse_name");
+            goto cleanup;
+        }
 
         retval = kadm5_get_principal(self->server_handle, princ, &entry, KADM5_PRINCIPAL);
         if (retval == KADM5_OK) { result = Py_True; }
         else if (retval == KADM5_UNK_PRINC) { result = Py_False; }
-        else { PyKAdmin_RETURN_ERROR(retval, "kadm5_delete_principal"); }
+        else { 
+            PyKAdminError_raise_error(retval, "kadm5_get_principal");
+            goto cleanup;
+        }
     }
+
+cleanup:
     
     krb5_free_principal(self->context, princ);
     kadm5_free_principal_ent(self->server_handle, &entry);
@@ -104,6 +127,8 @@ static PyObject *PyKAdminObject_delete_principal(PyKAdminObject *self, PyObject
     krb5_principal princ = NULL;
 
     char *client_name = NULL;
+    
+    PyObject *result = Py_True;
 
     if (!PyArg_ParseTuple(args, "s", &client_name))
         return NULL;
@@ -111,16 +136,28 @@ static PyObject *PyKAdminObject_delete_principal(PyKAdminObject *self, PyObject
     if (self->server_handle) {
 
         code = krb5_parse_name(self->context, client_name, &princ);
-        if (code) { PyKAdmin_RETURN_ERROR(retval, "krb5_parse_name"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_parse_name");
+            result = NULL;
+            goto cleanup;
+        }
 
         retval = kadm5_delete_principal(self->server_handle, princ);
-        if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_delete_principal"); }
+        if (retval != KADM5_OK) {
+            PyKAdminError_raise_error(retval, "kadm5_delete_principal");
+            result = NULL;
+            goto cleanup;
+        }
 
     }
+
+cleanup:
     
-    krb5_free_principal(self->context, princ);
+    if (princ)
+        krb5_free_principal(self->context, princ);
 
-    Py_RETURN_TRUE;
+    Py_XINCREF(result);
+    return result;
 
 }
 
@@ -131,7 +168,9 @@ static PyObject *PyKAdminObject_create_principal(PyKAdminObject *self, PyObject
     krb5_error_code code = 0;
     char *princ_name = NULL;
     char *princ_pass = NULL;
-    PyDictObject *db_args = NULL;
+    PyObject *db_args = NULL;
+
+    PyObject *result = Py_True;
 
     kadm5_principal_ent_rec entry;
     
@@ -152,16 +191,26 @@ static PyObject *PyKAdminObject_create_principal(PyKAdminObject *self, PyObject
     if (self->server_handle) {
 
         code = krb5_parse_name(self->context, princ_name, &entry.principal);
-        if (code) { PyKAdmin_RETURN_ERROR(retval, "krb5_parse_name"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_parse_name");
+            result = NULL;
+            goto cleanup;
+        }
 
-        retval = kadm5_create_principal(self->server_handle, &entry, KADM5_PRINCIPAL | KADM5_TL_DATA, princ_pass); 
-        if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_create_principal"); }
+        retval = kadm5_create_principal(self->server_handle, &entry, (KADM5_PRINCIPAL | KADM5_TL_DATA), princ_pass); 
+        if (retval != KADM5_OK) {
+            PyKAdminError_raise_error(retval, "kadm5_create_principal");
+            result = NULL;
+        }
 
     }
 
+cleanup:
+
     kadm5_free_principal_ent(self->server_handle, &entry);
 
-    Py_RETURN_TRUE;
+    Py_XINCREF(result);
+    return result;
 }
 
 
@@ -285,10 +334,12 @@ static int kdb_iter_princs(void *data, krb5_db_entry *kdb) {
 
 static PyObject *PyKAdminObject_each_principal(PyKAdminObject *self, PyObject *args, PyObject *kwds) {
 
+    PyObject *result = Py_True;
     char *match = NULL;
     krb5_error_code code = 0; 
     kadm5_ret_t lock = KADM5_OK; 
 
+
     static char *kwlist[] = {"callback", "data", "match", NULL};
     
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!|Oz", kwlist, &PyFunction_Type, &self->each_principal.callback, &self->each_principal.data, &match))
@@ -306,26 +357,37 @@ static PyObject *PyKAdminObject_each_principal(PyKAdminObject *self, PyObject *a
 
     if ((lock == KADM5_OK) || (lock == KRB5_PLUGIN_OP_NOTSUPP)) {
 
+        if (lock == KADM5_OK)
+            self->locked = 1;
+
         krb5_clear_error_message(self->context);
 
         code = krb5_db_iterate(self->context, match, kdb_iter_princs, (void *)self);
     
-        if (lock != KRB5_PLUGIN_OP_NOTSUPP)   
+        if (lock != KRB5_PLUGIN_OP_NOTSUPP)  {
             lock = kadm5_unlock(self->server_handle);
+            if (lock == KADM5_OK)
+                self->locked = 0;
+        }
     }
 
     Py_DECREF(self->each_principal.callback);
     Py_DECREF(self->each_principal.data);
 
-    if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_db_iterate"); }
+    if (code) { 
+        PyKAdminError_raise_error(code, "krb5_db_iterate");
+        result = NULL;
+        goto cleanup;
+    }
 
     if (self->each_principal.error) {
         _pykadmin_each_restore_error(self->each_principal.error);
-        return NULL;
     }
 
+cleanup:
 
-    Py_RETURN_TRUE;
+    Py_XINCREF(result);
+    return(result);
 
 }
 
@@ -362,6 +424,8 @@ static PyObject *PyKAdminObject_each_policy(PyKAdminObject *self, PyObject *args
     krb5_error_code code = 0; 
     kadm5_ret_t lock = KADM5_OK; 
 
+    PyObject *result = Py_True;
+
     static char *kwlist[] = {"", "data", "match", NULL};
     
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!|Oz", kwlist, &PyFunction_Type, &self->each_policy.callback, &self->each_policy.data, &match))
@@ -377,25 +441,38 @@ static PyObject *PyKAdminObject_each_policy(PyKAdminObject *self, PyObject *args
 
     if ((lock == KADM5_OK) || (lock == KRB5_PLUGIN_OP_NOTSUPP)) {
 
+        if (lock == KADM5_OK)
+            self->locked = 1;
+
         krb5_clear_error_message(self->context);
 
         code = krb5_db_iter_policy(self->context, match, kdb_iter_pols, (void *)self);
     
-        if (lock != KRB5_PLUGIN_OP_NOTSUPP)
+        if (lock != KRB5_PLUGIN_OP_NOTSUPP)  {
             lock = kadm5_unlock(self->server_handle);
+            if (lock == KADM5_OK)
+                self->locked = 0;
+        }
     }
 
     Py_DECREF(self->each_policy.callback);
     Py_DECREF(self->each_policy.data);
 
-    if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_db_iter_policy"); }
+    if (code) { 
+        PyKAdminError_raise_error(code, "krb5_db_iter_policy");
+        result = NULL;
+        goto cleanup;
+    } 
 
     if (self->each_policy.error) {
         _pykadmin_each_restore_error(self->each_policy.error);
-        return NULL;
+        result = NULL;
     }
 
-    Py_RETURN_TRUE;
+cleanup:
+
+    Py_XINCREF(result);
+    return result;
 
 }
 #endif
diff --git a/PyKAdminObject.h b/PyKAdminObject.h
index 828f731..fe5b305 100644
--- a/PyKAdminObject.h
+++ b/PyKAdminObject.h
@@ -18,6 +18,8 @@ typedef struct {
 typedef struct {
     PyObject_HEAD
     
+    uint8_t locked; 
+
     krb5_context context; 
     void *server_handle;
     char *realm;
diff --git a/PyKAdminPrincipalObject.c b/PyKAdminPrincipalObject.c
index b23275a..9be4e8e 100644
--- a/PyKAdminPrincipalObject.c
+++ b/PyKAdminPrincipalObject.c
@@ -196,7 +196,7 @@ static PyObject *PyKAdminPrincipal_set_attributes(PyKAdminPrincipalObject *self,
         self->mask |= KADM5_ATTRIBUTES;
 
         //retval = kadm5_modify_principal(self->kadmin->server_handle, &self->entry, KADM5_ATTRIBUTES);
-        //if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_modify_principal"); return NULL; }
+        //if (retval != KADM5_OK) { raise_error(retval, "kadm5_modify_principal"); return NULL; }
     }
 
     Py_RETURN_TRUE;
@@ -222,46 +222,67 @@ static PyObject *PyKAdminPrincipal_unset_attributes(PyKAdminPrincipalObject *sel
 
 static PyObject *PyKAdminPrincipal_commit(PyKAdminPrincipalObject *self) {
 
+    PyObject *result = NULL;
     kadm5_ret_t retval = KADM5_OK; 
 
     if (self && self->mask) {
 
         retval = kadm5_modify_principal(self->kadmin->server_handle, &self->entry, self->mask);
-        if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_modify_principal"); } 
+        
+        if (retval == KADM5_OK) {
+            result = Py_True;
+        } else { 
+            PyKAdminError_raise_error(retval, "kadm5_modify_principal"); 
+            goto cleanup;
+        } 
 
         self->mask = 0;
     }
 
-    Py_RETURN_TRUE;
+cleanup:
+
+    Py_XINCREF(result);
+    return result;
 }
 
 static PyObject *PyKAdminPrincipal_reload(PyKAdminPrincipalObject *self) {
 
-    krb5_error_code ret = 0;
+    PyObject *result = NULL;
+
+    krb5_error_code code = 0;
+
     kadm5_ret_t retval = KADM5_OK; 
 
     krb5_principal temp = NULL;
 
     if (self) {
 
-        // we need to free prior to fetching otherwise we leak memory since principal and policy are pointers, alternitively we could manually free those
-        ret = krb5_copy_principal(self->kadmin->context, self->entry.principal, &temp);
-        if (ret) {}
+        code = krb5_copy_principal(self->kadmin->context, self->entry.principal, &temp);
+        if (code) {
+            PyKAdminError_raise_error(code, "krb5_copy_principal");
+            goto cleanup;
+        }
 
         retval = kadm5_free_principal_ent(self->kadmin->server_handle, &self->entry);
-        if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_free_principal_ent"); } 
-
-        if (retval == KADM5_OK) {
-            retval = kadm5_get_principal(self->kadmin->server_handle, temp, &self->entry, KADM5_PRINCIPAL_NORMAL_MASK);
-            if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_get_principal"); }
-        }
+        if (retval != KADM5_OK) { 
+            PyKAdminError_raise_error(retval, "kadm5_free_principal_ent"); 
+            goto cleanup;
+        } 
 
-        krb5_free_principal(self->kadmin->context, temp);
+        retval = kadm5_get_principal(self->kadmin->server_handle, temp, &self->entry, KADM5_PRINCIPAL_NORMAL_MASK);
+        if (retval != KADM5_OK) { 
+            PyKAdminError_raise_error(retval, "kadm5_get_principal"); 
+            goto cleanup;
+        }    
 
-        if (retval != KADM5_OK) { return NULL; }        
     }
 
-    Py_RETURN_TRUE;
+cleanup:
+    
+    krb5_free_principal(self->kadmin->context, temp);
+
+    Py_XINCREF(result);
+    return result;
 }
 
 
@@ -272,6 +293,7 @@ static PyObject *PyKAdminPrincipal_unlock(PyKAdminPrincipalObject *self) {
 
 static PyObject *PyKAdminPrincipal_change_password(PyKAdminPrincipalObject *self, PyObject *args, PyObject *kwds) {
 
+    PyObject *result   = Py_True;
     kadm5_ret_t retval = KADM5_OK; 
     char *password     = NULL;
 
@@ -279,19 +301,28 @@ static PyObject *PyKAdminPrincipal_change_password(PyKAdminPrincipalObject *self
         return NULL; 
 
     retval = kadm5_chpass_principal(self->kadmin->server_handle, self->entry.principal, password);
-    if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_chpass_principal"); return NULL; }
+    if (retval != KADM5_OK) {
+        PyKAdminError_raise_error(retval, "kadm5_chpass_principal");
+        result = NULL;
+    }
 
-    Py_RETURN_TRUE;
+    Py_XINCREF(result);
+    return result;
 }
 
 static PyObject *PyKAdminPrincipal_randomize_key(PyKAdminPrincipalObject *self) {
 
+    PyObject *result   = Py_True;
     kadm5_ret_t retval = KADM5_OK; 
 
     retval = kadm5_randkey_principal(self->kadmin->server_handle, self->entry.principal, NULL, NULL);
-    if (retval != KADM5_OK) { PyKAdminError_raise_error(retval, "kadm5_randkey_principal"); return NULL; }
+    if (retval != KADM5_OK)  {
+        PyKAdminError_raise_error(retval, "kadm5_randkey_principal");
+        result = NULL;
+    }
 
-    Py_RETURN_TRUE;
+    Py_XINCREF(result);
+    return result;
 }
 
 PyObject *PyKAdminPrincipal_RichCompare(PyObject *o1, PyObject *o2, int opid) {
@@ -331,17 +362,21 @@ PyObject *PyKAdminPrincipal_RichCompare(PyObject *o1, PyObject *o2, int opid) {
 static PyObject *PyKAdminPrincipal_get_principal(PyKAdminPrincipalObject *self, void *closure) {
   
     krb5_error_code code = 0;
-    PyObject *principal = NULL;
-    char *client_name   = NULL;
+    PyObject *principal  = NULL;
+    char *client_name    = NULL;
     
-    // todo: handle krb5_error_code
     code = krb5_unparse_name(self->kadmin->context, self->entry.principal, &client_name);
-    if (code) {}
+    if (code) {
+        PyKAdminError_raise_error(code, "krb5_unparse_name");
+        goto cleanup;
+    }
 
-    if (client_name) {
-        principal = PyUnicode_FromString(client_name);
+    principal = PyUnicode_FromString(client_name);
+
+cleanup:
+
+    if (client_name)
         free(client_name);
-    }
 
     return principal;
 }
@@ -349,17 +384,22 @@ static PyObject *PyKAdminPrincipal_get_principal(PyKAdminPrincipalObject *self,
 
 static PyObject *PyKAdminPrincipal_get_mod_name(PyKAdminPrincipalObject *self, void *closure) {
   
-    krb5_error_code ret = 0;
-    PyObject *principal = NULL;
-    char *client_name   = NULL;
+    krb5_error_code code = 0;
+    PyObject *principal  = NULL;
+    char *client_name    = NULL;
     
-    // todo: handle error
-    ret = krb5_unparse_name(self->kadmin->context, self->entry.mod_name, &client_name);
+    code = krb5_unparse_name(self->kadmin->context, self->entry.mod_name, &client_name);
+    if (code) {
+        PyKAdminError_raise_error(code, "krb5_unparse_name");
+        goto cleanup;
+    }
 
-    if (client_name) {
-        principal = PyUnicode_FromString(client_name);
+    principal = PyUnicode_FromString(client_name);
+
+cleanup:
+
+    if (client_name)
         free(client_name);
-    }
 
     return principal;
 }
@@ -950,7 +990,7 @@ PyTypeObject PyKAdminPrincipalObject_Type = {
 
 PyKAdminPrincipalObject *PyKAdminPrincipalObject_principal_with_name(PyKAdminObject *kadmin, char *client_name) {
         
-    krb5_error_code errno;
+    krb5_error_code code;
     kadm5_ret_t retval = KADM5_OK;
 
     PyKAdminPrincipalObject *principal = (PyKAdminPrincipalObject *)Py_None;
@@ -965,12 +1005,13 @@ PyKAdminPrincipalObject *PyKAdminPrincipalObject_principal_with_name(PyKAdminObj
             Py_INCREF(kadmin);
             principal->kadmin = kadmin;
 
-            errno = krb5_parse_name(kadmin->context, client_name, &temp);
+            code = krb5_parse_name(kadmin->context, client_name, &temp);
+
             retval = kadm5_get_principal(kadmin->server_handle, temp, &principal->entry, (KADM5_PRINCIPAL_NORMAL_MASK | KADM5_KEY_DATA));
 
             krb5_free_principal(kadmin->context, temp);
 
-            if ((retval != KADM5_OK) || errno) {
+            if ((retval != KADM5_OK) || code) {
                 PyKAdminPrincipal_dealloc(principal);
                 principal = (PyKAdminPrincipalObject *)Py_None;
             }
diff --git a/kadmin.c b/kadmin.c
index 650c620..c9e78ce 100644
--- a/kadmin.c
+++ b/kadmin.c
@@ -182,19 +182,23 @@ static PyKAdminObject *_kadmin_local(PyObject *self, PyObject *args) {
 
     static const char *kROOT_ADMIN = "root/admin";
 
-    PyKAdminObject *kadmin = PyKAdminObject_create();
-    PyObject *py_db_args = NULL;
-    kadm5_ret_t retval     = KADM5_OK; 
-    int result             = 0;
+    PyKAdminObject *kadmin = NULL;
+    PyObject *py_db_args   = NULL;
     char **db_args         = NULL;
     char *client_name      = NULL;
+    kadm5_config_params *params = NULL;
+
+    kadm5_ret_t retval     = KADM5_OK; 
+    int result             = 0;
 
     if (!PyArg_ParseTuple(args, "|O", &py_db_args))
         return NULL; 
 
-    db_args = pykadmin_parse_db_args(py_db_args);
+    kadmin = PyKAdminObject_create();
+
+    params = calloc(0x1, sizeof(kadm5_config_params));
 
-    kadm5_config_params *params = calloc(0x1, sizeof(kadm5_config_params));
+    db_args = pykadmin_parse_db_args(py_db_args);
 
     result = asprintf(&client_name, "%s@%s", kROOT_ADMIN, kadmin->realm);
 
@@ -213,10 +217,22 @@ static PyKAdminObject *_kadmin_local(PyObject *self, PyObject *args) {
                 db_args, 
                 &kadmin->server_handle);
 
+    if (retval != KADM5_OK) {
 
-    pykadmin_free_db_args(db_args);
+        Py_XDECREF(kadmin);
+        kadmin = NULL;
+
+        PyKAdminError_raise_error(retval, "kadm5_init_with_password.local");
+
+    }
+    
+    if (client_name)
+        free(client_name);
 
-    if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_init_with_password.local"); }
+    if (params)
+        free(params);
+
+    pykadmin_free_db_args(db_args);
 
     return kadmin;
 
@@ -226,19 +242,20 @@ static PyKAdminObject *_kadmin_local(PyObject *self, PyObject *args) {
 
 static PyKAdminObject *_kadmin_init_with_ccache(PyObject *self, PyObject *args) {
     
-    PyKAdminObject *kadmin = PyKAdminObject_create();
-    PyObject *py_db_args = NULL;
-    kadm5_ret_t retval = KADM5_OK;
-    krb5_error_code code = 0;
+    PyKAdminObject *kadmin = NULL;
+    PyObject *py_db_args   = NULL;
+    kadm5_ret_t retval     = KADM5_OK;
+    krb5_error_code code   = 0;
 
-    krb5_principal princ = NULL;
-    char *ccache_name    = NULL;
-    char *client_name    = NULL;
-    char **db_args       = NULL;
+    krb5_principal princ   = NULL;
+    char *ccache_name      = NULL;
+    char *client_name      = NULL;
+    char *_resolved_client = NULL;
+    char **db_args         = NULL;
 
-    krb5_ccache cc;             
+    kadm5_config_params *params = NULL;
 
-    kadm5_config_params *params = calloc(0x1, sizeof(kadm5_config_params));
+    krb5_ccache cc;             
 
     memset(&cc, 0, sizeof(krb5_ccache));
 
@@ -246,29 +263,44 @@ static PyKAdminObject *_kadmin_init_with_ccache(PyObject *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "|zzO", &client_name, &ccache_name, &py_db_args))
         return NULL; 
 
+    kadmin = PyKAdminObject_create();
+    params = calloc(0x1, sizeof(kadm5_config_params));
+
     db_args = pykadmin_parse_db_args(py_db_args);
 
     if (!ccache_name) {
         code = krb5_cc_default(kadmin->context, &cc);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_cc_default"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_cc_default");
+            goto cleanup;
+        }
     } else {
         code = krb5_cc_resolve(kadmin->context, ccache_name, &cc);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_cc_resolve"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_cc_resolve");
+            goto cleanup;
+        }
     } 
 
-    if (!client_name) {
-        code = krb5_cc_get_principal(kadmin->context, cc, &princ);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_cc_get_principal"); }
-
-        code = krb5_unparse_name(kadmin->context, princ, &client_name);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_unparse_name"); }
+    _resolved_client = client_name;
 
-        krb5_free_principal(kadmin->context, princ);
+    if (!_resolved_client) {
+        code = krb5_cc_get_principal(kadmin->context, cc, &princ);
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_cc_get_principal");
+            goto cleanup;
+        }
+
+        code = krb5_unparse_name(kadmin->context, princ, &_resolved_client);
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_unparse_name");
+            goto cleanup;
+        }
     }
     
     retval = kadm5_init_with_creds(
                 kadmin->context, 
-                client_name, 
+                _resolved_client, 
                 cc, 
                 service_name, 
                 params,
@@ -277,9 +309,29 @@ static PyKAdminObject *_kadmin_init_with_ccache(PyObject *self, PyObject *args)
                 db_args, 
                 &kadmin->server_handle);
 
-    pykadmin_free_db_args(db_args);
+    if (retval != KADM5_OK) { 
+
+        Py_XDECREF(kadmin);
+        kadmin = NULL;
+
+        PyKAdminError_raise_error(retval, "kadm5_init_with_creds");
+    }
+
+
+cleanup:
+    
+    // we only clean up _resolved_client if we calculated it, otherwise it is 
+    //  an internal pointer of a python object and freeing it will be illegal.
+    if ((client_name == NULL) && _resolved_client)
+        free(_resolved_client);
+
+    krb5_free_principal(kadmin->context, princ);
+    krb5_cc_close(kadmin->context, cc);
 
-    if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_init_with_creds"); }
+    if (params)
+        free(params);  
+
+    pykadmin_free_db_args(db_args);
 
     return kadmin;
 }
@@ -288,10 +340,10 @@ static PyKAdminObject *_kadmin_init_with_ccache(PyObject *self, PyObject *args)
 
 static PyKAdminObject *_kadmin_init_with_keytab(PyObject *self, PyObject *args) {
 
-    PyKAdminObject *kadmin = PyKAdminObject_create();
+    PyKAdminObject *kadmin = NULL;
 
     PyObject *py_db_args = NULL;
-    kadm5_ret_t retval = KADM5_OK;
+    kadm5_ret_t retval   = KADM5_OK;
     krb5_error_code code = 0;
 
     krb5_principal princ = NULL;
@@ -299,11 +351,14 @@ static PyKAdminObject *_kadmin_init_with_keytab(PyObject *self, PyObject *args)
     char *keytab_name    = NULL;
     char **db_args       = NULL;
 
-    kadm5_config_params *params = calloc(0x1, sizeof(kadm5_config_params));
+    kadm5_config_params *params = NULL;
 
     if (!PyArg_ParseTuple(args, "|zzO", &client_name, &keytab_name, &py_db_args))
         return NULL; 
 
+    kadmin = PyKAdminObject_create();
+    params = calloc(0x1, sizeof(kadm5_config_params));
+
     db_args = pykadmin_parse_db_args(py_db_args);
 
     if (keytab_name == NULL) {
@@ -313,15 +368,18 @@ static PyKAdminObject *_kadmin_init_with_keytab(PyObject *self, PyObject *args)
     if (client_name == NULL) {
         
         code = krb5_sname_to_principal(kadmin->context, NULL, "host", KRB5_NT_SRV_HST, &princ);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_sname_to_principal"); }
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_sname_to_principal");
+            goto cleanup;
+        }
         
         code = krb5_unparse_name(kadmin->context, princ, &client_name);
-        if (code) { PyKAdmin_RETURN_ERROR(code, "krb5_unparse_name"); }
-
-        krb5_free_principal(kadmin->context, princ);
+        if (code) { 
+            PyKAdminError_raise_error(code, "krb5_unparse_name");
+            goto cleanup;
+        }
     }
 
-
     retval = kadm5_init_with_skey(
                 kadmin->context, 
                 client_name, 
@@ -333,9 +391,23 @@ static PyKAdminObject *_kadmin_init_with_keytab(PyObject *self, PyObject *args)
                 db_args, 
                 &kadmin->server_handle);
 
-    pykadmin_free_db_args(db_args);
+    if (retval != KADM5_OK) {
 
-    if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_init_with_skey"); }
+        Py_XDECREF(kadmin);
+        kadmin = NULL;
+
+        PyKAdminError_raise_error(retval, "kadm5_init_with_skey");
+    }
+
+cleanup:
+    
+    if (princ)
+        krb5_free_principal(kadmin->context, princ);
+
+    if (params)
+        free(params);
+
+    pykadmin_free_db_args(db_args);
 
     return kadmin;
 }
@@ -343,19 +415,22 @@ static PyKAdminObject *_kadmin_init_with_keytab(PyObject *self, PyObject *args)
 
 static PyKAdminObject *_kadmin_init_with_password(PyObject *self, PyObject *args) {
 
-    PyKAdminObject *kadmin = PyKAdminObject_create();
-    PyObject *py_db_args = NULL;
-    kadm5_ret_t retval = KADM5_OK;
+    PyKAdminObject *kadmin = NULL;
+    PyObject *py_db_args   = NULL;
+    kadm5_ret_t retval     = KADM5_OK;
     
     char *client_name = NULL;
     char *password    = NULL;
     char **db_args    = NULL;
      
-    kadm5_config_params *params = calloc(0x1, sizeof(kadm5_config_params));
+    kadm5_config_params *params = NULL;
 
     if (!PyArg_ParseTuple(args, "zz|O", &client_name, &password, &py_db_args))
         return NULL;
 
+    kadmin = PyKAdminObject_create();
+    params = calloc(0x1, sizeof(kadm5_config_params));
+
     db_args = pykadmin_parse_db_args(py_db_args);
 
     retval = kadm5_init_with_password(
@@ -369,9 +444,18 @@ static PyKAdminObject *_kadmin_init_with_password(PyObject *self, PyObject *args
                 db_args, 
                 &kadmin->server_handle);
 
-    pykadmin_free_db_args(db_args);
+    if (retval != KADM5_OK) { 
 
-    if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_init_with_password"); }
+        Py_XDECREF(kadmin);
+        kadmin = NULL;
+
+        PyKAdminError_raise_error(retval, "kadm5_init_with_password");
+    }
+
+    if (params)
+        free(params);
+
+    pykadmin_free_db_args(db_args);
 
     return kadmin;
 
diff --git a/pykadmin.h b/pykadmin.h
index 61908a0..61217f5 100644
--- a/pykadmin.h
+++ b/pykadmin.h
@@ -8,6 +8,10 @@ struct module_state {
     PyObject *error;
 };
 
+#define Py_XRETURN(obj) { Py_XINCREF(obj); return obj; } 
+
+#define Py_DEBUG_REFCOUNT(obj, str) {fprintf(stderr, "%s: %d\n", str, obj->ob_refcnt);}
+
 #ifdef KADMIN_LOCAL
 #	define kMODULE_NAME "kadmin_local"
 #else
diff --git a/test/bootstrap_kdb.py b/test/bootstrap_kdb.py
index 8efde35..23577e1 100644
--- a/test/bootstrap_kdb.py
+++ b/test/bootstrap_kdb.py
@@ -9,7 +9,7 @@ for a in xrange(97, 98):
         for c in xrange(97, 123):
             for d in xrange(97, 123):
                 try:
-                    admin.create_principal(chr(a) + chr(b) + chr(c) + chr(d));
+                    admin.ank(chr(a) + chr(b) + chr(c) + chr(d));
                 except kadmin.KAdminError as error:
                     print error
                     pass
diff --git a/test/krb5.supp b/test/krb5.supp
new file mode 100644
index 0000000..bdc7072
--- /dev/null
+++ b/test/krb5.supp
@@ -0,0 +1,10 @@
+
+
+{
+   kadm5_init_leaks
+   Memcheck:Leak
+   fun:malloc
+   ...
+   fun:kadm5_init
+   ...
+}
\ No newline at end of file
diff --git a/test/unittests.py b/test/unittests.py
index 733b0f2..f608cc9 100644
--- a/test/unittests.py
+++ b/test/unittests.py
@@ -1,4 +1,5 @@
 
+import gc
 import time
 import sys
 import kadmin
@@ -6,6 +7,7 @@ import kadmin_local
 import unittest
 import subprocess
 import logging
+from pprint import pprint
 
 import os.path
 
@@ -31,6 +33,8 @@ def create_test_prinicipal():
 
     if not os.path.isfile(TEST_KEYTAB):
 
+        sys.stderr.write("Generating test/admin keytab...\n")
+
         command = '''
 spawn kadmin.local -p root@EXAMPLE.COM
 
@@ -280,7 +284,6 @@ class KAdminUnitTests(unittest.TestCase):
 
 
 class KAdminLocalUnitTests(unittest.TestCase):
-#class KAdminLocalUnitTests():
 
     ''' Missing in 2.6 '''
     def assertIsNotNone(self, expr, msg=None):
@@ -291,7 +294,7 @@ class KAdminLocalUnitTests(unittest.TestCase):
 
     
     def setUp(self):
-    
+
         # let the exception bubble up the test.
         kadm = kadmin_local.local();
         
@@ -302,7 +305,7 @@ class KAdminLocalUnitTests(unittest.TestCase):
 
         self.logger = logging.getLogger('python-kadmin')
     
-    
+
     def test_local(self):
         
         try:    
@@ -374,7 +377,6 @@ class KAdminLocalUnitTests(unittest.TestCase):
         kadm = self.kadm
 
         create_test_accounts()
-        
         pre_size = database_size()
 
         try: 
@@ -383,6 +385,7 @@ class KAdminLocalUnitTests(unittest.TestCase):
         except:
             self.fail("kadmin_local.ank rasied an error deleting an account.") 
 
+        
         post_size = database_size()
     
         self.assertEqual(pre_size, len(TEST_ACCOUNTS) + post_size)
@@ -494,18 +497,43 @@ class KAdminLocalUnitTests(unittest.TestCase):
 
 def main():
     
-    confirm = input('run tests against local kadmin server [yes/no] ? ')
+    #confirm = input('run tests against local kadmin server [yes/no] ? ')
 
-    if confirm.lower() == 'yes':
+    #if confirm.lower() == 'yes':
 
+    if True:
         create_test_prinicipal()
         create_ccache()
 
         logging.basicConfig(filename=TEST_LOG, format=LOG_FORMAT, level=logging.DEBUG)
 
-        unittest.main()
+        # setup unit tests
+
+        kadmin_tests = unittest.TestLoader().loadTestsFromTestCase(KAdminUnitTests)
+        kadmin_local_tests = unittest.TestLoader().loadTestsFromTestCase(KAdminLocalUnitTests)
+
+        unittest.TextTestRunner(verbosity=2).run(kadmin_tests)
+        unittest.TextTestRunner(verbosity=2).run(kadmin_local_tests)
+
+        #unittest.main()
+
 
 if __name__ == '__main__':
     main()
 
+# delete global constants
+
+del TEST_PRINCIPAL
+del TEST_KEYTAB
+del TEST_CCACHE
+del TEST_PASSWORD
+del TEST_LOG
+del LOG_FORMAT
+del TEST_ACCOUNTS
+
+# collect memory so our valgrind reports clear up any still reachable which shouldnt be
+gc.collect()
+
 
+pprint(locals())
+pprint(globals())
diff --git a/test/valgrind-python-leaks.supp b/test/valgrind-python-leaks.supp
new file mode 100644
index 0000000..685bde0
--- /dev/null
+++ b/test/valgrind-python-leaks.supp
@@ -0,0 +1,193 @@
+# These are unfreed memory (still reachable blocks) detected in python
+# valgrind should not systematically run with these suppressions on, since
+# they might potentially hide real leaks (even though we would likely have
+# a matching Ada leak in any case), but these suppressions might prove
+# useful when analyzing the result of valgrind with --show-reachable=yes
+
+{
+  Python leak
+  Memcheck:Leak
+  fun:malloc
+  fun:*
+  fun:PyString_InternInPlace
+}
+{
+  Python leak
+  Memcheck:Leak
+  fun:malloc
+  fun:*
+  fun:PyEval_EvalFrameEx
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  fun:PyDict_Merge
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  fun:PyObject_GenericSetAttr
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*_gobject.so*
+  obj:*atk.so*
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*_gobject.so*
+  obj:*pango.so*
+  fun:initpango
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*_gobject.so*
+  obj:*gio.so*
+  fun:init_gio
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:realloc
+  fun:g_realloc
+  fun:g_type_set_qdata
+  obj:*_gobject.so*
+  fun:init_gobject
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*_gobject.so
+  obj:*_gtk.so
+  fun:init_gtk
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  fun:_PyInt_Init
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  fun:PyType_Ready
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  fun:PyString_FromStringAndSize
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*libpython*
+  fun:PyObject_Call
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  fun:PyInt_FromLong
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  fun:_PyObject_GC_Malloc
+}
+{
+  From opensuse 11.2 (python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  obj:*libpython*
+  obj:*libpython*
+  fun:PyEval_EvalFrameEx
+}
+
+
+{
+  Python leak
+  Memcheck:Leak
+  fun:malloc
+  fun:*
+  fun:PyDict_SetItemString
+}
+{                                                                               
+   <insert_a_suppression_name_here>                                             
+   Memcheck:Leak                                                                
+   fun:malloc                                                                   
+   fun:_PyObject_GC_Malloc                                                      
+   fun:PyType_GenericAlloc  
+}
+{
+  Python leak (ignore all memory allocated by python itself....)
+  Memcheck:Leak
+  fun:malloc
+  fun:PyObject_Malloc
+}
+{
+  Python leak (ignore all memory allocated by python itself....)
+  Memcheck:Leak
+  fun:realloc
+  fun:_PyObject_GC_Resize
+}
+{
+  Python leak (ignore all memory allocated by python itself....)
+  Memcheck:Leak
+  fun:realloc
+  fun:_PyObject_GC_NewVar
+}
+{
+  Python leak (ignore all memory allocated by python itself....)
+  Memcheck:Leak
+  fun:malloc
+  fun:_PyObject_GC_NewVar
+}
+{
+  Python leak
+  Memcheck:Leak
+  fun:malloc
+  fun:_PyObject_GC_NewVar
+  fun:PyFrame_New
+  fun:PyEval_EvalCodeEx
+  fun:function_call
+}
+{
+  Python leak (Python 2.6)
+  Memcheck:Leak
+  fun:malloc
+  fun:_PyObject_GC_Malloc
+  fun:_PyObject_GC_NewVar
+  fun:PyFrame_New
+}
+
+
+{
+  GNAT does not release the secondary stack
+  Memcheck:Leak
+  fun:malloc
+  fun:gnatcoll__memory__alloc
+  fun:system__secondary_stack__ss_allocate
+}
-- 
GitLab