[Freeipa-devel] [PATCH 0088] Flush BIND caches if forwarder configuration was changed

[Petr Spacek]

On 11/08/2012 04:33 PM, Adam Tkac wrote:
> On Tue, Nov 06, 2012 at 03:02:37PM +0100, Petr Spacek wrote:
>> >Hello,
>> >
>> >     Flush BIND caches if forwarder configuration was changed.
>> >
>> >     Cache will not be flushed if old and new configurations are equal.
>> >     Without this optimization cache would be flushed during each zone refresh.
> Ack, just please check my comment below.
>
> Regards, Adam
>
>> > From faed16b4861d4263ed942608d3767324fc2fae88 Mon Sep 17 00:00:00 2001
>> >From: Petr Spacek<pspacek at redhat.com>
>> >Date: Tue, 6 Nov 2012 14:53:02 +0100
>> >Subject: [PATCH] Flush BIND caches if forwarder configuration was changed.
>> >
>> >Cache will not be flushed if old and new configurations are equal.
>> >Without this optimization cache would be flushed during each zone refresh.
>> >
>> >Signed-off-by: Petr Spacek<pspacek at redhat.com>
>> >---
>> >  src/ldap_helper.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-----------
>> >  1 file changed, 54 insertions(+), 13 deletions(-)
>> >
>> >diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>> >index 4f004515f513ecf6459b3bddfbc5474fe3cfabd2..b36892b4e8180fb2a5f335e3fa1b5589dae8bf14 100644
>> >--- a/src/ldap_helper.c
>> >+++ b/src/ldap_helper.c
>> >@@ -975,11 +975,15 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
>> >  {
>> >  	const char *dn = entry->dn;
>> >  	isc_result_t result;
>> >+	isc_result_t orig_result;
>> >  	ldap_valuelist_t values;
>> >  	ldap_value_t *value;
>> >  	isc_sockaddrlist_t addrs;
>> >  	isc_boolean_t is_global_config;
>> >  	isc_boolean_t fwdtbl_deletion_requested = ISC_TRUE;
>> >+	isc_boolean_t fwdtbl_update_requested = ISC_FALSE;
>> >+	dns_forwarders_t *old_setting = NULL;
>> >+	dns_fixedname_t foundname;
>> >  	const char *msg_use_global_fwds;
>> >  	const char *msg_obj_type;
>> >  	const char *msg_forwarders_not_def;
>> >@@ -993,6 +997,7 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
>> >
>> >  	REQUIRE(entry != NULL && inst != NULL && name != NULL);
>> >  	ISC_LIST_INIT(addrs);
>> >+	dns_fixedname_init(&foundname);
>> >  	if (dns_name_equal(name, dns_rootname)) {
>> >  		is_global_config = ISC_TRUE;
>> >  		msg_obj_type = "global configuration";
>> >@@ -1083,16 +1088,49 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
>> >  			  msg_obj_type, dn);
>> >  	}
>> >
>> >-	/* Set forward table up. */
>> >-	CHECK(delete_forwarding_table(inst, name, msg_obj_type, dn));
>> >-	result = dns_fwdtable_add(inst->view->fwdtable, name, &addrs, fwdpolicy);
>> >+	/* Check for old and new forwarding settings equality. */
>> >+	result = dns_fwdtable_find2(inst->view->fwdtable, name,
>> >+				    dns_fixedname_name(&foundname),
>> >+				    &old_setting);
>> >+	if (result == ISC_R_SUCCESS &&
>> >+	   (dns_name_equal(name, dns_fixedname_name(&foundname)) == ISC_TRUE)) {
>> >+		isc_sockaddr_t *s1, *s2;
>> >+
>> >+		if (fwdpolicy != old_setting->fwdpolicy)
>> >+			fwdtbl_update_requested = ISC_TRUE;
>> >+
>> >+		/* Check address lists item by item. */
>> >+		for (s1 = ISC_LIST_HEAD(addrs), s2 = ISC_LIST_HEAD(old_setting->addrs);
>> >+		     s1 != NULL && s2 != NULL && !fwdtbl_update_requested;
>> >+		     s1 = ISC_LIST_NEXT(s1, link), s2 = ISC_LIST_NEXT(s2, link))
>> >+			if (!isc_sockaddr_equal(s1, s2))
>> >+				fwdtbl_update_requested = ISC_TRUE;
>> >+
>> >+		if ((s1 == NULL) != (s2 == NULL))
> Although this is correct, something like
>
> if ((s1 != NULL) || (s2 != NULL))
>
> or even
>
> if (!fwdtbl_update_requested && ((s1 != NULL) || (s2 != NULL)))
>
> shouldn't affect functionality and is more readable than rare (==) != (==)
> construction.
>
> You don't have to pass the patch for review again, just directly push it.
>

Corrected, pushed to master and v2:
25ca3ce13a28b6c856025e887891cff55165d648

-- 
Petr^2 Spacek