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<pspa...@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<pspa...@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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to