Index: tools/univention_policy_result.c
===================================================================
--- tools/univention_policy_result.c	(Revision 24237)
+++ tools/univention_policy_result.c	(Arbeitskopie)
@@ -77,13 +77,13 @@
 			break;
 		switch (c) {
 			case 'h':
-				ldap_parameters->host = strdup(optarg);
+				ldap_parameters->host = optarg;
 				break;
 			case 'D':
-				ldap_parameters->binddn = strdup(optarg);
+				ldap_parameters->binddn = optarg;
 				break;
 			case 'w':
-				ldap_parameters->bindpw = strdup(optarg);
+				ldap_parameters->bindpw = optarg;
 				break;
 			case 'd':
 				opt_debug = 1;
@@ -111,7 +111,7 @@
 		univention_debug_init("/dev/null", 0, 0);
 	}
 
-	dn = argv[argc-1];
+	dn = argv[argc - 1];
 
 	if (univention_ldap_open(ldap_parameters) != 0) {
 		if (output == OUTPUT_VERBOSE) {
@@ -155,7 +155,7 @@
 					printf("\n");
 				} else if (output == OUTPUT_SHELL) {
 					for (i = 0; attribute->values->values[i] != NULL; i++) {
-						for (j = 0; j<strlen(attribute->name); j++) {
+						for (j = 0; j < strlen(attribute->name); j++) {
 							if (attribute->name[j] == ';' || attribute->name[j] == '-') {
 								printf("_");
 							} else {
@@ -181,7 +181,7 @@
 					if (attribute != policy->attributes)
 						printf(" ");
 					for (i = 0; attribute->values->values[i] != NULL; i++) {
-						if (i>0)
+						if (i > 0)
 							printf(" ");
 						printf("%s=\"%s\"", attribute->name, attribute->values->values[i]);
 					}
Index: debian/changelog
===================================================================
--- debian/changelog	(Revision 24237)
+++ debian/changelog	(Arbeitskopie)
@@ -7,8 +7,12 @@
   * Remove broken libunivention-policy-krb5-dev.
   * Add manual page.
   * Add linkting to liblber.
+  * Fix memory leaks and NULL pointer exceptions.
+  * Simplify allocating and cleaning memory.
+  * Handle line terminattion in ldap.secret.
+  * Make univention_krb5_init() thread-save.
 
- -- Philipp Hahn <hahn@univention.de>  Tue, 17 May 2011 18:34:11 +0200
+ -- Philipp Hahn <hahn@univention.de>  Tue, 17 May 2011 19:19:37 +0200
 
 univention-policy (4.0.2-1) unstable; urgency=low
 
Index: include/univention/policy.h
===================================================================
--- include/univention/policy.h	(Revision 24236)
+++ include/univention/policy.h	(Arbeitskopie)
@@ -60,8 +60,8 @@
 } univention_policy_handle_t;
 
 
-univention_policy_handle_t* univention_policy_open(LDAP* ld, char* base, char* dn);
-univention_policy_result_t* univention_policy_get(univention_policy_handle_t* handle, char* policy_name, char* attribute_name);
+univention_policy_handle_t* univention_policy_open(LDAP *ld, const char *base, const char *dn);
+univention_policy_result_t* univention_policy_get(univention_policy_handle_t *handle, const char *policy_name, const char *attribute_name);
 void univention_policy_close(univention_policy_handle_t* handle);
 
 #endif
Index: lib/policy.c
===================================================================
--- lib/policy.c	(Revision 24237)
+++ lib/policy.c	(Arbeitskopie)
@@ -53,7 +53,7 @@
 /*
  * returns object from list if it already exists, create new object otherwise
  */
-struct univention_policy_list_s* univention_policy_list_get(struct univention_policy_list_s** list, char* name)
+struct univention_policy_list_s* univention_policy_list_get(struct univention_policy_list_s** list, const char *name)
 {
 	struct univention_policy_list_s *new;
 	struct univention_policy_list_s *cur;
@@ -82,7 +82,7 @@
 /*
  * returns object from list if it already exists, create new object otherwise
  */
-struct univention_policy_attribute_list_s* univention_policy_attribute_list_get(struct univention_policy_attribute_list_s** list, char* name)
+struct univention_policy_attribute_list_s* univention_policy_attribute_list_get(struct univention_policy_attribute_list_s **list, const char *name)
 {
 	struct univention_policy_attribute_list_s *new;
 	struct univention_policy_attribute_list_s *cur;
@@ -99,10 +99,7 @@
 	new->name = strdup(name);
 	new->values = NULL;
 
-	if (*list == NULL)
-		new->next = NULL;
-	else
-		new->next = *list;
+	new->next = *list;
 	*list = new;
 
 	return new;
@@ -171,7 +168,7 @@
 	}
 }
 
-void univention_policy_merge(LDAP* ld, char *dn, univention_policy_handle_t* handle, char** object_classes)
+void univention_policy_merge(LDAP *ld, const char *dn, univention_policy_handle_t *handle, char **object_classes)
 {
 	int		rc;
 	LDAPMessage	*res;
@@ -327,7 +324,7 @@
 /*
  * reads policies for dn from conn
  */
-univention_policy_handle_t* univention_policy_open(LDAP* ld, char* base, char* dn)
+univention_policy_handle_t* univention_policy_open(LDAP* ld, const char *base, const char *dn)
 {
 	const char* pdn;
 	int rc;
@@ -356,7 +353,7 @@
 	timeout.tv_usec = 0;
 
 	for (pdn = dn; pdn != NULL; pdn = parent_dn(pdn)) {
-		char* filter;
+		const char *filter;
 
 		univention_debug(UV_DEBUG_POLICY, UV_DEBUG_INFO, "processing dn %s", pdn);
 
@@ -369,8 +366,7 @@
 			if ( rc == LDAP_NO_SUCH_OBJECT ) {
 				univention_debug(UV_DEBUG_LDAP, UV_DEBUG_WARN, "Not found");
 			} else if ( rc != LDAP_SUCCESS ) {
-				univention_debug(UV_DEBUG_LDAP, UV_DEBUG_ERROR,
-						"%s: %s", pdn, ldap_err2string(rc));
+				univention_debug(UV_DEBUG_LDAP, UV_DEBUG_ERROR, "%s: %s", pdn, ldap_err2string(rc));
 				univention_policy_close(handle);
 				return NULL;
 			}
@@ -449,7 +445,7 @@
 /*
  * returns values for policy/attribute
  */
-univention_policy_result_t* univention_policy_get(univention_policy_handle_t* handle, char* policy_name, char* attribute_name)
+univention_policy_result_t* univention_policy_get(univention_policy_handle_t* handle, const char *policy_name, const char *attribute_name)
 {
 	struct univention_policy_list_s* policy;
 	struct univention_policy_attribute_list_s* attribute;
Index: lib/ldap.c
===================================================================
--- lib/ldap.c	(Revision 24237)
+++ lib/ldap.c	(Arbeitskopie)
@@ -42,25 +42,19 @@
 #include <univention/ldap.h>
 #include <univention/debug.h>
 
+#define FREE(p) \
+	do { \
+		if (p != NULL) { \
+			free(p); \
+			p = NULL; \
+		} \
+	} while (0)
+
 univention_ldap_parameters_t* univention_ldap_new(void)
 {
 	univention_ldap_parameters_t* lp;
-	if ((lp = malloc(sizeof(univention_ldap_parameters_t))) == NULL)
+	if ((lp = calloc(1, sizeof(univention_ldap_parameters_t))) == NULL)
 		return NULL;
-	lp->ld = NULL;
-	lp->version = 0;
-	lp->host = NULL;
-	lp->port = 0;
-	lp->uri = NULL;
-	lp->start_tls = 0;
-	lp->base = NULL;
-	lp->binddn = NULL;
-	lp->bindpw = NULL;
-	lp->authmethod = 0;
-	lp->sasl_mech = NULL;
-	lp->sasl_realm = NULL;
-	lp->sasl_authcid = NULL;
-	lp->sasl_authzid = NULL;
 	return lp;
 }
 
@@ -113,50 +107,59 @@
 	return LDAP_SUCCESS;
 }
 
+#define _UNIVENTION_LDAP_SECRET_LEN_MAX 27
 int univention_ldap_set_admin_connection( univention_ldap_parameters_t *lp )
 {
 	FILE *secret;
-	char *base    = NULL;
+	char *base = NULL;
+	size_t len;
 
 	base = univention_config_get_string("ldap/base");
-	if ( !base ) {
-		return 1;
-	}
-	lp->binddn = malloc( ( strlen(base) + strlen("cn=admin,") + 1) * sizeof (char) );
-	if ( !lp->binddn ) {
+	if (!base)
+		goto err;
+	len = strlen(base) + strlen("cn=admin,") + 1;
+	lp->binddn = malloc(sizeof(char) * len);
+	if (!lp->binddn) {
 		free(base);
-		return 1;
+		goto err;
 	}
-	sprintf(lp->binddn, "cn=admin,%s", base );
+	snprintf(lp->binddn, len, "cn=admin,%s", base);
 
 	free(base);
 
 	secret = fopen("/etc/ldap.secret", "r" );
+	if (!secret)
+		goto err1;
 
-	if ( !secret ) {
-		return 1;
+	lp->bindpw = calloc(_UNIVENTION_LDAP_SECRET_LEN_MAX, sizeof(char));
+	if (!lp->bindpw) {
+		fclose(secret);
+		goto err1;
 	}
 
-	lp->bindpw = malloc(25*sizeof(char));
+	len = fread(lp->bindpw, _UNIVENTION_LDAP_SECRET_LEN_MAX, sizeof(char), secret);
+	if (ferror(secret))
+		len = -1;
+	fclose(secret);
 
-	if ( !lp->bindpw ) {
-		return 1;
+	for (; len >= 0; len--) {
+		switch (lp->bindpw[len]) {
+			case '\r':
+			case '\n':
+				lp->bindpw[len] = '\0';
+			case '\0':
+				continue;
+			default:
+				return 0;
+		}
 	}
 
-	memset(lp->bindpw, 0, 25);
-
-	fread(lp->bindpw, 24, 1, secret);
-
-	if ( lp->bindpw[strlen(lp->bindpw)-1] == '\r' ) {
-		lp->bindpw[strlen(lp->bindpw)-1] = '\0';
-	}
-	if ( lp->bindpw[strlen(lp->bindpw)-1] == '\n' ) {
-		lp->bindpw[strlen(lp->bindpw)-1] = '\0';
-	}
-
-	fclose(secret);
-
-	return 0;
+err2:
+	FREE(lp->bindpw);
+err1:
+	FREE(lp->binddn);
+err:
+	return 1;
 }
 
 int univention_ldap_open(univention_ldap_parameters_t *lp)
@@ -281,44 +284,14 @@
 		ldap_unbind_ext(lp->ld, NULL, NULL);
 		lp->ld = NULL;
 	}
-	if (lp->uri != NULL) {
-		free(lp->uri);
-		lp->uri = NULL;
-	}
-	if (lp->host != NULL) {
-		free(lp->host);
-		lp->host = NULL;
-	}
-	if (lp->base != NULL) {
-		free(lp->base);
-		lp->base = NULL;
-	}
-	if (lp->binddn != NULL) {
-		free(lp->binddn);
-		lp->binddn = NULL;
-	}
-	if (lp->bindpw != NULL) {
-		free(lp->bindpw);
-		lp->bindpw = NULL;
-	}
-	if (lp->sasl_mech != NULL) {
-		free(lp->sasl_mech);
-		lp->sasl_mech = NULL;
-	}
-	if (lp->sasl_realm != NULL) {
-		free(lp->sasl_realm);
-		lp->sasl_realm = NULL;
-	}
-	if (lp->sasl_authcid != NULL) {
-		free(lp->sasl_authcid);
-		lp->sasl_authcid = NULL;
-	}
-	if (lp->sasl_authzid != NULL) {
-		free(lp->sasl_authzid);
-		lp->sasl_authzid = NULL;
-	}
-	if (lp != NULL) {
-		free(lp);
-		lp = NULL;
-	}
+	FREE(lp->uri);
+	FREE(lp->host);
+	FREE(lp->base);
+	FREE(lp->binddn);
+	FREE(lp->bindpw);
+	FREE(lp->sasl_mech);
+	FREE(lp->sasl_realm);
+	FREE(lp->sasl_authcid);
+	FREE(lp->sasl_authzid);
+	FREE(lp);
 }
Index: lib/krb5.c
===================================================================
--- lib/krb5.c	(Revision 24237)
+++ lib/krb5.c	(Arbeitskopie)
@@ -44,11 +44,8 @@
 univention_krb5_parameters_t* univention_krb5_new(void)
 {
 	univention_krb5_parameters_t* kp;
-	if ((kp = malloc(sizeof(univention_krb5_parameters_t))) == NULL)
+	if ((kp = calloc(1, sizeof(univention_krb5_parameters_t))) == NULL)
 		return NULL;
-	kp->username = NULL;
-	kp->realm = NULL;
-	kp->password = NULL;
 	return kp;
 }
 
@@ -73,68 +70,69 @@
 
 int univention_krb5_init(univention_krb5_parameters_t *kp)
 {
-	krb5_error_code rv;
+	krb5_error_code rv = -1;
 	char *principal_name;
 
 	if (kp->username == NULL) {
-		struct passwd *pwd;
-		pwd = getpwuid(getuid());
-		if (pwd == NULL) {
-			return 1;
+		struct passwd pwd, *result;
+		char *buf;
+		size_t bufsize;
+		int s;
+
+		bufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
+		if (bufsize == -1)
+			bufsize = 16384;
+		buf = malloc(bufsize);
+		if (buf == NULL)
+			goto err;
 		}
-		kp->username = strdup(pwd->pw_name);
+		s = getpwnam_r(argv[1], &pwd, buf, bufsize, &result);
+		if (result != NULL)
+			kp->username = strdup(pwd.pw_name);
+		free(buf);
 	}
-	if (kp->realm == NULL) {
+
+	if (kp->realm == NULL)
 		kp->realm = univention_config_get_string("kerberos/realm");
-		if (kp->realm == NULL) {
-			return 1;
-		}
-	}
+
+	if (kp->username == NULL || kp->realm == NULL)
+		goto err;
 	asprintf(&principal_name, "%s@%s", kp->username, kp->realm);
+	if (principal_name == NULL)
+		goto err;
 
 	univention_debug(UV_DEBUG_KERBEROS, UV_DEBUG_INFO, "receiving Kerberos ticket for %s", principal_name);
 
-	if ((rv = krb5_init_context(&kp->context))) {
-		free(principal_name);
-		return rv;
-	}
-	if ((rv = krb5_cc_default(kp->context, &kp->ccache))) {
-		free(principal_name);
-		krb5_free_context(kp->context);
-		return rv;
-	}
-	if ((rv = krb5_parse_name(kp->context, principal_name, &kp->principal))) {
-		free(principal_name);
-		krb5_free_context(kp->context);
-		return rv;
-	}
+	if ((rv = krb5_init_context(&kp->context)))
+		goto err1;
+	if ((rv = krb5_cc_default(kp->context, &kp->ccache)))
+		goto err2;
+	if ((rv = krb5_parse_name(kp->context, principal_name, &kp->principal)))
+		goto err2;
 	if ((rv = krb5_get_init_creds_password(kp->context, &kp->creds, kp->principal,
-					NULL, kerb_prompter, kp->password, 0, NULL, NULL))) {
-		free(principal_name);
-		krb5_free_principal(kp->context, kp->principal);
-		krb5_free_context(kp->context);
-		return rv;
-	}
-	if ((rv = krb5_cc_initialize(kp->context, kp->ccache, kp->principal))) {
-		free(principal_name);
-		krb5_free_cred_contents(kp->context, &kp->creds);
-		krb5_free_principal(kp->context, kp->principal);
-		krb5_free_context(kp->context);
-		return rv;
-	}
-	if ((rv = krb5_cc_store_cred(kp->context, kp->ccache, &kp->creds))) {
-		free(principal_name);
-		krb5_cc_close(kp->context, kp->ccache);
-		krb5_free_cred_contents(kp->context, &kp->creds);
-		krb5_free_principal(kp->context, kp->principal);
-		krb5_free_context(kp->context);
-		return rv;
-	}
+					NULL, kerb_prompter, kp->password, 0, NULL, NULL)))
+		goto err3;
+	if ((rv = krb5_cc_initialize(kp->context, kp->ccache, kp->principal)))
+		goto err4;
+	if ((rv = krb5_cc_store_cred(kp->context, kp->ccache, &kp->creds)))
+		goto err5;
 
-	free(principal_name);
+	rv = 0;
+
+err5:
 	krb5_cc_close(kp->context, kp->ccache);
+	kp->ccache = NULL;
+err4:
 	krb5_free_cred_contents(kp->context, &kp->creds);
+	kp->creds = NULL;
+err3:
 	krb5_free_principal(kp->context, kp->principal);
+	kp->principal = NULL;
+err2:
 	krb5_free_context(kp->context);
-	return 0;
+	kp->context = NULL;
+err1:
+	free(principal_name);
+err:
+	return rv;
 }