checkpoint

bin/named/client.c

@@ -29,7 +29,6 @@
 #include <dns/message.h>
 
 #include <named/client.h>
-#include <named/query.h>
 
 #include "../../isc/util.h"		/* XXX */
 
@@ -84,19 +83,10 @@ static void clientmgr_destroy(ns_clientmgr_t *manager);
 static inline void
 client_free(ns_client_t *client) {
 	dns_dispatchevent_t **deventp;
-	isc_dynbuffer_t *dbuf, *dbuf_next;
-	isc_mem_t *mctx;
 
 	CTRACE("free");
 
-	mctx = client->manager->mctx;
+	ns_query_free(client);
-	for (dbuf = ISC_LIST_HEAD(client->namebufs);
-	     dbuf != NULL;
-	     dbuf = dbuf_next) {
-		dbuf_next = ISC_LIST_NEXT(dbuf, link);
-		isc_dynbuffer_free(mctx, &dbuf);
-	}
-	ISC_LIST_INIT(client->namebufs);
 	isc_mempool_destroy(&client->sendbufs);
 	dns_message_destroy(&client->message);
 	isc_timer_detach(&client->timer);
@@ -114,7 +104,7 @@ client_free(ns_client_t *client) {
 	isc_task_detach(&client->task);
 	client->magic = 0;
 
-	isc_mem_put(mctx, client, sizeof *client);
+	isc_mem_put(client->mctx, client, sizeof *client);
 }
 
 void
@@ -163,7 +153,6 @@ client_shutdown(isc_task_t *task, isc_event_t *event) {
 
 void
 ns_client_next(ns_client_t *client, isc_result_t result) {
-	isc_dynbuffer_t *dbuf, *dbuf_next;
 
 	REQUIRE(NS_CLIENT_VALID(client));
 	REQUIRE(client->state == ns_clientstate_listening ||
@@ -178,22 +167,6 @@ ns_client_next(ns_client_t *client, isc_result_t result) {
 	 */
 	(void)result;
 
-	/*
-	 * Reset any changes made by answering a query.  XXXRTH  Should this
-	 * be in query.c?  We'll free all but one namebuf.
-	 */
-	client->qname = NULL;
-	INSIST(!ISC_LIST_EMPTY(client->namebufs));
-	for (dbuf = ISC_LIST_HEAD(client->namebufs);
-	     dbuf != NULL;
-	     dbuf = dbuf_next) {
-		dbuf_next = ISC_LIST_NEXT(dbuf, link);
-		if (dbuf_next != NULL) {
-			ISC_LIST_UNLINK(client->namebufs, dbuf, link);
-			isc_dynbuffer_free(client->manager->mctx, &dbuf);
-		}
-	}
-
 	dns_message_reset(client->message, DNS_MESSAGE_INTENTPARSE);
 	if (client->dispevent != NULL) {
 		dns_dispatch_freeevent(client->dispatch, client->dispentry,
@@ -399,30 +372,12 @@ client_timeout(isc_task_t *task, isc_event_t *event) {
 	ns_client_next(client, ISC_R_TIMEDOUT);
 }
 
-isc_result_t
-ns_client_newnamebuf(ns_client_t *client) {
-	isc_dynbuffer_t *dbuf;
-	isc_result_t result;
-
-	REQUIRE(NS_CLIENT_VALID(client));
-
-	dbuf = NULL;
-	result = isc_dynbuffer_allocate(client->manager->mctx, &dbuf, 1024,
-					ISC_BUFFERTYPE_BINARY);
-	if (result != ISC_R_SUCCESS)
-		return (result);
-	ISC_LIST_APPEND(client->namebufs, dbuf, link);
-
-	return (ISC_R_SUCCESS);
-}
-
 static isc_result_t
 client_create(ns_clientmgr_t *manager, ns_clienttype_t type,
 	      ns_client_t **clientp)
 {
 	ns_client_t *client;
 	isc_result_t result;
-	isc_dynbuffer_t *dbuf;
 
 	/*
 	 * Caller must be holding the manager lock.
@@ -468,11 +423,8 @@ client_create(ns_clientmgr_t *manager, ns_clienttype_t type,
 	isc_mempool_setfreemax(client->sendbufs, 3);
 	isc_mempool_setmaxalloc(client->sendbufs, 3);
 
-	/*
-	 * We do the rest of the initialization here because the
-	 * ns_client_newnamebuf() call below REQUIREs a valid client.
-	 */
 	client->magic = NS_CLIENT_MAGIC;
+	client->mctx = manager->mctx;
 	client->manager = manager;
 	client->type = type;
 	client->state = ns_clientstate_idle;
@@ -480,13 +432,15 @@ client_create(ns_clientmgr_t *manager, ns_clienttype_t type,
 	client->dispatch = NULL;
 	client->dispentry = NULL;
 	client->dispevent = NULL;
-	client->qname = NULL;
 	client->nsends = 0;
-	ISC_LIST_INIT(client->namebufs);
 	ISC_LINK_INIT(client, link);
 
-	dbuf = NULL;
+	/*
-	result = ns_client_newnamebuf(client);
+	 * We call the init routines for the various kinds of client here,
+	 * after we have created an otherwise valid client, because some
+	 * of them call routines that REQUIRE(NS_CLIENT_VALID(client)).
+	 */
+	result = ns_query_init(client);
 	if (result != ISC_R_SUCCESS)
 		goto cleanup_sendbufs;
 
@@ -499,6 +453,8 @@ client_create(ns_clientmgr_t *manager, ns_clienttype_t type,
  cleanup_sendbufs:
 	isc_mempool_destroy(&client->sendbufs);
 
+	client->magic = 0;
+
  cleanup_message:
 	dns_message_destroy(&client->message);
 

bin/named/include/named/client.h

@@ -24,6 +24,7 @@
 #include <dns/types.h>
 
 #include <named/types.h>
+#include <named/query.h>
 
 typedef enum ns_clienttype {
 	ns_clienttype_basic = 0,
@@ -41,6 +42,7 @@ typedef enum {
 
 struct ns_client {
 	unsigned int			magic;
+	isc_mem_t *			mctx;
 	ns_clientmgr_t *		manager;
 	ns_clienttype_t			type;
 	ns_clientstate_t		state;
@@ -51,10 +53,9 @@ struct ns_client {
 	dns_dispatchevent_t *		dispevent;
 	isc_timer_t *			timer;
 	dns_message_t *			message;
-	dns_name_t *			qname;
 	unsigned int			nsends;
 	isc_mempool_t *			sendbufs;
-	ISC_LIST(isc_dynbuffer_t)	namebufs;
+	ns_query_t			query;
 	ISC_LINK(struct ns_client)	link;
 };
 

bin/named/include/named/query.h

@@ -18,8 +18,25 @@
 #ifndef NS_QUERY_H
 #define NS_QUERY_H 1
 
+#include <isc/types.h>
+#include <isc/buffer.h>
+
+#include <dns/types.h>
+
 #include <named/types.h>
 
-void ns_query_start(ns_client_t *client);
+struct ns_query {
+	dns_name_t *			qname;
+	ISC_LIST(isc_dynbuffer_t)	namebufs;
+};
+
+isc_result_t
+ns_query_init(ns_client_t *client);
+
+void
+ns_query_free(ns_client_t *client);
+
+void
+ns_query_start(ns_client_t *client);
 
 #endif /* NS_QUERY_H */

bin/named/include/named/types.h

@@ -24,6 +24,7 @@
 
 typedef struct ns_client		ns_client_t;
 typedef struct ns_clientmgr		ns_clientmgr_t;
+typedef struct ns_query			ns_query_t;
 
 /*
  * XXXRTH  This stuff is temporary.  As soon as we have zone and config

bin/named/query.c

@@ -42,6 +42,89 @@
 
 #include "../../isc/util.h"		/* XXX */
 
+static inline void
+query_cleanup(ns_client_t *client, isc_boolean_t everything) {
+	isc_dynbuffer_t *dbuf, *dbuf_next;
+
+	client->query.qname = NULL;
+	for (dbuf = ISC_LIST_HEAD(client->query.namebufs);
+	     dbuf != NULL;
+	     dbuf = dbuf_next) {
+		dbuf_next = ISC_LIST_NEXT(dbuf, link);
+		if (dbuf_next != NULL || everything) {
+			ISC_LIST_UNLINK(client->query.namebufs, dbuf, link);
+			isc_dynbuffer_free(client->mctx, &dbuf);
+		}
+	}
+}
+
+void
+ns_query_free(ns_client_t *client) {
+	query_cleanup(client, ISC_TRUE);
+}
+
+static void
+query_next(ns_client_t *client, isc_result_t result) {
+	query_cleanup(client, ISC_FALSE);
+	ns_client_next(client, result);
+}
+
+static void
+query_error(ns_client_t *client, isc_result_t result) {
+	query_cleanup(client, ISC_FALSE);
+	ns_client_error(client, result);
+}
+
+static inline isc_result_t
+query_newnamebuf(ns_client_t *client) {
+	isc_dynbuffer_t *dbuf;
+	isc_result_t result;
+
+	REQUIRE(NS_CLIENT_VALID(client));
+
+	dbuf = NULL;
+	result = isc_dynbuffer_allocate(client->mctx, &dbuf, 1024,
+					ISC_BUFFERTYPE_BINARY);
+	if (result != ISC_R_SUCCESS)
+		return (result);
+	ISC_LIST_APPEND(client->query.namebufs, dbuf, link);
+
+	return (ISC_R_SUCCESS);
+}
+
+static inline isc_dynbuffer_t *
+query_getnamebuf(ns_client_t *client) {
+	isc_dynbuffer_t *dbuf;
+	isc_result_t result;
+	isc_region_t r;
+
+	if (ISC_LIST_EMPTY(client->query.namebufs)) {
+		result = query_newnamebuf(client);
+		if (result != ISC_R_SUCCESS)
+			return (NULL);
+	}
+
+	dbuf = ISC_LIST_TAIL(client->query.namebufs);
+	INSIST(dbuf != NULL);
+	isc_buffer_available(&dbuf->buffer, &r);
+	if (r.length < 255) {
+		result = query_newnamebuf(client);
+		if (result != ISC_R_SUCCESS)
+			return (NULL);
+		dbuf = ISC_LIST_TAIL(client->query.namebufs);
+		isc_buffer_available(&dbuf->buffer, &r);
+		INSIST(r.length >= 255);
+	}
+	return (dbuf);
+}
+
+isc_result_t
+ns_query_init(ns_client_t *client) {
+	client->query.qname = NULL;
+	ISC_LIST_INIT(client->query.namebufs);
+	return (query_newnamebuf(client));
+}
+
 static isc_result_t
 find(ns_client_t *client, dns_rdatatype_t type) {
 	isc_result_t result;
@@ -82,21 +165,19 @@ find(ns_client_t *client, dns_rdatatype_t type) {
 		/*
 		 * Get the resources we'll need.
 		 */
-		dbuf = ISC_LIST_TAIL(client->namebufs);
+		dbuf = query_getnamebuf(client);
-		isc_buffer_available(&dbuf->buffer, &r);
+		if (dbuf == NULL) {
-		if (r.length < 255) {
+			if (first_time)
-			result = ns_client_newnamebuf(client);
+				return (DNS_R_SERVFAIL);
-			if (result != ISC_R_SUCCESS)
+			else
-				return (result);
+				return (ISC_R_SUCCESS);
-			dbuf = ISC_LIST_TAIL(client->namebufs);
-			isc_buffer_available(&dbuf->buffer, &r);
-			INSIST(r.length >= 255);
 		}
 		isc_buffer_init(&b, r.base, r.length, ISC_BUFFERTYPE_BINARY);
 		fname = NULL;
 		result = dns_message_gettempname(client->message, &fname);
 		if (result != ISC_R_SUCCESS)
 			return (result);
+		/* XXXRTH  should return success if not first time. */
 		dns_name_init(fname, NULL);
 		dns_name_setbuffer(fname, &b);
 		rdataset = NULL;
@@ -104,13 +185,15 @@ find(ns_client_t *client, dns_rdatatype_t type) {
 						     &rdataset);
 		if (result != ISC_R_SUCCESS)
 			return (result);
+		/* XXXRTH  should return success if not first time. */
 		dns_rdataset_init(rdataset);
 
 		/*
 		 * Find a database to answer the query.
 		 */
 		db = NULL;
-		result = dns_dbtable_find(ns_g_dbtable, client->qname, &db);
+		result = dns_dbtable_find(ns_g_dbtable, client->query.qname,
+					  &db);
 		if (result != ISC_R_SUCCESS && result != DNS_R_PARTIALMATCH) {
 			if (first_time)
 				return (DNS_R_SERVFAIL);
@@ -124,7 +207,7 @@ find(ns_client_t *client, dns_rdatatype_t type) {
 		if (dns_db_iszone(db))
 			auth = ISC_TRUE;
 		node = NULL;
-		result = dns_db_find(db, client->qname, NULL, type, 0, 0,
+		result = dns_db_find(db, client->query.qname, NULL, type, 0, 0,
 				     &node, fname, rdataset);
 		switch (result) {
 		case DNS_R_SUCCESS:
@@ -146,7 +229,7 @@ find(ns_client_t *client, dns_rdatatype_t type) {
 			r.length = rdata.length;
 			dns_name_init(tname, NULL);
 			dns_name_fromregion(tname, &r);
-			client->qname = tname;
+			client->query.qname = tname;
 			break;
 		case DNS_R_GLUE:
 		case DNS_R_ZONECUT:
@@ -251,7 +334,7 @@ ns_query_start(ns_client_t *client) {
 
 	result = dns_message_reply(client->message, ISC_TRUE);
 	if (result != ISC_R_SUCCESS) {
-		ns_client_next(client, result);
+		query_next(client, result);
 		return;
 	}
 
@@ -267,15 +350,15 @@ ns_query_start(ns_client_t *client) {
 	result = dns_message_firstname(client->message, DNS_SECTION_QUESTION);
 	while (result == ISC_R_SUCCESS) {
 		nquestions++;
-		client->qname = NULL;
+		client->query.qname = NULL;
 		dns_message_currentname(client->message, DNS_SECTION_QUESTION,
-					&client->qname);
+					&client->query.qname);
-		for (rdataset = ISC_LIST_HEAD(client->qname->list);
+		for (rdataset = ISC_LIST_HEAD(client->query.qname->list);
 		     rdataset != NULL;
 		     rdataset = ISC_LIST_NEXT(rdataset, link)) {
 			result = find(client, rdataset->type);
 			if (result != ISC_R_SUCCESS) {
-				ns_client_error(client, result);
+				query_error(client, result);
 				return;
 			}
 		}
@@ -283,12 +366,12 @@ ns_query_start(ns_client_t *client) {
 					      DNS_SECTION_QUESTION);
 	}
 	if (result != ISC_R_NOMORE) {
-		ns_client_error(client, result);
+		query_error(client, result);
 		return;
 	}
 
 	if (nquestions == 0) {
-		ns_client_error(client, DNS_R_FORMERR);
+		query_error(client, DNS_R_FORMERR);
 		return;
 	}