From 76baed33438dd416e9b961f2e3b96508da0bc278 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Wed, 15 Sep 2021 20:56:55 -0700
Subject: [PATCH 1/3] rewrite logfileconfig system test

the logfileconfig system test did not conform to the style of
other tests, and was difficult to read and maintain. it has
been cleaned up and simplifeid in several ways:

- named.args used when appropriate so that named can be started with
  specified command line arguments, instead of having it launched
  directly from tests.sh
- unused root zone removed from named configuration
- an existing directory used instead of using 'mkdir' to create one
- dnssec-validation disabled to stop the server sending unnecessary queries

incidental fix: removed leftover debugging printfs from logconf.c.
---
 bin/named/logconf.c                           |   6 -
 bin/named/server.c                            |   4 +-
 bin/tests/system/logfileconfig/clean.sh       |   5 +-
 bin/tests/system/logfileconfig/named1.args    |   1 +
 bin/tests/system/logfileconfig/named2.args    |   1 +
 .../system/logfileconfig/ns1/controls.conf.in |  16 -
 .../ns1/{named.dirconf => named.dirconf.in}   |  16 +-
 ...named.iso8601-utc => named.iso8601-utc.in} |  14 +-
 .../ns1/{named.iso8601 => named.iso8601.in}   |  15 +-
 .../ns1/{named.pipeconf => named.pipeconf.in} |  14 +-
 .../ns1/{named.plain => named.plain.in}       |  15 +-
 .../{named.plainconf => named.plainconf.in}   |  14 +-
 .../ns1/{named.symconf => named.symconf.in}   |  14 +-
 .../ns1/{named.tsconf => named.tsconf.in}     |  15 +-
 .../{named.unlimited => named.unlimited.in}   |  15 +-
 .../ns1/{named.versconf => named.versconf.in} |  15 +-
 .../system/logfileconfig/ns1/rndc.conf.in     |  24 -
 bin/tests/system/logfileconfig/ns1/root.db    |  25 -
 bin/tests/system/logfileconfig/setup.sh       |   4 +-
 bin/tests/system/logfileconfig/tests.sh       | 502 ++++++------------
 util/copyrights                               |  22 +-
 21 files changed, 237 insertions(+), 520 deletions(-)
 create mode 100644 bin/tests/system/logfileconfig/named1.args
 create mode 100644 bin/tests/system/logfileconfig/named2.args
 delete mode 100644 bin/tests/system/logfileconfig/ns1/controls.conf.in
 rename bin/tests/system/logfileconfig/ns1/{named.dirconf => named.dirconf.in} (82%)
 rename bin/tests/system/logfileconfig/ns1/{named.iso8601-utc => named.iso8601-utc.in} (84%)
 rename bin/tests/system/logfileconfig/ns1/{named.iso8601 => named.iso8601.in} (84%)
 rename bin/tests/system/logfileconfig/ns1/{named.pipeconf => named.pipeconf.in} (84%)
 rename bin/tests/system/logfileconfig/ns1/{named.plain => named.plain.in} (86%)
 rename bin/tests/system/logfileconfig/ns1/{named.plainconf => named.plainconf.in} (81%)
 rename bin/tests/system/logfileconfig/ns1/{named.symconf => named.symconf.in} (84%)
 rename bin/tests/system/logfileconfig/ns1/{named.tsconf => named.tsconf.in} (87%)
 rename bin/tests/system/logfileconfig/ns1/{named.unlimited => named.unlimited.in} (87%)
 rename bin/tests/system/logfileconfig/ns1/{named.versconf => named.versconf.in} (87%)
 delete mode 100644 bin/tests/system/logfileconfig/ns1/rndc.conf.in
 delete mode 100644 bin/tests/system/logfileconfig/ns1/root.db

diff --git a/bin/named/logconf.c b/bin/named/logconf.c
index 4aae8ca239..68dd83773e 100644
--- a/bin/named/logconf.c
+++ b/bin/named/logconf.c
@@ -299,10 +299,6 @@ channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *logconfig) {
 					       dest.file.name,
 					       isc_result_totext(result));
 				}
-				fprintf(stderr,
-					"isc_stdio_open '%s' failed: %s\n",
-					dest.file.name,
-					isc_result_totext(result));
 			} else {
 				(void)isc_stdio_close(fp);
 			}
@@ -312,8 +308,6 @@ channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *logconfig) {
 			syslog(LOG_ERR, "isc_file_isplainfile '%s' failed: %s",
 			       dest.file.name, isc_result_totext(result));
 		}
-		fprintf(stderr, "isc_file_isplainfile '%s' failed: %s\n",
-			dest.file.name, isc_result_totext(result));
 	}
 
 done:
diff --git a/bin/named/server.c b/bin/named/server.c
index ab853b0b73..c8e7232da4 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9355,8 +9355,8 @@ load_configuration(const char *filename, named_server_t *server,
 		logobj = NULL;
 		(void)cfg_map_get(config, "logging", &logobj);
 		if (logobj != NULL) {
-			CHECKM(named_logconfig(logc, logobj), "configuring "
-							      "logging");
+			CHECKM(named_logconfig(logc, logobj),
+			       "configuring logging");
 		} else {
 			named_log_setdefaultchannels(logc);
 			CHECKM(named_log_setunmatchedcategory(logc),
diff --git a/bin/tests/system/logfileconfig/clean.sh b/bin/tests/system/logfileconfig/clean.sh
index dd179a6549..2ba3b370f8 100644
--- a/bin/tests/system/logfileconfig/clean.sh
+++ b/bin/tests/system/logfileconfig/clean.sh
@@ -12,10 +12,9 @@
 #
 # Clean up after log file tests
 #
-rm -f ns1/rndc.conf
-rm -f ns1/controls.conf
 rm -f ns1/named.conf
-rm -f ns1/named.pid ns1/named.run
+rm -f ns1/named.args
+rm -f ns1/named.pid ns1/named.run ns1/named.run.prev
 rm -f ns1/named.memstats ns1/dig.out
 rm -f ns1/named_log ns1/named_pipe ns1/named_sym
 rm -rf ns1/named_dir
diff --git a/bin/tests/system/logfileconfig/named1.args b/bin/tests/system/logfileconfig/named1.args
new file mode 100644
index 0000000000..764d4c969e
--- /dev/null
+++ b/bin/tests/system/logfileconfig/named1.args
@@ -0,0 +1 @@
+-c named.conf -m record -T nosyslog -d 99 -D logfileconfig-ns1 -X named.lock -U 4
diff --git a/bin/tests/system/logfileconfig/named2.args b/bin/tests/system/logfileconfig/named2.args
new file mode 100644
index 0000000000..fb9fe57371
--- /dev/null
+++ b/bin/tests/system/logfileconfig/named2.args
@@ -0,0 +1 @@
+-c named.conf -m record -T nosyslog -d 99 -D logfileconfig-ns1 -X named.lock -U 4 -L named_deflog
diff --git a/bin/tests/system/logfileconfig/ns1/controls.conf.in b/bin/tests/system/logfileconfig/ns1/controls.conf.in
deleted file mode 100644
index 74084d96af..0000000000
--- a/bin/tests/system/logfileconfig/ns1/controls.conf.in
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * 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.
- */
-
-controls {
-	inet 127.0.0.1 port @CONTROLPORT@
-		allow { 127.0.0.1/32; ::1/128; }
-		keys { "rndc-key"; };
-};
diff --git a/bin/tests/system/logfileconfig/ns1/named.dirconf b/bin/tests/system/logfileconfig/ns1/named.dirconf.in
similarity index 82%
rename from bin/tests/system/logfileconfig/ns1/named.dirconf
rename to bin/tests/system/logfileconfig/ns1/named.dirconf.in
index e64ca456f3..fe7df47118 100644
--- a/bin/tests/system/logfileconfig/ns1/named.dirconf
+++ b/bin/tests/system/logfileconfig/ns1/named.dirconf.in
@@ -17,27 +17,25 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
 
 logging {
 	channel default_log {
-	  file "named_dir";
+	  file "/tmp";
 	  print-time yes;
 	};
 	category default { default_log; default_debug; };
 	category lame-servers { null; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.iso8601-utc b/bin/tests/system/logfileconfig/ns1/named.iso8601-utc.in
similarity index 84%
rename from bin/tests/system/logfileconfig/ns1/named.iso8601-utc
rename to bin/tests/system/logfileconfig/ns1/named.iso8601-utc.in
index 186abd1b0b..d900831a96 100644
--- a/bin/tests/system/logfileconfig/ns1/named.iso8601-utc
+++ b/bin/tests/system/logfileconfig/ns1/named.iso8601-utc.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -30,14 +31,11 @@ logging {
 	category default { default_log; default_debug; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.iso8601 b/bin/tests/system/logfileconfig/ns1/named.iso8601.in
similarity index 84%
rename from bin/tests/system/logfileconfig/ns1/named.iso8601
rename to bin/tests/system/logfileconfig/ns1/named.iso8601.in
index b8ad96c9bf..37e7cf112d 100644
--- a/bin/tests/system/logfileconfig/ns1/named.iso8601
+++ b/bin/tests/system/logfileconfig/ns1/named.iso8601.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -30,14 +31,12 @@ logging {
 	category default { default_log; default_debug; };
 };
 
-include "controls.conf";
 
-key "rndc-key" {
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
+
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.pipeconf b/bin/tests/system/logfileconfig/ns1/named.pipeconf.in
similarity index 84%
rename from bin/tests/system/logfileconfig/ns1/named.pipeconf
rename to bin/tests/system/logfileconfig/ns1/named.pipeconf.in
index ba48fa9b8e..7c9fa55f69 100644
--- a/bin/tests/system/logfileconfig/ns1/named.pipeconf
+++ b/bin/tests/system/logfileconfig/ns1/named.pipeconf.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -30,14 +31,11 @@ logging {
 	category lame-servers { null; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.plain b/bin/tests/system/logfileconfig/ns1/named.plain.in
similarity index 86%
rename from bin/tests/system/logfileconfig/ns1/named.plain
rename to bin/tests/system/logfileconfig/ns1/named.plain.in
index fff82940bf..181070727f 100644
--- a/bin/tests/system/logfileconfig/ns1/named.plain
+++ b/bin/tests/system/logfileconfig/ns1/named.plain.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -37,15 +38,11 @@ logging {
 	category queries { query_log; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.plainconf b/bin/tests/system/logfileconfig/ns1/named.plainconf.in
similarity index 81%
rename from bin/tests/system/logfileconfig/ns1/named.plainconf
rename to bin/tests/system/logfileconfig/ns1/named.plainconf.in
index 3b6054a3da..b46fde5c56 100644
--- a/bin/tests/system/logfileconfig/ns1/named.plainconf
+++ b/bin/tests/system/logfileconfig/ns1/named.plainconf.in
@@ -17,18 +17,16 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.symconf b/bin/tests/system/logfileconfig/ns1/named.symconf.in
similarity index 84%
rename from bin/tests/system/logfileconfig/ns1/named.symconf
rename to bin/tests/system/logfileconfig/ns1/named.symconf.in
index 32b0d1507d..2d1e4a98a7 100644
--- a/bin/tests/system/logfileconfig/ns1/named.symconf
+++ b/bin/tests/system/logfileconfig/ns1/named.symconf.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -30,14 +31,11 @@ logging {
 	category lame-servers { null; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.tsconf b/bin/tests/system/logfileconfig/ns1/named.tsconf.in
similarity index 87%
rename from bin/tests/system/logfileconfig/ns1/named.tsconf
rename to bin/tests/system/logfileconfig/ns1/named.tsconf.in
index 55df326abb..e7513edb49 100644
--- a/bin/tests/system/logfileconfig/ns1/named.tsconf
+++ b/bin/tests/system/logfileconfig/ns1/named.tsconf.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -39,15 +40,11 @@ logging {
 	category queries { query_log; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.unlimited b/bin/tests/system/logfileconfig/ns1/named.unlimited.in
similarity index 87%
rename from bin/tests/system/logfileconfig/ns1/named.unlimited
rename to bin/tests/system/logfileconfig/ns1/named.unlimited.in
index 61080538f1..0e1a788e15 100644
--- a/bin/tests/system/logfileconfig/ns1/named.unlimited
+++ b/bin/tests/system/logfileconfig/ns1/named.unlimited.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -39,15 +40,11 @@ logging {
 	category queries { query_log; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/named.versconf b/bin/tests/system/logfileconfig/ns1/named.versconf.in
similarity index 87%
rename from bin/tests/system/logfileconfig/ns1/named.versconf
rename to bin/tests/system/logfileconfig/ns1/named.versconf.in
index 1310ba09b8..0df547317c 100644
--- a/bin/tests/system/logfileconfig/ns1/named.versconf
+++ b/bin/tests/system/logfileconfig/ns1/named.versconf.in
@@ -17,6 +17,7 @@ options {
 	pid-file "named.pid";
 	listen-on { 10.53.0.1; };
 	listen-on-v6 { none; };
+	dnssec-validation no;
 	recursion no;
 	notify yes;
 };
@@ -39,15 +40,11 @@ logging {
 	category queries { query_log; };
 };
 
-include "controls.conf";
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; };
+};
 
-key "rndc-key" {
+key rndc-key {
+	secret "1234abcd8765";
 	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
-
-
-zone "." {
-	type primary;
-	file "root.db";
 };
diff --git a/bin/tests/system/logfileconfig/ns1/rndc.conf.in b/bin/tests/system/logfileconfig/ns1/rndc.conf.in
deleted file mode 100644
index 7e67a2405a..0000000000
--- a/bin/tests/system/logfileconfig/ns1/rndc.conf.in
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * 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 {
-	default-server 127.0.0.1;
-};
-
-server 127.0.0.1 {
-	key "rndc-key";
-	addresses   { 127.0.0.1 port @CONTROLPORT@; };
-};
-
-key "rndc-key" {
-	algorithm hmac-sha256;
-	secret "Am9vCg==";
-};
diff --git a/bin/tests/system/logfileconfig/ns1/root.db b/bin/tests/system/logfileconfig/ns1/root.db
deleted file mode 100644
index 8aaa4ea9fd..0000000000
--- a/bin/tests/system/logfileconfig/ns1/root.db
+++ /dev/null
@@ -1,25 +0,0 @@
-; 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.
-
-$TTL 300
-. 			IN SOA	gson.nominum.com. a.root.servers.nil. (
-				2000042100   	; serial
-				600         	; refresh
-				600         	; retry
-				1200    	; expire
-				600       	; minimum
-				)
-.			NS	a.root-servers.nil.
-a.root-servers.nil.	A	10.53.0.1
-
-example.		NS	ns2.example.
-ns2.example.		A	10.53.0.2
-
-tsigzone.		NS	ns2.tsigzone.
-ns2.tsigzone.		A	10.53.0.2
diff --git a/bin/tests/system/logfileconfig/setup.sh b/bin/tests/system/logfileconfig/setup.sh
index 935e36c5f8..19b246985f 100644
--- a/bin/tests/system/logfileconfig/setup.sh
+++ b/bin/tests/system/logfileconfig/setup.sh
@@ -13,6 +13,4 @@
 
 $SHELL clean.sh
 
-copy_setports ns1/named.plain ns1/named.conf
-copy_setports ns1/rndc.conf.in ns1/rndc.conf
-copy_setports ns1/controls.conf.in ns1/controls.conf
+copy_setports ns1/named.plain.in ns1/named.conf
diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh
index 191680873e..5fce3f9813 100644
--- a/bin/tests/system/logfileconfig/tests.sh
+++ b/bin/tests/system/logfileconfig/tests.sh
@@ -10,32 +10,6 @@
 # information regarding copyright ownership.
 
 . ../conf.sh
-THISDIR=`pwd`
-CONFDIR="ns1"
-
-PLAINCONF="${THISDIR}/${CONFDIR}/named.plainconf"
-PLAINFILE="named_log"
-DIRCONF="${THISDIR}/${CONFDIR}/named.dirconf"
-DIRFILE="named_dir"
-PIPECONF="${THISDIR}/${CONFDIR}/named.pipeconf"
-PIPEFILE="named_pipe"
-SYMCONF="${THISDIR}/${CONFDIR}/named.symconf"
-SYMFILE="named_sym"
-VERSCONF="${THISDIR}/${CONFDIR}/named.versconf"
-VERSFILE="named_vers"
-TSCONF="${THISDIR}/${CONFDIR}/named.tsconf"
-TSFILE="named_ts"
-UNLIMITEDCONF="${THISDIR}/${CONFDIR}/named.unlimited"
-UNLIMITEDFILE="named_unlimited"
-ISOCONF="${THISDIR}/${CONFDIR}/named.iso8601"
-ISOFILE="named_iso8601"
-ISOCONFUTC="${THISDIR}/${CONFDIR}/named.iso8601-utc"
-ISOUTCFILE="named_iso8601_utc"
-DLFILE="named_deflog"
-
-PIDFILE="${THISDIR}/${CONFDIR}/named.pid"
-myRNDC="$RNDC -c ${THISDIR}/${CONFDIR}/rndc.conf"
-myNAMED="$NAMED -c ${THISDIR}/${CONFDIR}/named.conf -m record -T nosyslog -d 99 -D logfileconfig-ns1 -X named.lock -U 4"
 
 # Test given condition.  If true, test again after a second.  Used for testing
 # filesystem-dependent conditions in order to prevent false negatives caused by
@@ -50,366 +24,202 @@ test_with_retry() {
 	return 1
 }
 
-waitforpidfile() {
-	for _w in 1 2 3 4 5 6 7 8 9 10
-	do
-		test -f $PIDFILE && break
-		sleep 1
-	done
-}
-
 status=0
 n=0
 
-cd $CONFDIR
-
-echo_i "testing log file validity (named -g + only plain files allowed)"
-
-n=`expr $n + 1`
-echo_i "testing plain file (named -g) ($n)"
 # First run with a known good config.
-echo > $PLAINFILE
-copy_setports $PLAINCONF named.conf
-$myRNDC reconfig > rndc.out.test$n 2>&1
-grep "reloading configuration failed" named.run > /dev/null 2>&1
-if [ $? -ne 0 ]
-then
-	echo_i "testing plain file succeeded"
-else
-	echo_i "testing plain file failed (unexpected)"
-	echo_i "exit status: 1"
-	exit 1
-fi
+n=$((n+1))
+echo_i "testing log file validity (only plain files allowed) ($n)"
+ret=0
+cat /dev/null > ns1/named_log
+copy_setports ns1/named.plainconf.in ns1/named.conf
+nextpart ns1/named.run > /dev/null
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+wait_for_log 5 "reloading configuration succeeded" ns1/named.run || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
 # Now try directory, expect failure
-n=`expr $n + 1`
-echo_i "testing directory as log file (named -g) ($n)"
-echo > named.run
-rm -rf $DIRFILE
-mkdir -p $DIRFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $DIRCONF named.conf
-	echo > named.run
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	grep "checking logging configuration failed: invalid file" named.run > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing directory as file succeeded (UNEXPECTED)"
-		echo_i "exit status: 1"
-		exit 1
-	else
-		echo_i "testing directory as log file failed (expected)"
-	fi
-else
-	echo_i "skipping directory test (unable to create directory)"
-fi
-
-# Now try pipe file, expect failure
-n=`expr $n + 1`
-echo_i "testing pipe file as log file (named -g) ($n)"
-echo > named.run
-rm -f $PIPEFILE
-mkfifo $PIPEFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $PIPECONF named.conf
-	echo > named.run
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	grep "checking logging configuration failed: invalid file" named.run  > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing pipe file as log file succeeded (UNEXPECTED)"
-		echo_i "exit status: 1"
-		exit 1
-	else
-		echo_i "testing pipe file as log file failed (expected)"
-	fi
-else
-	echo_i "skipping pipe test (unable to create pipe)"
-fi
-
-# Now try symlink file to plain file, expect success
-n=`expr $n + 1`
-echo_i "testing symlink to plain file as log file (named -g) ($n)"
-# Assume success
-echo > named.run
-echo > $PLAINFILE
-rm -f  $SYMFILE  $SYMFILE
-ln -s $PLAINFILE $SYMFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $SYMCONF named.conf
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	echo > named.run
-	grep "reloading configuration failed" named.run > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing symlink to plain file succeeded"
-	else
-		echo_i "testing symlink to plain file failed (unexpected)"
-		echo_i "exit status: 1"
-		exit 1
-	fi
-else
-	echo_i "skipping symlink test (unable to create symlink)"
-fi
-# Stop the server and run through a series of tests with various config
-# files while controlling the stop/start of the server.
-# Have to stop the stock server because it uses "-g"
-#
-$PERL ../../stop.pl logfileconfig ns1
-
-$myNAMED > /dev/null 2>&1
-
-if [ $? -ne 0 ]
-then
-	echo_i "failed to start $myNAMED"
-	echo_i "exit status: $status"
-	exit $status
-fi
-
-status=0
-
-echo_i "testing log file validity (only plain files allowed)"
-
-n=`expr $n + 1`
-echo_i "testing plain file (named -g) ($n)"
-# First run with a known good config.
-echo > $PLAINFILE
-copy_setports $PLAINCONF named.conf
-$myRNDC reconfig > rndc.out.test$n 2>&1
-grep "reloading configuration failed" named.run > /dev/null 2>&1
-if [ $? -ne 0 ]
-then
-	echo_i "testing plain file succeeded"
-else
-	echo_i "testing plain file failed (unexpected)"
-	echo_i "exit status: 1"
-	exit 1
-fi
-
-# Now try directory, expect failure
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing directory as log file ($n)"
-echo > named.run
-rm -rf $DIRFILE
-mkdir -p $DIRFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $DIRCONF named.conf
-	echo > named.run
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	grep "configuring logging: invalid file" named.run > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing directory as file succeeded (UNEXPECTED)"
-		echo_i "exit status: 1"
-		exit 1
-	else
-		echo_i "testing directory as log file failed (expected)"
-	fi
-else
-	echo_i "skipping directory test (unable to create directory)"
-fi
+ret=0
+nextpart ns1/named.run > /dev/null
+copy_setports ns1/named.dirconf.in ns1/named.conf
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+wait_for_log 5 "reloading configuration failed: invalid file" ns1/named.run || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
 # Now try pipe file, expect failure
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing pipe file as log file ($n)"
-echo > named.run
-rm -f $PIPEFILE
-mkfifo $PIPEFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $PIPECONF named.conf
-	echo > named.run
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	grep "configuring logging: invalid file" named.run  > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing pipe file as log file succeeded (UNEXPECTED)"
-		echo_i "exit status: 1"
-		exit 1
-	else
-		echo_i "testing pipe file as log file failed (expected)"
-	fi
+ret=0
+nextpart ns1/named.run > /dev/null
+rm -f ns1/named_pipe
+if mkfifo ns1/named_pipe >/dev/null 2>&1; then
+    copy_setports ns1/named.pipeconf.in ns1/named.conf
+    rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+    wait_for_log 5 "reloading configuration failed: invalid file" ns1/named.run || ret=1
+    if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+    status=$((status+ret))
 else
-	echo_i "skipping pipe test (unable to create pipe)"
+    echo_i "skipping pipe test (unable to create pipe)"
 fi
 
 # Now try symlink file to plain file, expect success
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing symlink to plain file as log file ($n)"
-# Assume success
-status=0
-echo > named.run
-echo > $PLAINFILE
-rm -f $SYMFILE
-ln -s $PLAINFILE $SYMFILE >/dev/null 2>&1
-if [ $? -eq 0 ]
-then
-	copy_setports $SYMCONF named.conf
-	$myRNDC reconfig > rndc.out.test$n 2>&1
-	echo > named.run
-	grep "reloading configuration failed" named.run > /dev/null 2>&1
-	if [ $? -ne 0 ]
-	then
-		echo_i "testing symlink to plain file succeeded"
-	else
-		echo_i "testing symlink to plain file failed (unexpected)"
-		echo_i "exit status: 1"
-		exit 1
-	fi
+ret=0
+rm -f ns1/named_log ns1/named_sym
+touch ns1/named_log
+if ln -s $(pwd)/ns1/named_log $(pwd)/ns1/named_sym >/dev/null 2>&1; then
+    nextpart ns1/named.run > /dev/null
+    copy_setports ns1/named.symconf.in ns1/named.conf
+    rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+    wait_for_log 5 "reloading configuration succeeded" ns1/named.run || ret=1
+    if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+    status=$((status+ret))
 else
 	echo_i "skipping symlink test (unable to create symlink)"
 fi
 
-n=`expr $n + 1`
-echo_i "testing default logfile using named -L file ($n)"
-# Now stop the server again and test the -L option
-rm -f $DLFILE
-$PERL ../../stop.pl logfileconfig ns1
-if ! test -f $PIDFILE; then
-	copy_setports $PLAINCONF named.conf
-	$myNAMED -L $DLFILE > /dev/null 2>&1
-	if [ $? -ne 0 ]; then
-		echo_i "failed to start $myNAMED"
-		echo_i "exit status: $status"
-		exit $status
-	fi
+echo_i "repeat previous tests without named -g"
+copy_setports ns1/named.plain.in ns1/named.conf
+$PERL ../stop.pl --use-rndc --port ${CONTROLPORT} logfileconfig ns1
+cp named1.args ns1/named.args
+start_server --noclean --restart --port ${PORT} logfileconfig ns1
 
-	waitforpidfile
+n=$((n+1))
+echo_i "testing log file validity (only plain files allowed) ($n)"
+ret=0
+cat /dev/null > ns1/named_log
+copy_setports ns1/named.plainconf.in ns1/named.conf
+nextpart ns1/named.run > /dev/null
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+wait_for_log 5 "reloading configuration succeeded" ns1/named.run || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
-	sleep 1
-	if [ -f "$DLFILE" ]; then
-		echo_i "testing default logfile using named -L succeeded"
-	else
-		echo_i "testing default logfile using named -L failed"
-		echo_i "exit status: 1"
-		exit 1
-	fi
+# Now try directory, expect failure
+n=$((n+1))
+echo_i "testing directory as log file ($n)"
+ret=0
+nextpart ns1/named.run > /dev/null
+copy_setports ns1/named.dirconf.in ns1/named.conf
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+wait_for_log 5 "reloading configuration failed: invalid file" ns1/named.run || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
+
+# Now try pipe file, expect failure
+n=$((n+1))
+echo_i "testing pipe file as log file ($n)"
+ret=0
+nextpart ns1/named.run > /dev/null
+rm -f ns1/named_pipe
+if mkfifo ns1/named_pipe >/dev/null 2>&1; then
+    copy_setports ns1/named.pipeconf.in ns1/named.conf
+    rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+    wait_for_log 5 "reloading configuration failed: invalid file" ns1/named.run || ret=1
+    if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+    status=$((status+ret))
 else
-	echo_i "failed to cleanly stop $myNAMED"
-	echo_i "exit status: 1"
-	exit 1
+    echo_i "skipping pipe test (unable to create pipe)"
+fi
+
+# Now try symlink file to plain file, expect success
+n=$((n+1))
+echo_i "testing symlink to plain file as log file ($n)"
+ret=0
+rm -f ns1/named_log ns1/named_sym
+touch ns1/named_log
+if ln -s $(pwd)/ns1/named_log $(pwd)/ns1/named_sym >/dev/null 2>&1; then
+    nextpart ns1/named.run > /dev/null
+    copy_setports ns1/named.symconf.in ns1/named.conf
+    rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+    wait_for_log 5 "reloading configuration succeeded" ns1/named.run || ret=1
+    if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+    status=$((status+ret))
+else
+	echo_i "skipping symlink test (unable to create symlink)"
 fi
 
 echo_i "testing logging functionality"
-
-n=`expr $n + 1`
+n=$((n+1))
+ret=0
 echo_i "testing iso8601 timestamp ($n)"
-copy_setports $ISOCONF named.conf
-$myRNDC reconfig > rndc.out.test$n 2>&1
-if grep '^....-..-..T..:..:..\.... ' $ISOFILE > /dev/null; then
-	echo_i "testing iso8601 timestamp succeeded"
-else
-	echo_i "testing iso8601 timestamp failed"
-	status=`expr $status + 1`
-fi
+copy_setports ns1/named.iso8601.in ns1/named.conf
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+grep '^....-..-..T..:..:..\.... ' ns1/named_iso8601 > /dev/null || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing iso8601-utc timestamp ($n)"
-copy_setports $ISOCONFUTC named.conf
-$myRNDC reconfig > rndc.out.test$n 2>&1
-if grep '^....-..-..T..:..:..\....Z' $ISOUTCFILE > /dev/null; then
-	echo_i "testing iso8601-utc timestamp succeeded"
-else
-	echo_i "testing iso8601-utc timestamp failed"
-	status=`expr $status + 1`
-fi
+ret=0
+copy_setports ns1/named.iso8601-utc.in ns1/named.conf
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
+grep '^....-..-..T..:..:..\....Z' ns1/named_iso8601_utc > /dev/null || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing explicit versions ($n)"
-copy_setports $VERSCONF named.conf
+ret=0
+copy_setports ns1/named.versconf.in ns1/named.conf
 # a seconds since epoch version number
-touch $VERSFILE.1480039317
-t1=`$PERL -e 'print time()."\n";'`
-$myRNDC reconfig > rndc.out.test$n 2>&1
+touch ns1/named_vers.1480039317
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
 $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n
-t2=`$PERL -e 'print time()."\n";'`
-t=`expr ${t2:-0} - ${t1:-0}`
-if test ${t:-1000} -gt 5
-then
-        echo_i "testing explicit versions failed: cleanup of old entries took too long ($t secs)"
-	status=`expr $status + 1`
-fi
-if ! grep "status: NOERROR" dig.out.test$n > /dev/null
-then
-	echo_i "testing explicit versions failed: DiG lookup failed"
-	status=`expr $status + 1`
-fi
-if test_with_retry -f $VERSFILE.1480039317
-then
-	echo_i "testing explicit versions failed: $VERSFILE.1480039317 not removed"
-	status=`expr $status + 1`
-fi
-if test_with_retry -f $VERSFILE.5
-then
-	echo_i "testing explicit versions failed: $VERSFILE.5 exists"
-	status=`expr $status + 1`
-fi
-if test_with_retry ! -f $VERSFILE.4
-then
-	echo_i "testing explicit versions failed: $VERSFILE.4 does not exist"
-	status=`expr $status + 1`
-fi
+grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1
+# we are configured to retain five logfiles (a current file
+# and 4 backups). so files with version number 5 or higher
+# should be removed.
+test_with_retry -f ns1/named_vers.1480039317 && ret=1
+test_with_retry -f ns1/named_vers.5 && ret=1
+test_with_retry -f ns1/named_vers.4 || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing timestamped versions ($n)"
-copy_setports $TSCONF named.conf
+ret=0
+copy_setports ns1/named.tsconf.in ns1/named.conf
 # a seconds since epoch version number
-touch $TSFILE.2015010112000012
-t1=`$PERL -e 'print time()."\n";'`
-$myRNDC reconfig > rndc.out.test$n 2>&1
+touch ns1/named_ts.1480039317
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
 $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n
-t2=`$PERL -e 'print time()."\n";'`
-t=`expr ${t2:-0} - ${t1:-0}`
-if test ${t:-1000} -gt 5
-then
-        echo_i "testing timestamped versions failed: cleanup of old entries took too long ($t secs)"
-	status=`expr $status + 1`
-fi
-if ! grep "status: NOERROR" dig.out.test$n > /dev/null
-then
-	echo_i "testing timestamped versions failed: DiG lookup failed"
-	status=`expr $status + 1`
-fi
-if test_with_retry -f $TSFILE.1480039317
-then
-	echo_i "testing timestamped versions failed: $TSFILE.1480039317 not removed"
-	status=`expr $status + 1`
-fi
+grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1
+test_with_retry -f ns1/named_ts.1480039317 && ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
-n=`expr $n + 1`
+n=$((n+1))
 echo_i "testing unlimited versions ($n)"
-copy_setports $UNLIMITEDCONF named.conf
+ret=0
+copy_setports ns1/named.unlimited.in ns1/named.conf
 # a seconds since epoch version number
-touch $UNLIMITEDFILE.1480039317
-t1=`$PERL -e 'print time()."\n";'`
-$myRNDC reconfig > rndc.out.test$n 2>&1
+touch ns1/named_unlimited.1480039317
+rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
 $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n
-t2=`$PERL -e 'print time()."\n";'`
-t=`expr ${t2:-0} - ${t1:-0}`
-if test ${t:-1000} -gt 5
-then
-        echo_i "testing unlimited versions failed: took too long ($t secs)"
-	status=`expr $status + 1`
-fi
-if ! grep "status: NOERROR" dig.out.test$n > /dev/null
-then
-	echo_i "testing unlimited versions failed: DiG lookup failed"
-	status=`expr $status + 1`
-fi
-if test_with_retry ! -f $UNLIMITEDFILE.1480039317
-then
-	echo_i "testing unlimited versions failed: $UNLIMITEDFILE.1480039317 removed"
-	status=`expr $status + 1`
-fi
-if test_with_retry ! -f $UNLIMITEDFILE.4
-then
-	echo_i "testing unlimited versions failed: $UNLIMITEDFILE.4 does not exist"
-	status=`expr $status + 1`
-fi
+grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1
+test_with_retry -f ns1/named_unlimited.1480039317 || ret=1
+test_with_retry -f ns1/named_unlimited.4 || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
+
+n=$((n+1))
+echo_i "testing default logfile using named -L file ($n)"
+ret=0
+$PERL ../stop.pl logfileconfig ns1
+cp named2.args ns1/named.args
+test -f ns1/named.pid && ret=1
+rm -f ns1/named_deflog
+copy_setports ns1/named.plainconf.in ns1/named.conf
+start_server --noclean --restart --port ${PORT} logfileconfig ns1
+[ -f "ns1/named_deflog" ] || ret=1
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1
diff --git a/util/copyrights b/util/copyrights
index 68eb1b2d7b..553ad00317 100644
--- a/util/copyrights
+++ b/util/copyrights
@@ -489,16 +489,18 @@
 ./bin/tests/system/limits/setup.sh		SH	2018,2019,2020,2021
 ./bin/tests/system/limits/tests.sh		SH	2000,2001,2004,2007,2011,2012,2016,2018,2019,2020,2021
 ./bin/tests/system/logfileconfig/clean.sh	SH	2011,2012,2014,2015,2016,2017,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.dirconf	X	2011,2013,2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.iso8601	X	2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.iso8601-utc	X	2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.pipeconf	X	2011,2013,2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.plain	X	2011,2013,2014,2016,2018,2019,2020
-./bin/tests/system/logfileconfig/ns1/named.plainconf	X	2014,2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.symconf	X	2011,2013,2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.tsconf	X	2017,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.unlimited	X	2016,2018,2019,2020,2021
-./bin/tests/system/logfileconfig/ns1/named.versconf	X	2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/named1.args	X	2021
+./bin/tests/system/logfileconfig/named2.args	X	2021
+./bin/tests/system/logfileconfig/ns1/named.dirconf.in	X	2011,2013,2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.iso8601-utc.in	X	2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.iso8601.in	X	2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.pipeconf.in	X	2011,2013,2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.plain.in	X	2011,2013,2014,2016,2018,2019,2020
+./bin/tests/system/logfileconfig/ns1/named.plainconf.in	X	2014,2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.symconf.in	X	2011,2013,2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.tsconf.in	X	2017,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.unlimited.in	X	2016,2018,2019,2020,2021
+./bin/tests/system/logfileconfig/ns1/named.versconf.in	X	2016,2018,2019,2020,2021
 ./bin/tests/system/logfileconfig/setup.sh	SH	2011,2012,2014,2016,2018,2019,2020,2021
 ./bin/tests/system/logfileconfig/tests.sh	SH	2011,2012,2013,2014,2016,2017,2018,2019,2020,2021
 ./bin/tests/system/makejournal.c		C	2013,2015,2016,2017,2018,2019,2020,2021

From 9a9e906306e1bf5c358572643798d94ce5fc36a1 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Tue, 12 Oct 2021 16:31:47 -0700
Subject: [PATCH 2/3] fixed a bug in rolling timestamp logfiles

due to comparing logfile suffixes as 32 bit rather than 64 bit
integers, logfiles with timestamp suffixes that should have been
removed when rolling could be left in place. this has been fixed.
---
 .../system/logfileconfig/ns1/named.tsconf.in  |  2 +-
 bin/tests/system/logfileconfig/tests.sh       | 18 ++++++++++++---
 lib/isc/log.c                                 | 22 +++++++++++--------
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/bin/tests/system/logfileconfig/ns1/named.tsconf.in b/bin/tests/system/logfileconfig/ns1/named.tsconf.in
index e7513edb49..7e62a93782 100644
--- a/bin/tests/system/logfileconfig/ns1/named.tsconf.in
+++ b/bin/tests/system/logfileconfig/ns1/named.tsconf.in
@@ -25,7 +25,7 @@ options {
 logging {
 	channel default_log {
 	  buffered no;
-	  file "named_ts" versions 10 size 1000 suffix timestamp; # small size
+	  file "named_ts" versions 3 size 1000 suffix timestamp; # small size
 	  severity debug 100;
 	  print-time yes;
 	};
diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh
index 5fce3f9813..ec3e265d32 100644
--- a/bin/tests/system/logfileconfig/tests.sh
+++ b/bin/tests/system/logfileconfig/tests.sh
@@ -187,10 +187,22 @@ ret=0
 copy_setports ns1/named.tsconf.in ns1/named.conf
 # a seconds since epoch version number
 touch ns1/named_ts.1480039317
+# a timestamp version number
+touch ns1/named_ts.20150101120000120
 rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n
-$DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n
-grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1
-test_with_retry -f ns1/named_ts.1480039317 && ret=1
+_found2() (
+        $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n
+        grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1
+
+        # we are configured to keep three versions, so the oldest
+        # timestamped versions should be gone, and there should
+        # be two new ones.
+        [ -f ns1/named_ts.1480039317 ] && return 1
+        [ -f ns1/named_ts.20150101120000120 ] && return 1
+        set -- ns1/named_ts.*
+        [ "$#" -eq 2 ] || return 1
+)
+retry_quiet 5 _found2 || ret=1
 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
diff --git a/lib/isc/log.c b/lib/isc/log.c
index 8f48f4718d..031393d430 100644
--- a/lib/isc/log.c
+++ b/lib/isc/log.c
@@ -1088,7 +1088,7 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) {
 }
 
 static void
-insert_sort(int64_t to_keep[], int64_t versions, int version) {
+insert_sort(int64_t to_keep[], int64_t versions, int64_t version) {
 	int i = 0;
 	while (i < versions && version < to_keep[i]) {
 		i++;
@@ -1105,12 +1105,13 @@ insert_sort(int64_t to_keep[], int64_t versions, int version) {
 
 static int64_t
 last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
-	if (versions <= 0) {
-		return INT64_MAX;
-	}
-
 	int64_t to_keep[ISC_LOG_MAX_VERSIONS] = { 0 };
 	int64_t version = 0;
+
+	if (versions <= 0) {
+		return (INT64_MAX);
+	}
+
 	if (versions > ISC_LOG_MAX_VERSIONS) {
 		versions = ISC_LOG_MAX_VERSIONS;
 	}
@@ -1119,6 +1120,9 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
 	 */
 	memset(to_keep, 0, sizeof(to_keep));
 	while (isc_dir_read(dirp) == ISC_R_SUCCESS) {
+		char *digit_end = NULL;
+		char *ename = NULL;
+
 		if (dirp->entry.length <= bnamelen ||
 		    strncmp(dirp->entry.name, bname, bnamelen) != 0 ||
 		    dirp->entry.name[bnamelen] != '.')
@@ -1126,8 +1130,7 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
 			continue;
 		}
 
-		char *digit_end;
-		char *ename = &dirp->entry.name[bnamelen + 1];
+		ename = &dirp->entry.name[bnamelen + 1];
 		version = strtoull(ename, &digit_end, 10);
 		if (*digit_end == '\0') {
 			insert_sort(to_keep, versions, version);
@@ -1145,12 +1148,13 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
 static isc_result_t
 remove_old_tsversions(isc_logfile_t *file, int versions) {
 	isc_result_t result;
-	char *bname, *digit_end;
-	const char *dirname;
+	char *bname = NULL, *digit_end = NULL;
+	const char *dirname = NULL;
 	int64_t version, last = INT64_MAX;
 	size_t bnamelen;
 	isc_dir_t dir;
 	char sep = '/';
+
 	/*
 	 * It is safe to DE_CONST the file.name because it was copied
 	 * with isc_mem_strdup().

From 96980adbad4efe1bd17f91e896d6663543b31250 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Tue, 12 Oct 2021 16:39:37 -0700
Subject: [PATCH 3/3] CHANGES and release note for [GL #828]

---
 CHANGES                     | 5 +++++
 doc/notes/notes-current.rst | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/CHANGES b/CHANGES
index a0bf41bc2b..f0fb0a3843 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,8 @@
+5741.	[bug]		Log files with "timestamp" suffixes could be left in
+			place after rolling, even if the number of preserved
+			log files exceeded the configured "versions" limit.
+			[GL #828]
+
 5740.	[func]		Implement incremental resizing of RBT hash table to
 			perform the rehashing gradually. [GL #2941]
 
diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 0235cbf023..9ad60d5809 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -100,3 +100,7 @@ Bug Fixes
 
 - Reloading a catalog zone that referenced a missing/deleted zone
   caused a crash. This has been fixed. :gl:`#2308`
+
+- Logfiles using ``timestamp``-style suffixes were not always correctly
+  removed when the number of files exceeded the limit set by ``versions``.
+  :gl:`#828`