Clean up after code review

bin/named/server.c

@@ -101,7 +101,7 @@ resolve_packet(isc_mem_t *mctx, dns_message_t *query, isc_buffer_t *target) {
 	isc_boolean_t possibly_auth = ISC_FALSE;
 
 	message = NULL;
-	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENT_RENDER);
+	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENTRENDER);
 	CHECKRESULT(result, "dns_message_create failed");
 
 	message->id = query->id;
@@ -284,7 +284,7 @@ dispatch(isc_mem_t *mctx, isc_region_t *rxr, unsigned int reslen)
 	isc_buffer_add(&source, rxr->length);
 
 	message = NULL;
-	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENT_PARSE);
+	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENTPARSE);
 	if (CHECKRESULT(result, "dns_message_create failed")) {
 		return (result);
 	}

bin/tests/wire_test.c

@@ -310,7 +310,7 @@ main(int argc, char *argv[]) {
 	isc_buffer_add(&source, bp - b);
 
 	message = NULL;
-	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENT_PARSE);
+	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENTPARSE);
 	CHECKRESULT(result, "dns_message_create failed");
 
 	result = dns_message_parse(message, &source);
@@ -326,7 +326,7 @@ main(int argc, char *argv[]) {
 	 * Changing this here is a hack, and should not be done in reasonable
 	 * application code, ever.
 	 */
-	message->from_to_wire = DNS_MESSAGE_INTENT_RENDER;
+	message->from_to_wire = DNS_MESSAGE_INTENTRENDER;
 	memset(b, 0, sizeof(b));
 	isc_buffer_clear(&source);
 
@@ -358,12 +358,12 @@ main(int argc, char *argv[]) {
 
 	dns_message_renderend(message);
 
-	message->from_to_wire = DNS_MESSAGE_INTENT_PARSE;
+	message->from_to_wire = DNS_MESSAGE_INTENTPARSE;
 	dns_message_destroy(&message);
 
 	isc_mem_stats(mctx, stdout);
 
-	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENT_PARSE);
+	result = dns_message_create(mctx, &message, DNS_MESSAGE_INTENTPARSE);
 	CHECKRESULT(result, "dns_message_create failed");
 
 	result = dns_message_parse(message, &source);

lib/dns/include/dns/message.h

@@ -63,12 +63,10 @@ ISC_LANG_BEGINDECLS
 #define DNS_MESSAGEFLAG_RD		0x0100U
 #define DNS_MESSAGEFLAG_RA		0x0080U
 
-#define DNS_MESSAGE_OPCODE_MASK		0x7000U
-#define DNS_MESSAGE_OPCODE_SHIFT	    11
-#define DNS_MESSAGE_RCODE_MASK		0x000fU
-#define DNS_MESSAGE_FLAG_MASK		0x8ff0U
+#define DNS_MESSAGE_HEADERLEN		    12 /* 6 u_int16_t's */
 
-#define DNS_MESSAGE_HEADER_LEN		    12 /* 6 u_int16_t's */
+#define MESSAGE_MAGIC		0x4d534740U	/* MSG@ */
+#define VALID_MESSAGE(msg)	(((msg)->magic) == MESSAGE_MAGIC)
 
 /*
  * Ordering here matters.  DNS_SECTION_ANY must be the lowest and negative,
@@ -93,13 +91,14 @@ typedef int dns_section_t;
 /*
  * These tell the message library how the created dns_message_t will be used.
  */
-#define DNS_MESSAGE_INTENT_UNKNOWN	0 /* internal use only */
-#define DNS_MESSAGE_INTENT_PARSE	1 /* parsing messages */
-#define DNS_MESSAGE_INTENT_RENDER	2 /* rendering */
+#define DNS_MESSAGE_INTENTUNKNOWN	0 /* internal use only */
+#define DNS_MESSAGE_INTENTPARSE		1 /* parsing messages */
+#define DNS_MESSAGE_INTENTRENDER	2 /* rendering */
 
 typedef struct dns_msgblock dns_msgblock_t;
 
 typedef struct {
+	/* public from here down */
 	unsigned int			magic;
 
 	unsigned int			id;
@@ -110,6 +109,8 @@ typedef struct {
 
 	/* 4 real, 1 pseudo */
 	unsigned int			counts[DNS_SECTION_MAX];
+
+	/* private from here down */
 	dns_namelist_t			sections[DNS_SECTION_MAX];
 	dns_name_t		       *cursors[DNS_SECTION_MAX];
 	dns_rdata_t		       *opt;
@@ -135,17 +136,18 @@ typedef struct {
 } dns_message_t;
 
 dns_result_t
-dns_message_create(isc_mem_t *mctx, dns_message_t **msg, unsigned int intent);
+dns_message_create(isc_mem_t *mctx, dns_message_t **msgp,
+		   unsigned int intent);
 /*
- * Initialize msg structure.  Must be called on a new (or reused) structure.
+ * Create msg structure.
  *
  * This function will allocate some internal blocks of memory that are
- * exptected to be needed for parsing or rendering nearly any type of message.
+ * expected to be needed for parsing or rendering nearly any type of message.
  *
  * Requires:
  *	'mctx' be a valid memory context.
  *
- *	'msg' be non-null and '*msg' be NULL.
+ *	'msgp' be non-null and '*msg' be NULL.
  *
  *	'intent' must be one of DNS_MESSAGE_INTENT_PARSE or
  *	DNS_MESSAGE_INTENT_RENDER.
@@ -167,27 +169,25 @@ dns_message_reset(dns_message_t *msg);
  * way to call dns_message_destroy() followed by dns_message_allocate(),
  * since it avoid many memory allocations.
  *
+ * If any data loanouts (buffers, names, rdatas, etc) were requested,
+ * the caller must no longer use them after this call.
+ *
  * Requires:
  *
  *	'msg' be valid.
- *
- *	If any data loanouts (buffers, names, rdatas, etc) were requested,
- *	the caller must no longer use them after this call.
  */
 
 void
-dns_message_destroy(dns_message_t **msg);
+dns_message_destroy(dns_message_t **msgp);
 /*
  * Destroy all state in the message.
  *
  * Requires:
  *
- *	'msg' be valid.
- *
- *	'msg' be "empty" with no message elements on the internal lists.
+ *	'msgp' be valid.
  *
  * Ensures:
- *	'msg' can be reused via re-initialization with dns_message_init()
+ *	'*msgp' == NULL
  */
 
 dns_result_t
@@ -209,7 +209,7 @@ dns_message_parse(dns_message_t *msg, isc_buffer_t *source);
  * Requires:
  *	"msg" be valid.
  *
- *	"buffer" have "sane" contents.
+ *	"buffer" be a wire format binary buffer.
  *
  * Ensures:
  *	The buffer's data format is correct.
@@ -221,7 +221,8 @@ dns_message_parse(dns_message_t *msg, isc_buffer_t *source);
  *	DNS_R_SUCCESS		-- all is well
  *	DNS_R_NOMEMORY		-- no memory
  *	DNS_R_MOREDATA		-- more packets needed for complete message
- *	DNS_R_???		-- bad signature (XXX need more of these)
+ *	DNS_R_???		-- bad signature (XXXMLG need more of these)
+ *	Many other errors possible XXXMLG
  */
 
 dns_result_t
@@ -230,13 +231,23 @@ dns_message_renderbegin(dns_message_t *msg, isc_buffer_t *buffer);
  * Begin rendering on a message.  Only one call can be made to this function
  * per message.
  *
+ * The buffer is "owned" buy the message library until dns_message_renderend()
+ * is called.
+ *
  * Requires:
  *
  *	'msg' be valid.
  *
- *	buffer != NULL.
+ *	buffer is a valid binary buffer.
+ *
+ * Side Effects:
+ *
+ *	The buffer is cleared before it is used.
  *
- *	buffer is empty.
+ * Returns:
+ *	DNS_R_SUCCESS		-- all is well
+ *	DNS_R_NOSPACE		-- output buffer is too small
+ *	Anything that dns_compress_init() can return.
  */
 
 dns_result_t
@@ -245,6 +256,10 @@ dns_message_renderchangebuffer(dns_message_t *msg, isc_buffer_t *buffer);
  * Reset the buffer.  This can be used after growing the old buffer
  * on a DNS_R_NOSPACE return from most of the render functions.
  *
+ * On successful completion, the old buffer is no longer used by the
+ * library.  The new buffer is owned by the library until
+ * dns_message_renderend() is called.
+ *
  * Requires:
  *
  *	'msg' be valid.
@@ -254,7 +269,6 @@ dns_message_renderchangebuffer(dns_message_t *msg, isc_buffer_t *buffer);
  *	buffer != NULL.
  *
  * Returns:
- *
  *	DNS_R_NOSPACE		-- new buffer is too small
  *	DNS_R_SUCCESS		-- all is well.
  */
@@ -262,6 +276,9 @@ dns_message_renderchangebuffer(dns_message_t *msg, isc_buffer_t *buffer);
 dns_result_t
 dns_message_renderreserve(dns_message_t *msg, unsigned int space);
 /*
+ * XXXMLG should use size_t rather than unsigned int once the buffer
+ * API is cleaned up
+ *
  * Reserve "space" bytes in the given buffer.
  *
  * Requires:
@@ -271,7 +288,6 @@ dns_message_renderreserve(dns_message_t *msg, unsigned int space);
  *	dns_message_renderbegin() was called.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- all is well.
  *	DNS_R_NOSPACE		-- not enough free space in the buffer.
  */
@@ -279,6 +295,9 @@ dns_message_renderreserve(dns_message_t *msg, unsigned int space);
 dns_result_t
 dns_message_renderrelease(dns_message_t *msg, unsigned int space);
 /*
+ * XXXMLG should use size_t rather than unsigned int once the buffer
+ * API is cleaned up
+ *
  * Release "space" bytes in the given buffer that was previously reserved.
  *
  * Requires:
@@ -288,14 +307,13 @@ dns_message_renderrelease(dns_message_t *msg, unsigned int space);
  *	dns_message_renderbegin() was called.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- all is well.
  *	DNS_R_NOSPACE		-- trying to release more than was reserved.
  */
 
 dns_result_t
 dns_message_rendersection(dns_message_t *msg, dns_section_t section,
-			  unsigned int priority, unsigned int flags);
+			  unsigned int priority, unsigned int options);
 /*
  * Render all names, rdatalists, etc from the given section at the
  * specified priority or higher.
@@ -305,9 +323,6 @@ dns_message_rendersection(dns_message_t *msg, dns_section_t section,
  *
  *	'section' be a valid section.
  *
- *	'buffer' be non-NULL and be initialized to point to a valid memory
- *	block.
- *
  *	dns_message_renderbegin() was called.
  *
  * Returns:
@@ -333,7 +348,6 @@ dns_message_renderend(dns_message_t *msg);
  *	dns_message_renderbegin() was called.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- all is well.
  */
 		      
@@ -350,7 +364,6 @@ dns_message_firstname(dns_message_t *msg, dns_section_t section);
  *	'section' be a valid section.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- All is well.
  *	DNS_R_NOMORE		-- No names on given section.
  */
@@ -371,7 +384,6 @@ dns_message_nextname(dns_message_t *msg, dns_section_t section);
  *	and the result was DNS_R_SUCCESS.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- All is well.
  *	DNS_R_NOMORE		-- No names in given section.
  */
@@ -399,7 +411,7 @@ dns_message_currentname(dns_message_t *msg, dns_section_t section,
 dns_result_t
 dns_message_findname(dns_message_t *msg, dns_section_t section,
 		     dns_name_t *target, dns_rdatatype_t type,
-		     dns_name_t **name, dns_rdataset_t **rdataset);
+		     dns_name_t **foundname, dns_rdataset_t **rdataset);
 /*
  * Search for a name in the specified section.  If it is found, *name is
  * set to point to the name, and *rdataset is set to point to the found
@@ -410,8 +422,8 @@ dns_message_findname(dns_message_t *msg, dns_section_t section,
  *
  *	'section' be a valid section.
  *
- *	If a pointer to the name is desired, 'name' should be non-NULL.
- *	If it is non-NULL, '*name' MUST be NULL.
+ *	If a pointer to the name is desired, 'foundname' should be non-NULL.
+ *	If it is non-NULL, '*foundname' MUST be NULL.
  *
  *	If a type other than dns_datatype_any is searched for, 'rdataset'
  *	may be non-NULL, '*rdataset' be NULL, and will point at the found
@@ -422,7 +434,6 @@ dns_message_findname(dns_message_t *msg, dns_section_t section,
  *	'type' be a valid type.
  *
  * Returns:
- *
  *	DNS_R_SUCCESS		-- all is well.
  *	DNS_R_NXDOMAIN		-- name does not exist in that section.
  *	DNS_R_NXRDATASET	-- The name does exist, but the desired
@@ -440,13 +451,11 @@ dns_message_movename(dns_message_t *msg, dns_name_t *name,
  *
  *	'msg' be valid.
  *
- *	'name' must be in 'fromsection'.
+ *	'name' must be a name already in 'fromsection'.
  *
  *	'fromsection' must be a valid section.
  *
- *	'tosection' must be a valid section, and be renderable.
- *
- *	'fromsection' and 'tosection' cannot be the same section.
+ *	'tosection' must be a valid section.
  */
 
 void
@@ -455,8 +464,8 @@ dns_message_addname(dns_message_t *msg, dns_name_t *name,
 /*
  * Adds the name to the given section.
  *
- * Caller must ensure that the name does not already exist.  This condition
- * is NOT checked for by this function.
+ * It is the caller's responsibility to enforce any unique name requirements
+ * in a section.
  *
  * Requires:
  *

lib/dns/message.c

@@ -37,8 +37,10 @@
 #include <dns/rdatalist.h>
 #include <dns/compress.h>
 
-#define MESSAGE_MAGIC		0x4d534740U	/* MSG@ */
-#define VALID_MESSAGE(msg)	(((msg)->magic) == MESSAGE_MAGIC)
+#define DNS_MESSAGE_OPCODE_MASK		0x7000U
+#define DNS_MESSAGE_OPCODE_SHIFT	    11
+#define DNS_MESSAGE_RCODE_MASK		0x000fU
+#define DNS_MESSAGE_FLAG_MASK		0x8ff0U
 
 #define VALID_NAMED_SECTION(s)  (((s) > DNS_SECTION_ANY) \
 				 && ((s) < DNS_SECTION_MAX))
@@ -48,6 +50,7 @@
 /*
  * This is the size of each individual scratchpad buffer, and the numbers
  * of various block allocations used within the server.
+ * XXXMLG These should come from a config setting.
  */
 #define SCRATCHPAD_SIZE		512
 #define NAME_COUNT		  8
@@ -61,13 +64,13 @@
  * size, or the allocated elements will not be alligned correctly.
  */
 struct dns_msgblock {
-	unsigned int			length;
+	unsigned int			count;
 	unsigned int			remaining;
 	ISC_LINK(dns_msgblock_t)	link;
 }; /* dynamically sized */
 
-static inline void
-msgblock_free(isc_mem_t *, dns_msgblock_t *);
+static inline dns_msgblock_t *
+msgblock_allocate(isc_mem_t *, unsigned int, unsigned int);
 
 #define msgblock_get(block, type) \
 	((type *)msgblock_internalget(block, sizeof(type)))
@@ -76,10 +79,10 @@ static inline void *
 msgblock_internalget(dns_msgblock_t *, unsigned int);
 
 static inline void
-msgblock_reset(dns_msgblock_t *, unsigned int);
+msgblock_reset(dns_msgblock_t *);
 
-static inline dns_msgblock_t *
-msgblock_allocate(isc_mem_t *, unsigned int, unsigned int);
+static inline void
+msgblock_free(isc_mem_t *, dns_msgblock_t *, unsigned int);
 
 /*
  * Allocate a new dns_msgblock_t, and return a pointer to it.  If no memory
@@ -96,9 +99,9 @@ msgblock_allocate(isc_mem_t *mctx, unsigned int sizeof_type,
 
 	block = isc_mem_get(mctx, length);
 	if (block == NULL)
-		return NULL;
+		return (NULL);
 
-	block->length = length;
+	block->count = count;
 	block->remaining = count;
 
 	ISC_LINK_INIT(block, link);
@@ -128,18 +131,23 @@ msgblock_internalget(dns_msgblock_t *block, unsigned int sizeof_type)
 }
 
 static inline void
-msgblock_reset(dns_msgblock_t *block, unsigned int count)
+msgblock_reset(dns_msgblock_t *block)
 {
-	block->remaining = count;
+	block->remaining = block->count;
 }
 
 /*
  * Release memory associated with a message block.
  */
 static inline void
-msgblock_free(isc_mem_t *mctx, dns_msgblock_t *block)
+msgblock_free(isc_mem_t *mctx, dns_msgblock_t *block,
+	      unsigned int sizeof_type)
 {
-	isc_mem_put(mctx, block, block->length);
+	unsigned int length;
+
+	length = sizeof(dns_msgblock_t) + (sizeof_type * block->count);
+
+	isc_mem_put(mctx, block, length);
 }
 
 /*
@@ -169,6 +177,7 @@ currentbuffer(dns_message_t *msg)
 	isc_dynbuffer_t *dynbuf;
 
 	dynbuf = ISC_LIST_TAIL(msg->scratchpad);
+	INSIST(dynbuf != NULL);
 
 	return (&dynbuf->buffer);
 }
@@ -191,7 +200,7 @@ newname(dns_message_t *msg)
 		return (name);
 	}
 
-	msgblock = ISC_LIST_HEAD(msg->names);
+	msgblock = ISC_LIST_TAIL(msg->names);
 	name = msgblock_get(msgblock, dns_name_t);
 	if (name == NULL) {
 		msgblock = msgblock_allocate(msg->mctx, sizeof(dns_name_t),
@@ -225,7 +234,7 @@ newrdata(dns_message_t *msg)
 		return (rdata);
 	}
 
-	msgblock = ISC_LIST_HEAD(msg->rdatas);
+	msgblock = ISC_LIST_TAIL(msg->rdatas);
 	rdata = msgblock_get(msgblock, dns_rdata_t);
 	if (rdata == NULL) {
 		msgblock = msgblock_allocate(msg->mctx, sizeof(dns_rdata_t),
@@ -259,7 +268,7 @@ newrdatalist(dns_message_t *msg)
 		return (rdatalist);
 	}
 
-	msgblock = ISC_LIST_HEAD(msg->rdatalists);
+	msgblock = ISC_LIST_TAIL(msg->rdatalists);
 	rdatalist = msgblock_get(msgblock, dns_rdatalist_t);
 	if (rdatalist == NULL) {
 		msgblock = msgblock_allocate(msg->mctx,
@@ -294,7 +303,7 @@ newrdataset(dns_message_t *msg)
 		return (rdataset);
 	}
 
-	msgblock = ISC_LIST_HEAD(msg->rdatasets);
+	msgblock = ISC_LIST_TAIL(msg->rdatasets);
 	rdataset = msgblock_get(msgblock, dns_rdataset_t);
 	if (rdataset == NULL) {
 		msgblock = msgblock_allocate(msg->mctx, sizeof(dns_rdataset_t),
@@ -338,7 +347,7 @@ msginit(dns_message_t *m)
 	m->nextrdataset = NULL;
 	m->nextrdatalist = NULL;
 
-	m->opt = NULL;
+	m->reserved = 0;
 
 	m->buffer = NULL;
 	m->need_cctx_cleanup = ISC_FALSE;
@@ -384,7 +393,7 @@ msgreset(dns_message_t *msg, isc_boolean_t everything)
 
 	dynbuf = ISC_LIST_HEAD(msg->scratchpad);
 	INSIST(dynbuf != NULL);
-	if (everything == ISC_FALSE) {
+	if (!everything) {
 		isc_dynbuffer_reset(dynbuf);
 		dynbuf = ISC_LIST_NEXT(dynbuf, link);
 	}
@@ -397,60 +406,56 @@ msgreset(dns_message_t *msg, isc_boolean_t everything)
 
 	msgblock = ISC_LIST_HEAD(msg->names);
 	INSIST(msgblock != NULL);
-	if (everything == ISC_FALSE) {
-		msgblock_reset(msgblock, NAME_COUNT);
+	if (!everything) {
+		msgblock_reset(msgblock);
 		msgblock = ISC_LIST_NEXT(msgblock, link);
 	}
 	while (msgblock != NULL) {
 		next_msgblock = ISC_LIST_NEXT(msgblock, link);
 		ISC_LIST_UNLINK(msg->names, msgblock, link);
-		msgblock_free(msg->mctx, msgblock);
+		msgblock_free(msg->mctx, msgblock, sizeof(dns_name_t));
 		msgblock = next_msgblock;
 	}
 
 	msgblock = ISC_LIST_HEAD(msg->rdatas);
 	INSIST(msgblock != NULL);
-	if (everything == ISC_FALSE) {
-		msgblock_reset(msgblock, RDATA_COUNT);
+	if (!everything) {
+		msgblock_reset(msgblock);
 		msgblock = ISC_LIST_NEXT(msgblock, link);
 	}
 	while (msgblock != NULL) {
 		next_msgblock = ISC_LIST_NEXT(msgblock, link);
 		ISC_LIST_UNLINK(msg->rdatas, msgblock, link);
-		msgblock_free(msg->mctx, msgblock);
+		msgblock_free(msg->mctx, msgblock, sizeof(dns_rdata_t));
 		msgblock = next_msgblock;
 	}
 
 	msgblock = ISC_LIST_HEAD(msg->rdatasets);
 	INSIST(msgblock != NULL);
-	if (everything == ISC_FALSE) {
-		msgblock_reset(msgblock, RDATASET_COUNT);
+	if (!everything) {
+		msgblock_reset(msgblock);
 		msgblock = ISC_LIST_NEXT(msgblock, link);
 	}
 	while (msgblock != NULL) {
 		next_msgblock = ISC_LIST_NEXT(msgblock, link);
 		ISC_LIST_UNLINK(msg->rdatasets, msgblock, link);
-		msgblock_free(msg->mctx, msgblock);
+		msgblock_free(msg->mctx, msgblock, sizeof(dns_rdataset_t));
 		msgblock = next_msgblock;
 	}
 
 	/*
-	 * "rdatalists" is special, since it is possible (but not recommended)
-	 * that the message could be switched from parse into render mode.
-	 *
-	 * Note that the reverse is not possible -- switching to parse after
-	 * render will not work, and should not.
+	 * rdatalists could be empty.
 	 */
 
 	msgblock = ISC_LIST_HEAD(msg->rdatalists);
-	if (everything == ISC_FALSE) {
-		msgblock_reset(msgblock, RDATALIST_COUNT);
+	if (!everything && msgblock != NULL) {
+		msgblock_reset(msgblock);
 		msgblock = ISC_LIST_NEXT(msgblock, link);
 	}
 	while (msgblock != NULL) {
 		next_msgblock = ISC_LIST_NEXT(msgblock, link);
 		ISC_LIST_UNLINK(msg->rdatalists, msgblock, link);
-		msgblock_free(msg->mctx, msgblock);
+		msgblock_free(msg->mctx, msgblock, sizeof(dns_rdatalist_t));
 		msgblock = next_msgblock;
 	}
 
@@ -460,7 +465,8 @@ msgreset(dns_message_t *msg, isc_boolean_t everything)
 	/*
 	 * Set other bits to normal default values.
 	 */
-	msginit(msg);
+	if (!everything)
+		msginit(msg);
 }
 
 dns_result_t
@@ -475,8 +481,8 @@ dns_message_create(isc_mem_t *mctx, dns_message_t **msg, unsigned int intent)
 	REQUIRE(mctx != NULL);
 	REQUIRE(msg != NULL);
 	REQUIRE(*msg == NULL);
-	REQUIRE(intent == DNS_MESSAGE_INTENT_PARSE
-		|| intent == DNS_MESSAGE_INTENT_RENDER);
+	REQUIRE(intent == DNS_MESSAGE_INTENTPARSE
+		|| intent == DNS_MESSAGE_INTENTRENDER);
 
 	m = isc_mem_get(mctx, sizeof(dns_message_t));
 	if (m == NULL)
@@ -518,7 +524,7 @@ dns_message_create(isc_mem_t *mctx, dns_message_t **msg, unsigned int intent)
 		goto cleanup4;
 	ISC_LIST_APPEND(m->rdatasets, msgblock, link);
 
-	if (intent == DNS_MESSAGE_INTENT_PARSE) {
+	if (intent == DNS_MESSAGE_INTENTPARSE) {
 		msgblock = msgblock_allocate(mctx, sizeof(dns_rdatalist_t),
 					     RDATALIST_COUNT);
 		if (msgblock == NULL)
@@ -534,13 +540,13 @@ dns_message_create(isc_mem_t *mctx, dns_message_t **msg, unsigned int intent)
 	 */
  cleanup5:
 	msgblock = ISC_LIST_HEAD(m->rdatasets);
-	msgblock_free(mctx, msgblock);
+	msgblock_free(mctx, msgblock, sizeof(dns_rdataset_t));
  cleanup4:
 	msgblock = ISC_LIST_HEAD(m->rdatas);
-	msgblock_free(mctx, msgblock);
+	msgblock_free(mctx, msgblock, sizeof(dns_rdata_t));
  cleanup3:
 	msgblock = ISC_LIST_HEAD(m->names);
-	msgblock_free(mctx, msgblock);
+	msgblock_free(mctx, msgblock, sizeof(dns_name_t));
  cleanup2:
 	dynbuf = ISC_LIST_HEAD(m->scratchpad);
 	isc_dynbuffer_free(mctx, &dynbuf);
@@ -622,11 +628,16 @@ getname(dns_name_t *name, isc_buffer_t *source, dns_message_t *msg,
 
 	scratch = currentbuffer(msg);
 
+	/* XXXMLG Can this be done just once? */
 	if (dns_decompress_edns(dctx) > 1 || !dns_decompress_strict(dctx))
 		dns_decompress_setmethods(dctx, DNS_COMPRESS_GLOBAL);
 	else
 		dns_decompress_setmethods(dctx, DNS_COMPRESS_GLOBAL14);
 
+	/*
+	 * First try:  use current buffer.
+	 * Second try:  allocate a new buffer and use that.
+	 */
 	tries = 0;
 	while (tries < 2) {
 		dns_name_init(name, NULL);
@@ -646,7 +657,8 @@ getname(dns_name_t *name, isc_buffer_t *source, dns_message_t *msg,
 		}
 	}
 
-	return (DNS_R_UNEXPECTED);  /* should never get here... XXXMLG */
+	INSIST(0);  /* Cannot get here... */
+	return (DNS_R_UNEXPECTED);
 }
 
 static dns_result_t
@@ -663,6 +675,10 @@ getrdata(dns_name_t *name, isc_buffer_t *source, dns_message_t *msg,
 	isc_buffer_setactive(source, rdatalen);
 	dns_decompress_localinit(dctx, name, source);
 
+	/*
+	 * First try:  use current buffer.
+	 * Second try:  allocate a new buffer and use that.
+	 */
 	tries = 0;
 	while (tries < 2) {
 		result = dns_rdata_fromwire(rdata, rdclass, rdtype,
@@ -682,7 +698,8 @@ getrdata(dns_name_t *name, isc_buffer_t *source, dns_message_t *msg,
 		}
 	}
 
-	return (DNS_R_UNEXPECTED);  /* should never get here... XXXMLG */
+	INSIST(0);  /* Cannot get here... */
+	return (DNS_R_UNEXPECTED);
 }