Merge branch '991-provide-ixfr-should-only-be-tests-on-tcp-clients' into 'master'

Resolve "provide-ixfr should only be tested on TCP clients."

Closes #991

See merge request !1846

CHANGES

+5224.	[bug]		Only test provide-ixfr on TCP streams. [GL #991]
+
 5223.	[bug]		Fixed a race in the filter-aaaa plugin accessing
 			the hash table. [GL #1005]
-5222.	[bug]		'delve -t ANY' could leak memory. [GL #983]
+5222.	[bug]		'delv -t ANY' could leak memory. [GL #983]
 5221.	[test]		Enable parallel execution of system tests on
 			Windows. [GL !4101]

bin/tests/system/ixfr/clean.sh

@@ -13,11 +13,12 @@ rm -f stats.*
 rm -f ns1/*.db ns1/*.jnl
 rm -f ns3/*.jnl ns3/mytest.db ns3/subtest.db
 rm -f ns4/*.jnl ns4/*.db
+rm -f ns5/*.jnl ns5/*.db
 rm -f */named.memstats
 rm -f */named.conf
 rm -f */named.run
 rm -f */ans.run
-rm -f dig.out dig.out1 dig.out2 dig.out3
+rm -f dig.out.test* dig.out1.test* dig.out2.test* dig.out3.test*
 rm -f ns3/large.db
 rm -f ns*/named.lock
 rm -f ns*/managed-keys.bind* ns*/*.mkeys

bin/tests/system/ixfr/ns3/mytest0.db

@@ -18,7 +18,9 @@ test.  15  IN        SOA ns1.test.  hostmaster.test. (
                              )
        IN  NS     ns1.test.
        IN  NS     ns2.test.
+       IN  NS     ns5.test.
 ns1    IN  A      10.53.0.3
 ns2    IN  A      10.53.0.4
+ns5    IN  A      10.53.0.5
 host1  IN  A      192.168.10.3
 host2  IN  A      192.168.10.4

bin/tests/system/ixfr/ns3/mytest1.db

@@ -18,7 +18,9 @@ test.  15  IN        SOA ns1.test.  hostmaster.test. (
                              )
        IN  NS     ns1.test.
        IN  NS     ns2.test.
+       IN  NS     ns5.test.
 ns1    IN  A      10.53.0.3
 ns2    IN  A      10.53.0.4
+ns5    IN  A      10.53.0.5
 host1  IN  A      192.168.10.13
 host2  IN  A      192.168.10.4

bin/tests/system/ixfr/ns3/mytest2.db

@@ -18,7 +18,9 @@ test.  15  IN        SOA ns1.test.  hostmaster.test. (
                              )
        IN  NS     ns1.test.
        IN  NS     ns2.test.
+       IN  NS     ns5.test.
 ns1    IN  A      10.53.0.3
 ns2    IN  A      10.53.0.4
+ns5    IN  A      10.53.0.5
 host1  IN  A      192.168.10.13
 host2  IN  A      192.168.10.14

bin/tests/system/ixfr/ns5/named.conf.in

+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+options {
+	query-source address 10.53.0.5;
+	notify-source 10.53.0.5;
+	transfer-source 10.53.0.5;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.5; };
+	listen-on-v6 { none; };
+	recursion no;
+	notify yes;
+	provide-ixfr no;
+};
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm hmac-sha256;
+};
+
+controls {
+	inet 10.53.0.5 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+view "primary" {
+	ixfr-from-differences yes;
+	request-ixfr yes;
+	zone "test" IN {
+		type slave;
+		file "mytest.db";
+		masters { 10.53.0.3; };
+	};
+	zone "sub.test" IN {
+		type slave;
+		file "subtest.db";
+		request-ixfr no;
+		masters { 10.53.0.3; };
+	};
+};

bin/tests/system/ixfr/setup.sh

@@ -39,6 +39,7 @@ EOF
 
 copy_setports ns3/named.conf.in ns3/named.conf
 copy_setports ns4/named.conf.in ns4/named.conf
+copy_setports ns5/named.conf.in ns5/named.conf
 
 # Setup initial db files for ns3
 cp ns3/mytest0.db ns3/mytest.db

bin/tests/system/ixfr/tests.sh

@@ -19,12 +19,14 @@ SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
 
 status=0
+n=0
 
 DIGOPTS="+tcp +noadd +nosea +nostat +noquest +nocomm +nocmd -p ${PORT}"
 SENDCMD="$PERL ../send.pl 10.53.0.2 ${EXTRAPORT1}"
 RNDCCMD="$RNDC -p ${CONTROLPORT} -c ../common/rndc.conf -s"
 
-echo_i "testing initial AXFR"
+n=$((n+1))
+echo_i "testing initial AXFR ($n)"
 
 $SENDCMD <<EOF
 /SOA/
@@ -58,8 +60,8 @@ rndc_reload ns1 10.53.0.1
 
 for i in 0 1 2 3 4 5 6 7 8 9
 do
-	$DIG $DIGOPTS @10.53.0.1 nil. SOA > dig.out
-	grep "SOA" dig.out > /dev/null && break
+	$DIG $DIGOPTS @10.53.0.1 nil. SOA > dig.out.test$n
+	grep "SOA" dig.out.test$n > /dev/null && break
 	sleep 1
 done
 
@@ -68,7 +70,8 @@ $DIG $DIGOPTS @10.53.0.1 nil. TXT | grep 'initial AXFR' >/dev/null || {
     status=1
 }
 
-echo_i "testing successful IXFR"
+n=$((n+1))
+echo_i "testing successful IXFR ($n)"
 
 # We change the IP address of a.nil., and the TXT record at the apex.
 # Then we do a SOA-only update.
@@ -100,7 +103,8 @@ $DIG $DIGOPTS @10.53.0.1 nil. TXT | grep 'successful IXFR' >/dev/null || {
     status=1
 }
 
-echo_i "testing AXFR fallback after IXFR failure"
+n=$((n+1))
+echo_i "testing AXFR fallback after IXFR failure ($n)"
 
 # Provide a broken IXFR response and a working fallback AXFR response
 
@@ -134,7 +138,8 @@ $DIG $DIGOPTS @10.53.0.1 nil. TXT | grep 'fallback AXFR' >/dev/null || {
     status=1
 }
 
-echo_i "testing ixfr-from-differences option"
+n=$((n+1))
+echo_i "testing ixfr-from-differences option ($n)"
 # ns3 is master; ns4 is slave
 $CHECKZONE test. ns3/mytest.db > /dev/null 2>&1
 if [ $? -ne 0 ]
@@ -150,8 +155,8 @@ fi
 # wait for slave to be stable
 for i in 0 1 2 3 4 5 6 7 8 9
 do
-	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out
-	grep -i "hostmaster\.test\..1" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..1" dig.out.test$n > /dev/null && break
 	sleep 1
 done
 
@@ -162,16 +167,16 @@ $RNDCCMD 10.53.0.3 reload | sed 's/^/ns3 /' | cat_i
 #wait for master to reload load
 for i in 0 1 2 3 4 5 6 7 8 9
 do
-	$DIG $DIGOPTS +tcp @10.53.0.3 SOA test > dig.out
-	grep -i "hostmaster\.test\..2" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.3 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..2" dig.out.test$n > /dev/null && break
 	sleep 1
 done
 
 #wait for slave to transfer zone
 for i in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14
 do
-	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out
-	grep -i "hostmaster\.test\..2" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..2" dig.out.test$n > /dev/null && break
 
 	# re-notify if we've been waiting a long time
 	if [ $i -ge 5 ]; then
@@ -194,7 +199,8 @@ then
     status=1
 fi
 
-echo_i "testing request-ixfr option in view vs zone"
+n=$((n+1))
+echo_i "testing request-ixfr option in view vs zone ($n)"
 # There's a view with 2 zones. In the view, "request-ixfr yes"
 # but in the zone "sub.test", request-ixfr no"
 # we want to make sure that a change to sub.test results in AXFR, while
@@ -207,16 +213,16 @@ $RNDCCMD 10.53.0.3 reload | sed 's/^/ns3 /' | cat_i
 #wait for master to reload zone
 for i in 0 1 2 3 4 5 6 7 8 9
 do
-	$DIG $DIGOPTS +tcp @10.53.0.3 SOA sub.test > dig.out
-	grep -i "hostmaster\.test\..3" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.3 SOA sub.test > dig.out.test$n
+	grep -i "hostmaster\.test\..3" dig.out.test$n > /dev/null && break
 	sleep 1
 done
 
 #wait for slave to transfer zone
 for i in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14
 do
-	$DIG $DIGOPTS +tcp @10.53.0.4 SOA sub.test > dig.out
-	grep -i "hostmaster\.test\..3" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.4 SOA sub.test > dig.out.test$n
+	grep -i "hostmaster\.test\..3" dig.out.test$n > /dev/null && break
 
 	# re-notify if we've been waiting a long time
 	if [ $i -ge 5 ]; then
@@ -247,16 +253,16 @@ $RNDCCMD 10.53.0.3 reload | sed 's/^/ns3 /' | cat_i
 # wait for master to reload zone
 for i in 0 1 2 3 4 5 6 7 8 9
 do
-	$DIG +tcp -p 5300 @10.53.0.3 SOA test > dig.out
-	grep -i "hostmaster\.test\..4" dig.out > /dev/null && break
+	$DIG +tcp -p 5300 @10.53.0.3 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..4" dig.out.test$n > /dev/null && break
 	sleep 1
 done
 
 # wait for slave to transfer zone
 for i in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14
 do
-	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out
-	grep -i "hostmaster\.test\..4" dig.out > /dev/null && break
+	$DIG $DIGOPTS +tcp @10.53.0.4 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..4" dig.out.test$n > /dev/null && break
 
 	# re-notify if we've been waiting a long time
 	if [ $i -ge 5 ]; then
@@ -279,58 +285,90 @@ else
     echo_i "  success: IXFR it was"
 fi
 
-echo_i "testing DiG's handling of a multi message AXFR style IXFR response"
+n=$((n+1))
+echo_i "testing DiG's handling of a multi message AXFR style IXFR response ($n)"
 (
 (sleep 10 && kill $$) 2>/dev/null &
 sub=$!
-$DIG -p ${PORT} ixfr=0 large @10.53.0.3 > dig.out
+$DIG -p ${PORT} ixfr=0 large @10.53.0.3 > dig.out.test$n
 kill $sub
 )
-lines=`grep hostmaster.large dig.out | wc -l`
+lines=`grep hostmaster.large dig.out.test$n | wc -l`
 test ${lines:-0} -eq 2 || { echo_i "failed"; status=1; }
-messages=`sed -n 's/^;;.*messages \([0-9]*\),.*/\1/p' dig.out`
+messages=`sed -n 's/^;;.*messages \([0-9]*\),.*/\1/p' dig.out.test$n`
 test ${messages:-0} -gt 1 || { echo_i "failed"; status=1; }
 
-echo_i "test 'dig +notcp ixfr=<value>' vs 'dig ixfr=<value> +notcp' vs 'dig ixfr=<value>'"
+n=$((n+1))
+echo_i "test 'dig +notcp ixfr=<value>' vs 'dig ixfr=<value> +notcp' vs 'dig ixfr=<value>' ($n)"
 ret=0
 # Should be "switch to TCP" response
-$DIG $DIGOPTS +notcp ixfr=1 test @10.53.0.4 > dig.out1 || ret=1
-$DIG $DIGOPTS ixfr=1 +notcp test @10.53.0.4 > dig.out2 || ret=1
-digcomp dig.out1 dig.out2 || ret=1
-awk '$4 == "SOA" { soacnt++} END {if (soacnt == 1) exit(0); else exit(1);}' dig.out1 || ret=1
-awk '$4 == "SOA" { if ($7 == 4) exit(0); else exit(1);}' dig.out1 || ret=1
+$DIG $DIGOPTS +notcp ixfr=1 test @10.53.0.4 > dig.out1.test$n || ret=1
+$DIG $DIGOPTS ixfr=1 +notcp test @10.53.0.4 > dig.out2.test$n || ret=1
+digcomp dig.out1.test$n dig.out2.test$n || ret=1
+awk '$4 == "SOA" { soacnt++} END {if (soacnt == 1) exit(0); else exit(1);}' dig.out1.test$n || ret=1
+awk '$4 == "SOA" { if ($7 == 4) exit(0); else exit(1);}' dig.out1.test$n || ret=1
 # Should be incremental transfer.
-$DIG $DIGOPTS ixfr=1 test @10.53.0.4 > dig.out3 || ret=1
-awk '$4 == "SOA" { soacnt++} END { if (soacnt == 6) exit(0); else exit(1);}' dig.out3 || ret=1
+$DIG $DIGOPTS ixfr=1 test @10.53.0.4 > dig.out3.test$n || ret=1
+awk '$4 == "SOA" { soacnt++} END { if (soacnt == 6) exit(0); else exit(1);}' dig.out3.test$n || ret=1
 if [ ${ret} != 0 ]; then
 	echo_i "failed";
 	status=1;
 fi
 
-echo_i "checking whether dig calculates IXFR statistics correctly"
+# wait for slave to transfer zone
+for i in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14
+do
+	$DIG $DIGOPTS +tcp @10.53.0.5 SOA test > dig.out.test$n
+	grep -i "hostmaster\.test\..4" dig.out.test$n > /dev/null && break
+
+	# re-notify if we've been waiting a long time
+	if [ $i -ge 5 ]; then
+	    $RNDCCMD 10.53.0.3 notify test | set 's/^/ns3 /' | cat_i
+	fi
+	sleep 1
+done
+
+n=$((n+1))
+echo_i "test 'provide-ixfr no;' ($n)"
+ret=0
+# Should be "AXFR style" response
+$DIG $DIGOPTS ixfr=1 test @10.53.0.5 > dig.out1.test$n || ret=1
+# Should be "switch to TCP" response
+$DIG $DIGOPTS ixfr=1 +notcp test @10.53.0.5 > dig.out2.test$n || ret=1
+awk '$4 == "SOA" { soacnt++} END {if (soacnt == 2) exit(0); else exit(1);}' dig.out1.test$n || ret=1
+awk '$4 == "SOA" { soacnt++} END {if (soacnt == 1) exit(0); else exit(1);}' dig.out2.test$n || ret=1
+if [ ${ret} != 0 ]; then
+	echo_i "failed";
+	status=1;
+fi
+
+n=$((n+1))
+echo_i "checking whether dig calculates IXFR statistics correctly ($n)"
 ret=0
-$DIG $DIGOPTS +noedns +stat -b 10.53.0.4 @10.53.0.4 test. ixfr=2 > dig.out1
-get_dig_xfer_stats dig.out1 > stats.dig
+$DIG $DIGOPTS +noedns +stat -b 10.53.0.4 @10.53.0.4 test. ixfr=2 > dig.out1.test$n
+get_dig_xfer_stats dig.out1.test$n > stats.dig
 diff ixfr-stats.good stats.dig || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 # Note: in the next two tests, we use ns4 logs for checking both incoming and
 # outgoing transfer statistics as ns4 is both a secondary server (for ns3) and a
 # primary server (for dig queries from the previous test) for "test".
-echo_i "checking whether named calculates incoming IXFR statistics correctly"
+n=$((n+1))
+echo_i "checking whether named calculates incoming IXFR statistics correctly ($n)"
 ret=0
 get_named_xfer_stats ns4/named.run 10.53.0.3 test "Transfer completed" > stats.incoming
 diff ixfr-stats.good stats.incoming || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
-echo_i "checking whether named calculates outgoing IXFR statistics correctly"
+n=$((n+1))
+echo_i "checking whether named calculates outgoing IXFR statistics correctly ($n)"
 ret=0
 get_named_xfer_stats ns4/named.run 10.53.0.4 test "IXFR ended" > stats.outgoing
 diff ixfr-stats.good stats.outgoing || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

lib/ns/xfrout.c

@@ -807,11 +807,13 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 	question_rdataset = ISC_LIST_HEAD(question_name->list);
 	question_class = question_rdataset->rdclass;
 	INSIST(question_rdataset->type == reqtype);
-	if (ISC_LIST_NEXT(question_rdataset, link) != NULL)
+	if (ISC_LIST_NEXT(question_rdataset, link) != NULL) {
 		FAILC(DNS_R_FORMERR, "multiple questions");
+	}
 	result = dns_message_nextname(request, DNS_SECTION_QUESTION);
-	if (result != ISC_R_NOMORE)
+	if (result != ISC_R_NOMORE) {
 		FAILC(DNS_R_FORMERR, "multiple questions");
+	}
 
 	result = dns_zt_find(client->view->zonetable, question_name, 0, NULL,
 			     &zone);
@@ -897,8 +899,9 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 		/*
 		 * Ignore data whose owner name is not the zone apex.
 		 */
-		if (! dns_name_equal(soa_name, question_name))
+		if (! dns_name_equal(soa_name, question_name)) {
 			continue;
+		}
 
 		for (soa_rdataset = ISC_LIST_HEAD(soa_name->list);
 		     soa_rdataset != NULL;
@@ -907,25 +910,29 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 			/*
 			 * Ignore non-SOA data.
 			 */
-			if (soa_rdataset->type != dns_rdatatype_soa)
+			if (soa_rdataset->type != dns_rdatatype_soa) {
 				continue;
-			if (soa_rdataset->rdclass != question_class)
+			}
+			if (soa_rdataset->rdclass != question_class) {
 				continue;
+			}
 
 			CHECK(dns_rdataset_first(soa_rdataset));
 			dns_rdataset_current(soa_rdataset, &soa_rdata);
 			result = dns_rdataset_next(soa_rdataset);
-			if (result == ISC_R_SUCCESS)
+			if (result == ISC_R_SUCCESS) {
 				FAILC(DNS_R_FORMERR,
 				      "IXFR authority section "
 				      "has multiple SOAs");
+			}
 			have_soa = true;
 			goto got_soa;
 		}
 	}
  got_soa:
-	if (result != ISC_R_NOMORE)
+	if (result != ISC_R_NOMORE) {
 		CHECK(result);
+	}
 
 	xfrout_log1(client, question_name, question_class, ISC_LOG_DEBUG(6),
 		    "%s authority section OK", mnemonic);
@@ -952,8 +959,9 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 	 * AXFR over UDP is not possible.
 	 */
 	if (reqtype == dns_rdatatype_axfr &&
-	    (client->attributes & NS_CLIENTATTR_TCP) == 0)
+	    (client->attributes & NS_CLIENTATTR_TCP) == 0) {
 		FAILC(DNS_R_FORMERR, "attempted AXFR over UDP");
+	}
 
 	/*
 	 * Look up the requesting server in the peer table.
@@ -964,8 +972,9 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 	/*
 	 * Decide on the transfer format (one-answer or many-answers).
 	 */
-	if (peer != NULL)
+	if (peer != NULL) {
 		(void)dns_peer_gettransferformat(peer, &format);
+	}
 
 	/*
 	 * Get a dynamically allocated copy of the current SOA.
@@ -979,21 +988,27 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 
 	current_serial = dns_soa_getserial(&current_soa_tuple->rdata);
 	if (reqtype == dns_rdatatype_ixfr) {
-		bool provide_ixfr;
-
 		/*
 		 * Outgoing IXFR may have been disabled for this peer
 		 * or globally.
 		 */
-		provide_ixfr = client->view->provideixfr;
-		if (peer != NULL)
-			(void) dns_peer_getprovideixfr(peer, &provide_ixfr);
-		if (provide_ixfr == false)
-			goto axfr_fallback;
+		if ((client->attributes & NS_CLIENTATTR_TCP) != 0) {
+			bool provide_ixfr;
+
+			provide_ixfr = client->view->provideixfr;
+			if (peer != NULL) {
+				(void) dns_peer_getprovideixfr(peer,
+							       &provide_ixfr);
+			}
+			if (provide_ixfr == false) {
+				goto axfr_fallback;
+			}
+		}
 
-		if (! have_soa)
+		if (! have_soa) {
 			FAILC(DNS_R_FORMERR,
 			      "IXFR request missing SOA");
+		}
 
 		begin_serial = dns_soa_getserial(&soa_rdata);
 
@@ -1016,16 +1031,16 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 			goto have_stream;
 		}
 		journalfile = is_dlz ? NULL : dns_zone_getjournal(zone);
-		if (journalfile != NULL)
+		if (journalfile != NULL) {
 			result = ixfr_rrstream_create(mctx,
 						      journalfile,
 						      begin_serial,
 						      current_serial,
 						      &data_stream);
-		else
+		} else {
 			result = ISC_R_NOTFOUND;
-		if (result == ISC_R_NOTFOUND ||
-		    result == ISC_R_RANGE) {
+		}
+		if (result == ISC_R_NOTFOUND || result == ISC_R_RANGE) {
 			xfrout_log1(client, question_name, question_class,
 				    ISC_LOG_DEBUG(4),
 				    "IXFR version not in journal, "
@@ -1058,7 +1073,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 
 
 
-	if (is_dlz)
+	if (is_dlz) {
 		CHECK(xfrout_ctx_create(mctx, client, request->id,
 					question_name, reqtype, question_class,
 					zone, db, ver, quota, stream,
@@ -1070,7 +1085,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 					(format == dns_many_answers) ?
 					true : false,
 					&xfr));
-	else
+	} else {
 		CHECK(xfrout_ctx_create(mctx, client, request->id,
 					question_name, reqtype, question_class,
 					zone, db, ver, quota, stream,
@@ -1082,6 +1097,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 					(format == dns_many_answers) ?
 					true : false,
 					&xfr));
+	}
 
 	xfr->mnemonic = mnemonic;
 	stream = NULL;
@@ -1089,24 +1105,26 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 
 	CHECK(xfr->stream->methods->first(xfr->stream));
 
-	if (xfr->tsigkey != NULL)
+	if (xfr->tsigkey != NULL) {
 		dns_name_format(&xfr->tsigkey->name, keyname, sizeof(keyname));
-	else
+	} else {
 		keyname[0] = '\0';
-	if (is_poll)
+	}
+	if (is_poll) {
 		xfrout_log1(client, question_name, question_class,
 			    ISC_LOG_DEBUG(1), "IXFR poll up to date%s%s",
 			    (xfr->tsigkey != NULL) ? ": TSIG " : "", keyname);
-	else if (is_ixfr)
+	} else if (is_ixfr) {
 		xfrout_log1(client, question_name, question_class,
 			    ISC_LOG_INFO, "%s started%s%s (serial %u -> %u)",
 			    mnemonic, (xfr->tsigkey != NULL) ? ": TSIG " : "",
 			    keyname, begin_serial, current_serial);
-	else
+	} else {
 		xfrout_log1(client, question_name, question_class,
 			    ISC_LOG_INFO, "%s started%s%s (serial %u)",
 			    mnemonic, (xfr->tsigkey != NULL) ? ": TSIG " : "",
 			    keyname, current_serial);
+	}
 
 
 	if (zone != NULL) {
@@ -1125,8 +1143,9 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 				client->expire = secs - client->now;
 			}
 		}
-		if (raw != NULL)
+		if (raw != NULL) {
 			dns_zone_detach(&raw);
+		}
 	}
 
 	/*
@@ -1140,24 +1159,33 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 	result = ISC_R_SUCCESS;
 
  failure:
-	if (result == DNS_R_REFUSED)
+	if (result == DNS_R_REFUSED) {
 		inc_stats(client, zone, ns_statscounter_xfrrej);
-	if (quota != NULL)
+	}
+	if (quota != NULL) {
 		isc_quota_detach(&quota);
-	if (current_soa_tuple != NULL)
+	}
+	if (current_soa_tuple != NULL) {
 		dns_difftuple_free(&current_soa_tuple);
-	if (stream != NULL)
+	}
+	if (stream != NULL) {
 		stream->methods->destroy(&stream);
-	if (soa_stream != NULL)
+	}
+	if (soa_stream != NULL) {
 		soa_stream->methods->destroy(&soa_stream);
-	if (data_stream != NULL)
+	}
+	if (data_stream != NULL) {
 		data_stream->methods->destroy(&data_stream);
-	if (ver != NULL)
+	}
+	if (ver != NULL) {
 		dns_db_closeversion(db, &ver, false);
-	if (db != NULL)
+	}
+	if (db != NULL) {
 		dns_db_detach(&db);
-	if (zone != NULL)
+	}
+	if (zone != NULL) {
 		dns_zone_detach(&zone);
+	}
 	/* XXX kludge */
 	if (xfr != NULL) {
 		xfrout_fail(xfr, result, "setting up zone transfer");