Hello,
attached series of patches cleans code a little (mostly removes unused
variables) and makes locking inside bind-dyndb-ldap more readable and
understandable.
Most of the cleanup series gets rid of the ldap_connection->database
reference because there is a reference from "database" variable to
ldap_connection. This "double link" makes current locking
hard-to-understand.
I tested this series well, it shouldn't cause any regression.
Regards, Adam
>From 78875095f5c8df8b3b05334f9b7a1fc0ed3a8ef7 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 15:34:56 +0200
Subject: [PATCH 01/13] Remove unused lock from ldapdb_t.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_driver.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 9c1da40..e812038 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -64,7 +64,6 @@ static dns_rdatasetmethods_t rdataset_methods;
typedef struct {
dns_db_t common;
isc_refcount_t refs;
- isc_mutex_t lock; /* convert to isc_rwlock_t ? */
ldap_instance_t *ldap_inst;
ldap_cache_t *ldap_cache;
} ldapdb_t;
@@ -262,7 +261,6 @@ detach(dns_db_t **dbp)
static void
free_ldapdb(ldapdb_t *ldapdb)
{
- DESTROYLOCK(&ldapdb->lock);
dns_name_free(&ldapdb->common.origin, ldapdb->common.mctx);
isc_mem_putanddetach(&ldapdb->common.mctx, ldapdb, sizeof(*ldapdb));
}
@@ -1053,7 +1051,6 @@ ldapdb_create(isc_mem_t *mctx, dns_name_t *name,
dns_dbtype_t type,
{
ldapdb_t *ldapdb = NULL;
isc_result_t result;
- int lock_is_initialized = 0;
UNUSED(driverarg); /* Currently we don't need any data */
@@ -1072,9 +1069,6 @@ ldapdb_create(isc_mem_t *mctx, dns_name_t *name,
dns_dbtype_t type,
dns_name_init(&ldapdb->common.origin, NULL);
isc_ondestroy_init(&ldapdb->common.ondest);
- CHECK(isc_mutex_init(&ldapdb->lock));
- lock_is_initialized = 1;
-
ldapdb->common.magic = DNS_DB_MAGIC;
ldapdb->common.impmagic = LDAPDB_MAGIC;
@@ -1094,8 +1088,6 @@ ldapdb_create(isc_mem_t *mctx, dns_name_t *name,
dns_dbtype_t type,
cleanup:
if (ldapdb != NULL) {
- if (lock_is_initialized)
- DESTROYLOCK(&ldapdb->lock);
if (dns_name_dynamic(&ldapdb->common.origin))
dns_name_free(&ldapdb->common.origin, mctx);
--
1.7.5.1
>From 5421f54bf46e457c48b8e5667147acab5c15476e Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 15:40:06 +0200
Subject: [PATCH 02/13] Use LOCK instead of CONTROLED_LOCK in
destroy_ldap_cache.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/cache.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index f0d2153..db6457b 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -143,17 +143,16 @@ void
destroy_ldap_cache(ldap_cache_t **cachep)
{
ldap_cache_t *cache;
- int is_locked = 0;
REQUIRE(cachep != NULL && *cachep != NULL);
cache = *cachep;
if (cache->rbt) {
- CONTROLED_LOCK(&cache->mutex);
+ LOCK(&cache->mutex);
dns_rbt_destroy(&cache->rbt);
cache->rbt = NULL;
- CONTROLED_UNLOCK(&cache->mutex);
+ UNLOCK(&cache->mutex);
DESTROYLOCK(&cache->mutex);
}
--
1.7.5.1
>From 7d6b822f420a683398bb533e31344d82c4245a06 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 16:03:59 +0200
Subject: [PATCH 03/13] Remove ldap_connection->database->mctx refs, use
directly ldap_connection->mctx.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 0689e00..5c3fa6d 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -206,7 +206,8 @@ const ldap_auth_pair_t supported_ldap_auth[] = {
/* TODO: reorganize this stuff & clean it up. */
static isc_result_t new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp);
-static void destroy_ldap_connection(ldap_connection_t **ldap_connp);
+static void destroy_ldap_connection(ldap_instance_t *ldap_inst,
+ ldap_connection_t **ldap_connp);
static isc_result_t findrdatatype_or_create(isc_mem_t *mctx,
ldapdb_rdatalist_t *rdatalist, ldap_entry_t *entry,
@@ -415,9 +416,9 @@ retry:
/* If the credentials are invalid, try passwordless login. */
if (result == ISC_R_NOPERM
&& ldap_inst->auth_method != AUTH_NONE) {
- destroy_ldap_connection(&ldap_conn);
+ destroy_ldap_connection(ldap_inst, &ldap_conn);
FOR_EACH_UNLINK(ldap_conn, ldap_inst->conn_list) {
- destroy_ldap_connection(&ldap_conn);
+ destroy_ldap_connection(ldap_inst, &ldap_conn);
} END_FOR_EACH_UNLINK(ldap_conn);
ldap_inst->auth_method = AUTH_NONE;
log_debug(2, "falling back to password-less login");
@@ -457,7 +458,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
while (elem != NULL) {
next = NEXT(elem, link);
UNLINK(ldap_inst->conn_list, elem, link);
- destroy_ldap_connection(&elem);
+ destroy_ldap_connection(ldap_inst, &elem);
elem = next;
}
@@ -521,13 +522,13 @@ new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp)
return ISC_R_SUCCESS;
cleanup:
- destroy_ldap_connection(&ldap_conn);
+ destroy_ldap_connection(ldap_inst, &ldap_conn);
return result;
}
static void
-destroy_ldap_connection(ldap_connection_t **ldap_connp)
+destroy_ldap_connection(ldap_instance_t *ldap_inst, ldap_connection_t
**ldap_connp)
{
ldap_connection_t *ldap_conn;
@@ -544,11 +545,11 @@ destroy_ldap_connection(ldap_connection_t **ldap_connp)
if (ldap_conn->lex != NULL)
isc_lex_destroy(&ldap_conn->lex);
if (ldap_conn->rdata_target_mem != NULL) {
- isc_mem_put(ldap_conn->database->mctx,
+ isc_mem_put(ldap_inst->mctx,
ldap_conn->rdata_target_mem, MINTSIZ);
}
- isc_mem_put(ldap_conn->database->mctx, *ldap_connp,
sizeof(ldap_connection_t));
+ isc_mem_put(ldap_inst->mctx, *ldap_connp, sizeof(ldap_connection_t));
*ldap_connp = NULL;
}
--
1.7.5.1
>From 235afee8f2f844dd1084bae46046c7066a1ccfae Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 16:56:02 +0200
Subject: [PATCH 04/13] Don't use ldap_connection->database->fake_mnape
reference in code.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 5c3fa6d..e725cdc 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -214,7 +214,7 @@ static isc_result_t findrdatatype_or_create(isc_mem_t *mctx,
dns_rdatatype_t rdtype, dns_rdatalist_t **rdlistp);
static isc_result_t add_soa_record(isc_mem_t *mctx, ldap_connection_t
*ldap_conn,
dns_name_t *origin, ldap_entry_t *entry,
- ldapdb_rdatalist_t *rdatalist);
+ ldapdb_rdatalist_t *rdatalist, ld_string_t *fake_mname);
static dns_rdataclass_t get_rdataclass(ldap_entry_t *ldap_entry);
static dns_ttl_t get_ttl(ldap_entry_t *ldap_entry);
static isc_result_t get_values(const ldap_entry_t *entry,
@@ -902,7 +902,7 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *na
entry = NEXT(entry, link)) {
result = add_soa_record(mctx, ldap_conn, origin, entry,
- rdatalist);
+ rdatalist, ldap_inst->fake_mname);
if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND)
goto cleanup;
@@ -977,8 +977,7 @@ get_ttl(ldap_entry_t *entry)
}
static isc_result_t
-get_soa_record(ldap_connection_t *ldap_conn, ldap_entry_t *entry,
- ld_string_t *target)
+get_soa_record(ldap_entry_t *entry, ld_string_t *fake_mname, ld_string_t
*target)
{
isc_result_t result = ISC_R_NOTFOUND;
ldap_value_list_t values;
@@ -994,9 +993,9 @@ get_soa_record(ldap_connection_t *ldap_conn, ldap_entry_t
*entry,
REQUIRE(target != NULL);
str_clear(target);
- if (str_len(ldap_conn->database->fake_mname) > 0) {
+ if (str_len(fake_mname) > 0) {
i = 1;
- CHECK(str_cat(target, ldap_conn->database->fake_mname));
+ CHECK(str_cat(target, fake_mname));
CHECK(str_cat_char(target, " "));
}
for (; soa_attrs[i] != NULL; i++) {
@@ -1011,7 +1010,8 @@ cleanup:
static isc_result_t
add_soa_record(isc_mem_t *mctx, ldap_connection_t *ldap_conn, dns_name_t
*origin,
- ldap_entry_t *entry, ldapdb_rdatalist_t *rdatalist)
+ ldap_entry_t *entry, ldapdb_rdatalist_t *rdatalist,
+ ld_string_t *fake_mname)
{
isc_result_t result;
ld_string_t *string = NULL;
@@ -1021,7 +1021,7 @@ add_soa_record(isc_mem_t *mctx, ldap_connection_t
*ldap_conn, dns_name_t *origin
CHECK(str_new(mctx, &string));
- CHECK(get_soa_record(ldap_conn, entry, string));
+ CHECK(get_soa_record(entry, fake_mname, string));
rdclass = get_rdataclass(entry);
CHECK(parse_rdata(mctx, ldap_conn, rdclass, dns_rdatatype_soa, origin,
str_buf(string), &rdata));
--
1.7.5.1
>From 01e14293d6c407778801408085f2bb80c1571f83 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 17:05:26 +0200
Subject: [PATCH 05/13] Don't reference
ldap_connection->database->conn_semaphore directly.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index e725cdc..584c452 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -250,7 +250,8 @@ static const char *get_dn(ldap_connection_t *inst);
#endif
static ldap_connection_t * get_connection(ldap_instance_t *ldap_inst);
-static void put_connection(ldap_connection_t *ldap_conn);
+static void put_connection(ldap_instance_t *ldap_inst,
+ ldap_connection_t *ldap_conn);
static isc_result_t ldap_connect(ldap_connection_t *ldap_conn);
static isc_result_t ldap_reconnect(ldap_connection_t *ldap_conn);
static int handle_connection_error(ldap_connection_t *ldap_conn,
@@ -723,7 +724,7 @@ next:
}
cleanup:
- put_connection(ldap_conn);
+ put_connection(ldap_inst, ldap_conn);
log_debug(2, "finished refreshing list of zones");
@@ -930,7 +931,7 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *na
result = ISC_R_SUCCESS;
cleanup:
- put_connection(ldap_conn);
+ put_connection(ldap_inst, ldap_conn);
str_destroy(&string);
if (result != ISC_R_SUCCESS)
@@ -1240,7 +1241,7 @@ get_connection(ldap_instance_t *ldap_inst)
}
static void
-put_connection(ldap_connection_t *ldap_conn)
+put_connection(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn)
{
if (ldap_conn == NULL)
return;
@@ -1269,7 +1270,7 @@ put_connection(ldap_connection_t *ldap_conn)
free_query_cache(ldap_conn);
UNLOCK(&ldap_conn->lock);
- semaphore_signal(&ldap_conn->database->conn_semaphore);
+ semaphore_signal(&ldap_inst->conn_semaphore);
}
@@ -2041,7 +2042,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t
*ldap_inst,
CHECK(ldap_modify_do(ldap_conn, str_buf(owner_dn), change,
delete_node));
cleanup:
- put_connection(ldap_conn);
+ put_connection(ldap_inst, ldap_conn);
str_destroy(&owner_dn);
free_ldapmod(mctx, &change[0]);
free_ldapmod(mctx, &change[1]);
--
1.7.5.1
>From 8438b60823637f2702abd7db0e84250bd7427e73 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 17:14:50 +0200
Subject: [PATCH 06/13] ldap_connection_t has now it's own reference to memory
context.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 79 ++++++++++++++++++++++++++++-------------------------
1 files changed, 42 insertions(+), 37 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 584c452..9d7c9c3 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -139,6 +139,7 @@ struct ldap_instance {
};
struct ldap_connection {
+ isc_mem_t *mctx;
ldap_instance_t *database;
isc_mutex_t lock;
LINK(ldap_connection_t) link;
@@ -512,11 +513,13 @@ new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp)
return result;
}
- CHECK(str_new(ldap_inst->mctx, &ldap_conn->query_string));
- CHECK(str_new(ldap_inst->mctx, &ldap_conn->base));
+ isc_mem_attach(ldap_inst->mctx, &ldap_conn->mctx);
+
+ CHECK(str_new(ldap_conn->mctx, &ldap_conn->query_string));
+ CHECK(str_new(ldap_conn->mctx, &ldap_conn->base));
- CHECK(isc_lex_create(ldap_inst->mctx, TOKENSIZ, &ldap_conn->lex));
- CHECKED_MEM_GET(ldap_inst->mctx, ldap_conn->rdata_target_mem, MINTSIZ);
+ CHECK(isc_lex_create(ldap_conn->mctx, TOKENSIZ, &ldap_conn->lex));
+ CHECKED_MEM_GET(ldap_conn->mctx, ldap_conn->rdata_target_mem, MINTSIZ);
*ldap_connp = ldap_conn;
@@ -546,10 +549,12 @@ destroy_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_con
if (ldap_conn->lex != NULL)
isc_lex_destroy(&ldap_conn->lex);
if (ldap_conn->rdata_target_mem != NULL) {
- isc_mem_put(ldap_inst->mctx,
+ isc_mem_put(ldap_conn->mctx,
ldap_conn->rdata_target_mem, MINTSIZ);
}
+ isc_mem_detach(&ldap_conn->mctx);
+
isc_mem_put(ldap_inst->mctx, *ldap_connp, sizeof(ldap_connection_t));
*ldap_connp = NULL;
}
@@ -1318,7 +1323,7 @@ ldap_query(ldap_connection_t *ldap_conn, const char
*base, int scope, char **att
}
static isc_result_t
-cache_query_results(ldap_connection_t *inst)
+cache_query_results(ldap_connection_t *conn)
{
isc_result_t result;
LDAP *ld;
@@ -1326,42 +1331,42 @@ cache_query_results(ldap_connection_t *inst)
LDAPMessage *entry;
ldap_entry_t *ldap_entry;
- REQUIRE(inst != NULL);
- REQUIRE(EMPTY(inst->ldap_entries));
- REQUIRE(inst->result != NULL);
+ REQUIRE(conn != NULL);
+ REQUIRE(EMPTY(conn->ldap_entries));
+ REQUIRE(conn->result != NULL);
- INIT_LIST(inst->ldap_entries);
+ INIT_LIST(conn->ldap_entries);
- if (inst->cache_active)
- free_query_cache(inst);
+ if (conn->cache_active)
+ free_query_cache(conn);
- ld = inst->handle;
- res = inst->result;
+ ld = conn->handle;
+ res = conn->result;
for (entry = ldap_first_entry(ld, res);
entry != NULL;
entry = ldap_next_entry(ld, entry)) {
- CHECKED_MEM_GET_PTR(inst->database->mctx, ldap_entry);
+ CHECKED_MEM_GET_PTR(conn->mctx, ldap_entry);
ZERO_PTR(ldap_entry);
ldap_entry->entry = entry;
INIT_LIST(ldap_entry->attributes);
INIT_LINK(ldap_entry, link);
- CHECK(fill_ldap_entry(inst, ldap_entry));
+ CHECK(fill_ldap_entry(conn, ldap_entry));
- APPEND(inst->ldap_entries, ldap_entry, link);
+ APPEND(conn->ldap_entries, ldap_entry, link);
}
return ISC_R_SUCCESS;
cleanup:
- free_query_cache(inst);
+ free_query_cache(conn);
return result;
}
static isc_result_t
-fill_ldap_entry(ldap_connection_t *inst, ldap_entry_t *ldap_entry)
+fill_ldap_entry(ldap_connection_t *conn, ldap_entry_t *ldap_entry)
{
isc_result_t result;
ldap_attribute_t *ldap_attr;
@@ -1369,22 +1374,22 @@ fill_ldap_entry(ldap_connection_t *inst, ldap_entry_t
*ldap_entry)
BerElement *ber;
LDAPMessage *entry;
- REQUIRE(inst != NULL);
+ REQUIRE(conn != NULL);
REQUIRE(ldap_entry != NULL);
result = ISC_R_SUCCESS;
entry = ldap_entry->entry;
- for (attribute = ldap_first_attribute(inst->handle, entry, &ber);
+ for (attribute = ldap_first_attribute(conn->handle, entry, &ber);
attribute != NULL;
- attribute = ldap_next_attribute(inst->handle, entry, ber)) {
- CHECKED_MEM_GET_PTR(inst->database->mctx, ldap_attr);
+ attribute = ldap_next_attribute(conn->handle, entry, ber)) {
+ CHECKED_MEM_GET_PTR(conn->mctx, ldap_attr);
ZERO_PTR(ldap_attr);
ldap_attr->name = attribute;
INIT_LIST(ldap_attr->values);
INIT_LINK(ldap_attr, link);
- CHECK(fill_ldap_attribute(inst, ldap_attr));
+ CHECK(fill_ldap_attribute(conn, ldap_attr));
APPEND(ldap_entry->attributes, ldap_attr, link);
}
@@ -1394,23 +1399,23 @@ fill_ldap_entry(ldap_connection_t *inst, ldap_entry_t
*ldap_entry)
cleanup:
if (result != ISC_R_SUCCESS) {
- free_ldap_attributes(inst->database->mctx, ldap_entry);
+ free_ldap_attributes(conn->mctx, ldap_entry);
}
return result;
}
static isc_result_t
-fill_ldap_attribute(ldap_connection_t *inst, ldap_attribute_t *ldap_attr)
+fill_ldap_attribute(ldap_connection_t *conn, ldap_attribute_t *ldap_attr)
{
isc_result_t result;
char **values;
ldap_value_t *ldap_val;
- REQUIRE(inst != NULL);
+ REQUIRE(conn != NULL);
REQUIRE(ldap_attr != NULL);
- values = ldap_get_values(inst->handle, inst->result, ldap_attr->name);
+ values = ldap_get_values(conn->handle, conn->result, ldap_attr->name);
/* TODO: proper ldap error handling */
if (values == NULL)
return ISC_R_FAILURE;
@@ -1418,7 +1423,7 @@ fill_ldap_attribute(ldap_connection_t *inst,
ldap_attribute_t *ldap_attr)
ldap_attr->ldap_values = values;
for (unsigned int i = 0; values[i] != NULL; i++) {
- CHECKED_MEM_GET_PTR(inst->database->mctx, ldap_val);
+ CHECKED_MEM_GET_PTR(conn->mctx, ldap_val);
ldap_val->value = values[i];
INIT_LINK(ldap_val, link);
@@ -1428,29 +1433,29 @@ fill_ldap_attribute(ldap_connection_t *inst,
ldap_attribute_t *ldap_attr)
return ISC_R_SUCCESS;
cleanup:
- free_ldap_values(inst->database->mctx, ldap_attr);
+ free_ldap_values(conn->mctx, ldap_attr);
ldap_value_free(values);
return result;
}
static void
-free_query_cache(ldap_connection_t *inst)
+free_query_cache(ldap_connection_t *conn)
{
ldap_entry_t *entry, *next;
- entry = HEAD(inst->ldap_entries);
+ entry = HEAD(conn->ldap_entries);
while (entry != NULL) {
next = NEXT(entry, link);
- UNLINK(inst->ldap_entries, entry, link);
- free_ldap_attributes(inst->database->mctx, entry);
+ UNLINK(conn->ldap_entries, entry, link);
+ free_ldap_attributes(conn->mctx, entry);
if (entry->dn != NULL)
ldap_memfree(entry->dn);
- isc_mem_put(inst->database->mctx, entry, sizeof(*entry));
+ isc_mem_put(conn->mctx, entry, sizeof(*entry));
entry = next;
}
- inst->cache_active = isc_boolean_false;
+ conn->cache_active = isc_boolean_false;
}
static void
@@ -1978,7 +1983,7 @@ static isc_result_t
modify_soa_record(ldap_connection_t *ldap_conn, const char *zone_dn,
dns_rdata_t *rdata)
{
- isc_mem_t *mctx = ldap_conn->database->mctx;
+ isc_mem_t *mctx = ldap_conn->mctx;
dns_rdata_soa_t soa;
LDAPMod change[5];
LDAPMod *changep[6] = {
--
1.7.5.1
>From 17ce678dc3f97bffd041762a1bcd331c56b99f0f Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 17:29:36 +0200
Subject: [PATCH 07/13] Remove the rest of ldap_connection->database
references.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 49 +++++++++++++++++++++++--------------------------
1 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 9d7c9c3..52e73c6 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -140,7 +140,6 @@ struct ldap_instance {
struct ldap_connection {
isc_mem_t *mctx;
- ldap_instance_t *database;
isc_mutex_t lock;
LINK(ldap_connection_t) link;
ld_string_t *query_string;
@@ -253,11 +252,14 @@ static const char *get_dn(ldap_connection_t *inst);
static ldap_connection_t * get_connection(ldap_instance_t *ldap_inst);
static void put_connection(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn);
-static isc_result_t ldap_connect(ldap_connection_t *ldap_conn);
-static isc_result_t ldap_reconnect(ldap_connection_t *ldap_conn);
-static int handle_connection_error(ldap_connection_t *ldap_conn,
- isc_result_t *result);
-static isc_result_t ldap_query(ldap_connection_t *ldap_conn, const char *base,
+static isc_result_t ldap_connect(ldap_instance_t *ldap_inst,
+ ldap_connection_t *ldap_conn);
+static isc_result_t ldap_reconnect(ldap_instance_t *ldap_inst,
+ ldap_connection_t *ldap_conn);
+static int handle_connection_error(ldap_instance_t *ldap_inst,
+ ldap_connection_t *ldap_conn, isc_result_t *result);
+static isc_result_t ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t
*ldap_conn,
+ const char *base,
int scope, char **attrs, int attrsonly, const char *filter,
...);
/* Functions for writing to LDAP. */
@@ -414,7 +416,7 @@ retry:
for (i = 0; i < ldap_inst->connections; i++) {
ldap_conn = NULL;
CHECK(new_ldap_connection(ldap_inst, &ldap_conn));
- result = ldap_connect(ldap_conn);
+ result = ldap_connect(ldap_inst, ldap_conn);
/* If the credentials are invalid, try passwordless login. */
if (result == ISC_R_NOPERM
&& ldap_inst->auth_method != AUTH_NONE) {
@@ -505,7 +507,6 @@ new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp)
ZERO_PTR(ldap_conn);
- ldap_conn->database = ldap_inst;
INIT_LINK(ldap_conn, link);
result = isc_mutex_init(&ldap_conn->lock);
if (result != ISC_R_SUCCESS) {
@@ -658,7 +659,7 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst,
isc_boolean_t create)
ldap_conn = get_connection(ldap_inst);
- CHECK(ldap_query(ldap_conn, str_buf(ldap_inst->base),
+ CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(ldap_inst->base),
LDAP_SCOPE_SUBTREE, attrs, 0,
"(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
CHECK(cache_query_results(ldap_conn));
@@ -894,8 +895,8 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t
*ldap_inst, dns_name_t *na
CHECK(str_new(mctx, &string));
CHECK(dnsname_to_dn(ldap_inst->zone_register, name, string));
- CHECK(ldap_query(ldap_conn, str_buf(string), LDAP_SCOPE_BASE, NULL, 0,
- "(objectClass=idnsRecord)"));
+ CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(string), LDAP_SCOPE_BASE,
+ NULL, 0, "(objectClass=idnsRecord)"));
CHECK(cache_query_results(ldap_conn));
if (EMPTY(ldap_conn->ldap_entries)) {
@@ -1280,7 +1281,8 @@ put_connection(ldap_instance_t *ldap_inst,
ldap_connection_t *ldap_conn)
static isc_result_t
-ldap_query(ldap_connection_t *ldap_conn, const char *base, int scope, char
**attrs,
+ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
+ const char *base, int scope, char **attrs,
int attrsonly, const char *filter, ...)
{
va_list ap;
@@ -1317,7 +1319,7 @@ ldap_query(ldap_connection_t *ldap_conn, const char
*base, int scope, char **att
return ISC_R_SUCCESS;
}
- } while (handle_connection_error(ldap_conn, &result));
+ } while (handle_connection_error(ldap_inst, ldap_conn, &result));
return result;
}
@@ -1563,21 +1565,18 @@ ldap_sasl_interact(LDAP *ld, unsigned flags, void
*defaults, void *sin)
/*
* Initialize the LDAP handle and bind to the server. Needed authentication
- * credentials and settings are available from the ldap_conn->database.
+ * credentials and settings are available from the ldap_inst.
*/
static isc_result_t
-ldap_connect(ldap_connection_t *ldap_conn)
+ldap_connect(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn)
{
LDAP *ld;
int ret;
int version;
- ldap_instance_t *ldap_inst;
struct timeval timeout;
REQUIRE(ldap_conn != NULL);
- ldap_inst = ldap_conn->database;
-
ret = ldap_initialize(&ld, str_buf(ldap_inst->uri));
if (ret != LDAP_SUCCESS) {
log_error("LDAP initialization failed: %s",
@@ -1589,7 +1588,7 @@ ldap_connect(ldap_connection_t *ldap_conn)
ret = ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &version);
LDAP_OPT_CHECK(ret, "failed to set LDAP version");
- timeout.tv_sec = ldap_conn->database->timeout;
+ timeout.tv_sec = ldap_inst->timeout;
timeout.tv_usec = 0;
ret = ldap_set_option(ld, LDAP_OPT_TIMEOUT, &timeout);
@@ -1599,7 +1598,7 @@ ldap_connect(ldap_connection_t *ldap_conn)
ldap_unbind_ext_s(ldap_conn->handle, NULL, NULL);
ldap_conn->handle = ld;
- return ldap_reconnect(ldap_conn);
+ return ldap_reconnect(ldap_inst, ldap_conn);
cleanup:
@@ -1610,15 +1609,12 @@ cleanup:
}
static isc_result_t
-ldap_reconnect(ldap_connection_t *ldap_conn)
+ldap_reconnect(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn)
{
int ret = 0;
- ldap_instance_t *ldap_inst;
const char *bind_dn = NULL;
const char *password = NULL;
- ldap_inst = ldap_conn->database;
-
if (ldap_conn->tries > 0) {
isc_time_t now;
int time_cmp;
@@ -1709,7 +1705,8 @@ ldap_reconnect(ldap_connection_t *ldap_conn)
}
static int
-handle_connection_error(ldap_connection_t *ldap_conn, isc_result_t *result)
+handle_connection_error(ldap_instance_t *ldap_inst, ldap_connection_t
*ldap_conn,
+ isc_result_t *result)
{
int ret;
int err_code;
@@ -1729,7 +1726,7 @@ handle_connection_error(ldap_connection_t *ldap_conn,
isc_result_t *result)
} else if (err_code == LDAP_SERVER_DOWN || err_code ==
LDAP_CONNECT_ERROR) {
if (ldap_conn->tries == 0)
log_error("connection to the LDAP server was lost");
- if (ldap_connect(ldap_conn) == ISC_R_SUCCESS)
+ if (ldap_connect(ldap_inst, ldap_conn) == ISC_R_SUCCESS)
return 1;
} else if (err_code == LDAP_TIMEOUT) {
log_error("LDAP query timed out. Try to adjust \"timeout\"
parameter");
--
1.7.5.1
>From c663fd3d30553a7b1c74242d4774912565499232 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 17:57:06 +0200
Subject: [PATCH 08/13] Store connections to LDAP in array instead of in list
inside ldap_instance_t.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 45 ++++++++++++++++++++++++---------------------
src/util.h | 15 ---------------
2 files changed, 24 insertions(+), 36 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 52e73c6..843c3c0 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -110,8 +110,9 @@ struct ldap_instance {
dns_zonemgr_t *zmgr;
/* List of LDAP connections. */
+ unsigned int connections; /* number of connections */
semaphore_t conn_semaphore;
- LIST(ldap_connection_t) conn_list;
+ ldap_connection_t **conns;
/* Our own list of zones. */
zone_register_t *zone_register;
@@ -122,7 +123,6 @@ struct ldap_instance {
/* Settings. */
ld_string_t *uri;
ld_string_t *base;
- unsigned int connections;
unsigned int reconnect_interval;
unsigned int timeout;
ldap_auth_t auth_method;
@@ -141,7 +141,6 @@ struct ldap_instance {
struct ldap_connection {
isc_mem_t *mctx;
isc_mutex_t lock;
- LINK(ldap_connection_t) link;
ld_string_t *query_string;
ld_string_t *base;
@@ -322,8 +321,6 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
/* commented out for now, cause named to hang */
//dns_view_attach(view, &ldap_inst->view);
- INIT_LIST(ldap_inst->conn_list);
-
CHECK(zr_create(mctx, &ldap_inst->zone_register));
CHECK(isc_mutex_init(&ldap_inst->kinit_lock));
@@ -411,6 +408,9 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
}
CHECK(semaphore_init(&ldap_inst->conn_semaphore,
ldap_inst->connections));
+ CHECKED_MEM_GET(mctx, ldap_inst->conns,
+ ldap_inst->connections * sizeof(ldap_connection_t *));
+ memset(ldap_inst->conns, 0, ldap_inst->connections *
sizeof(ldap_connection_t *));
retry:
for (i = 0; i < ldap_inst->connections; i++) {
@@ -421,9 +421,12 @@ retry:
if (result == ISC_R_NOPERM
&& ldap_inst->auth_method != AUTH_NONE) {
destroy_ldap_connection(ldap_inst, &ldap_conn);
- FOR_EACH_UNLINK(ldap_conn, ldap_inst->conn_list) {
- destroy_ldap_connection(ldap_inst, &ldap_conn);
- } END_FOR_EACH_UNLINK(ldap_conn);
+ /* It's safe to reuse "i" here */
+ for (i = 0; i < ldap_inst->connections; i++) {
+ ldap_conn = ldap_inst->conns[i];
+ if (ldap_conn != NULL)
+ destroy_ldap_connection(ldap_inst,
&ldap_conn);
+ }
ldap_inst->auth_method = AUTH_NONE;
log_debug(2, "falling back to password-less login");
goto retry;
@@ -433,7 +436,7 @@ retry:
} else if (result != ISC_R_SUCCESS) {
goto cleanup;
}
- APPEND(ldap_inst->conn_list, ldap_conn, link);
+ ldap_inst->conns[i] = ldap_conn;
}
cleanup:
@@ -451,21 +454,22 @@ void
destroy_ldap_instance(ldap_instance_t **ldap_instp)
{
ldap_instance_t *ldap_inst;
- ldap_connection_t *elem;
- ldap_connection_t *next;
+ ldap_connection_t *ldap_conn;
+ unsigned int i;
REQUIRE(ldap_instp != NULL && *ldap_instp != NULL);
ldap_inst = *ldap_instp;
- elem = HEAD(ldap_inst->conn_list);
- while (elem != NULL) {
- next = NEXT(elem, link);
- UNLINK(ldap_inst->conn_list, elem, link);
- destroy_ldap_connection(ldap_inst, &elem);
- elem = next;
+ for (i = 0; i < ldap_inst->connections; i++) {
+ ldap_conn = ldap_inst->conns[i];
+ if (ldap_conn != NULL)
+ destroy_ldap_connection(ldap_inst, &ldap_conn);
}
+ SAFE_MEM_PUT(ldap_inst->mctx, ldap_inst->conns,
+ ldap_inst->connections * sizeof(ldap_connection_t *));
+
str_destroy(&ldap_inst->uri);
str_destroy(&ldap_inst->base);
str_destroy(&ldap_inst->bind_dn);
@@ -507,7 +511,6 @@ new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp)
ZERO_PTR(ldap_conn);
- INIT_LINK(ldap_conn, link);
result = isc_mutex_init(&ldap_conn->lock);
if (result != ISC_R_SUCCESS) {
isc_mem_put(ldap_inst->mctx, ldap_inst,
sizeof(ldap_connection_t));
@@ -1226,15 +1229,15 @@ static ldap_connection_t *
get_connection(ldap_instance_t *ldap_inst)
{
ldap_connection_t *ldap_conn;
+ unsigned int i;
REQUIRE(ldap_inst != NULL);
semaphore_wait(&ldap_inst->conn_semaphore);
- ldap_conn = HEAD(ldap_inst->conn_list);
- while (ldap_conn != NULL) {
+ for (i = 0; i < ldap_inst->connections; i++) {
+ ldap_conn = ldap_inst->conns[i];
if (isc_mutex_trylock(&ldap_conn->lock) == ISC_R_SUCCESS)
break;
- ldap_conn = NEXT(ldap_conn, link);
}
RUNTIME_CHECK(ldap_conn != NULL);
diff --git a/src/util.h b/src/util.h
index 0f146a2..9e48ae8 100644
--- a/src/util.h
+++ b/src/util.h
@@ -99,19 +99,4 @@
dns_name_setbuffer(&name, &name##__buffer); \
} while (0)
-#define FOR_EACH(elt, list) \
- for ((elt) = HEAD(list); (elt) != NULL; (elt) = NEXT(elt, link))
-
-#define FOR_EACH_UNLINK(elt, list) \
- do { \
- typeof(elt) __next_elt; \
- for (elt = HEAD(list); elt != NULL; elt = NEXT(elt, link)) { \
- __next_elt = NEXT(elt, link); \
- UNLINK(list, elt, link);
-
-#define END_FOR_EACH_UNLINK(elt) \
- elt = __next_elt; \
- } \
- } while (0)
-
#endif /* !_LD_UTIL_H_ */
--
1.7.5.1
>From df01b0dbac3686c39ab6a5b843482faa8c26fc90 Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Thu, 12 May 2011 18:07:10 +0200
Subject: [PATCH 09/13] Remove unused ldap_connection.base variable.
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 843c3c0..ccae430 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -142,7 +142,6 @@ struct ldap_connection {
isc_mem_t *mctx;
isc_mutex_t lock;
ld_string_t *query_string;
- ld_string_t *base;
LDAP *handle;
LDAPMessage *result;
@@ -520,7 +519,6 @@ new_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_connp)
isc_mem_attach(ldap_inst->mctx, &ldap_conn->mctx);
CHECK(str_new(ldap_conn->mctx, &ldap_conn->query_string));
- CHECK(str_new(ldap_conn->mctx, &ldap_conn->base));
CHECK(isc_lex_create(ldap_conn->mctx, TOKENSIZ, &ldap_conn->lex));
CHECKED_MEM_GET(ldap_conn->mctx, ldap_conn->rdata_target_mem, MINTSIZ);
@@ -548,7 +546,6 @@ destroy_ldap_connection(ldap_instance_t *ldap_inst,
ldap_connection_t **ldap_con
ldap_unbind_ext_s(ldap_conn->handle, NULL, NULL);
str_destroy(&ldap_conn->query_string);
- str_destroy(&ldap_conn->base);
if (ldap_conn->lex != NULL)
isc_lex_destroy(&ldap_conn->lex);
@@ -1243,8 +1240,6 @@ get_connection(ldap_instance_t *ldap_inst)
RUNTIME_CHECK(ldap_conn != NULL);
INIT_LIST(ldap_conn->ldap_entries);
- /* TODO: find a clever way to not really require this */
- str_copy(ldap_conn->base, ldap_inst->base);
return ldap_conn;
}
--
1.7.5.1
>From 4155e949a00f43a6eee178f058122c99254fb85d Mon Sep 17 00:00:00 2001
From: Adam Tkac <at...@redhat.com>
Date: Fri, 13 May 2011 00:22:57 +0200
Subject: [PATCH 10/13] Add note about locking in ldap_helper.c
Signed-off-by: Adam Tkac <at...@redhat.com>
---
src/ldap_helper.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index ccae430..8f19d31 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -79,6 +79,20 @@
* LDAP related typedefs and structs.
*/
+/*
+ * Note about locking in this source.
+ *
+ * ldap_instance_t structure is equal to dynamic-db {}; statement in
named.conf.
+ * Attributes in ldap_instance_t can be modified only in new_ldap_instance
+ * function, which means server is started or reloaded.
+ *
+ * ldap_connection_t structure represents connection to the LDAP database and
+ * per-connection specific data. Access is controlled via
+ * ldap_connection_t->lock and ldap_instance_t->conn_semaphore. Each read
+ * or write access to ldap_connection_t structure (except create/destroy)
+ * must acquire the semaphore and the lock.
+ */
+
typedef struct ldap_connection ldap_connection_t;
typedef struct ldap_auth_pair ldap_auth_pair_t;
typedef struct settings settings_t;
--
1.7.5.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
