Don't allocate memory for operations that do not insert

Almost every operation performed on an nvlist was allocating a
new string to hold the key name.  The nvlist_exists* family of
functions would always return false if they failed to allocate
the string.  The rest of the functions would outright abort().
Fix the non-varargs variants of the functions to perform the
requested operations directly and the varargs versions to
allocate the string and call into the non-varargs versions.
The varargs versions are still broken and really can't be fixed,
so we might consider axing them entirely.  However, now the non-
varargs functions are always safe to call.

Differential Revision:		https://reviews.freebsd.org/D1879
Reviewed by:			pjd, jfv
MFC after:			1 month
Sponsored by:			Sandvine Inc.

lib/libnv/dnvlist.c

@@ -33,6 +33,7 @@ __FBSDID("$FreeBSD$");
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdint.h>
+#include <stdlib.h>
 
 #include "nv.h"
 #include "nv_impl.h"
@@ -44,7 +45,10 @@ ftype									\
 dnvlist_get_##type(const nvlist_t *nvl, const char *name, ftype defval)	\
 {									\
 									\
-	return (dnvlist_getf_##type(nvl, defval, "%s", name));		\
+	if (nvlist_exists_##type(nvl, name))				\
+		return (nvlist_get_##type(nvl, name));			\
+	else								\
+		return (defval);					\
 }
 
 DNVLIST_GET(bool, bool)
@@ -59,8 +63,16 @@ const void *
 dnvlist_get_binary(const nvlist_t *nvl, const char *name, size_t *sizep,
     const void *defval, size_t defsize)
 {
+	const void *value;
 
-	return (dnvlist_getf_binary(nvl, sizep, defval, defsize, "%s", name));
+	if (nvlist_exists_binary(nvl, name))
+		value = nvlist_get_binary(nvl, name, sizep);
+	else {
+		if (sizep != NULL)
+			*sizep = defsize;
+		value = defval;
+	}
+	return (value);
 }
 
 #define	DNVLIST_GETF(ftype, type)					\
@@ -106,15 +118,14 @@ ftype									\
 dnvlist_getv_##type(const nvlist_t *nvl, ftype defval,			\
     const char *namefmt, va_list nameap)				\
 {									\
-	va_list cnameap;						\
+	char *name;							\
 	ftype value;							\
 									\
-	va_copy(cnameap, nameap);					\
+	vasprintf(&name, namefmt, nameap);				\
-	if (nvlist_existsv_##type(nvl, namefmt, cnameap))		\
+	if (name == NULL)						\
-		value = nvlist_getv_##type(nvl, namefmt, nameap);	\
+		return (defval);					\
-	else								\
+	value = dnvlist_get_##type(nvl, name, defval);			\
-		value = defval;						\
+	free(name);							\
-	va_end(cnameap);						\
 	return (value);							\
 }
 
@@ -130,18 +141,18 @@ const void *
 dnvlist_getv_binary(const nvlist_t *nvl, size_t *sizep, const void *defval,
     size_t defsize, const char *namefmt, va_list nameap)
 {
-	va_list cnameap;
+	char *name;
 	const void *value;
 
-	va_copy(cnameap, nameap);
+	vasprintf(&name, namefmt, nameap);
-	if (nvlist_existsv_binary(nvl, namefmt, cnameap)) {
+	if (name != NULL) {
-		value = nvlist_getv_binary(nvl, sizep, namefmt, nameap);
+		value = dnvlist_get_binary(nvl, name, sizep, defval, defsize);
+		free(name);
 	} else {
 		if (sizep != NULL)
 			*sizep = defsize;
 		value = defval;
 	}
-	va_end(cnameap);
 	return (value);
 }
 
@@ -150,7 +161,10 @@ ftype									\
 dnvlist_take_##type(nvlist_t *nvl, const char *name, ftype defval)	\
 {									\
 									\
-	return (dnvlist_takef_##type(nvl, defval, "%s", name));		\
+	if (nvlist_exists_##type(nvl, name))				\
+		return (nvlist_take_##type(nvl, name));			\
+	else								\
+		return (defval);					\
 }
 
 DNVLIST_TAKE(bool, bool)
@@ -165,8 +179,16 @@ void *
 dnvlist_take_binary(nvlist_t *nvl, const char *name, size_t *sizep,
     void *defval, size_t defsize)
 {
+	void *value;
 
-	return (dnvlist_takef_binary(nvl, sizep, defval, defsize, "%s", name));
+	if (nvlist_exists_binary(nvl, name))
+		value = nvlist_take_binary(nvl, name, sizep);
+	else {
+		if (sizep != NULL)
+			*sizep = defsize;
+		value = defval;
+	}
+	return (value);
 }
 
 #define	DNVLIST_TAKEF(ftype, type)					\
@@ -212,15 +234,14 @@ ftype									\
 dnvlist_takev_##type(nvlist_t *nvl, ftype defval, const char *namefmt,	\
     va_list nameap)							\
 {									\
-	va_list cnameap;						\
+	char *name;							\
 	ftype value;							\
 									\
-	va_copy(cnameap, nameap);					\
+	vasprintf(&name, namefmt, nameap);				\
-	if (nvlist_existsv_##type(nvl, namefmt, cnameap))		\
+	if (name == NULL)						\
-		value = nvlist_takev_##type(nvl, namefmt, nameap);	\
+		return (defval);					\
-	else								\
+	value = dnvlist_take_##type(nvl, name, defval);			\
-		value = defval;						\
+	free(name);							\
-	va_end(cnameap);						\
 	return (value);							\
 }
 
@@ -236,17 +257,18 @@ void *
 dnvlist_takev_binary(nvlist_t *nvl, size_t *sizep, void *defval,
     size_t defsize, const char *namefmt, va_list nameap)
 {
-	va_list cnameap;
+	char *name;
 	void *value;
 
-	va_copy(cnameap, nameap);
+	vasprintf(&name, namefmt, nameap);
-	if (nvlist_existsv_binary(nvl, namefmt, cnameap)) {
+	if (name != NULL) {
-		value = nvlist_takev_binary(nvl, sizep, namefmt, nameap);
+		value = dnvlist_take_binary(nvl, name, sizep, defval, defsize);
+		free(name);
 	} else {
 		if (sizep != NULL)
 			*sizep = defsize;
 		value = defval;
 	}
-	va_end(cnameap);
+
 	return (value);
 }

lib/libnv/nv_impl.h

@@ -97,14 +97,6 @@ const void	*nvpair_get_binary(const nvpair_t *nvp, size_t *sizep);
 
 void nvpair_free(nvpair_t *nvp);
 
-const nvpair_t *nvlist_getf_nvpair(const nvlist_t *nvl, const char *namefmt, ...) __printflike(2, 3);
-
-const nvpair_t *nvlist_getv_nvpair(const nvlist_t *nvl, const char *namefmt, va_list nameap) __printflike(2, 0);
-
-nvpair_t *nvlist_takef_nvpair(nvlist_t *nvl, const char *namefmt, ...) __printflike(2, 3);
-
-nvpair_t *nvlist_takev_nvpair(nvlist_t *nvl, const char *namefmt, va_list nameap) __printflike(2, 0);
-
 nvpair_t *nvpair_createf_null(const char *namefmt, ...) __printflike(1, 2);
 nvpair_t *nvpair_createf_bool(bool value, const char *namefmt, ...) __printflike(2, 3);
 nvpair_t *nvpair_createf_number(uint64_t value, const char *namefmt, ...) __printflike(2, 3);

lib/libnv/nvlist.c

@@ -208,29 +208,23 @@ nvlist_empty(const nvlist_t *nvl)
 }
 
 static void
-nvlist_report_missing(int type, const char *namefmt, va_list nameap)
+nvlist_report_missing(int type, const char *name)
 {
-	char *name;
 
-	vasprintf(&name, namefmt, nameap);
 	PJDLOG_ABORT("Element '%s' of type %s doesn't exist.",
-	    name != NULL ? name : "N/A", nvpair_type_string(type));
+	    name, nvpair_type_string(type));
 }
 
 static nvpair_t *
-nvlist_findv(const nvlist_t *nvl, int type, const char *namefmt, va_list nameap)
+nvlist_find(const nvlist_t *nvl, int type, const char *name)
 {
 	nvpair_t *nvp;
-	char *name;
 
 	NVLIST_ASSERT(nvl);
 	PJDLOG_ASSERT(nvl->nvl_error == 0);
 	PJDLOG_ASSERT(type == NV_TYPE_NONE ||
 	    (type >= NV_TYPE_FIRST && type <= NV_TYPE_LAST));
 
-	if (vasprintf(&name, namefmt, nameap) < 0)
-		return (NULL);
-
 	for (nvp = nvlist_first_nvpair(nvl); nvp != NULL;
 	    nvp = nvlist_next_nvpair(nvl, nvp)) {
 		if (type != NV_TYPE_NONE && nvpair_type(nvp) != type)
@@ -245,8 +239,6 @@ nvlist_findv(const nvlist_t *nvl, int type, const char *namefmt, va_list nameap)
 		break;
 	}
 
-	free(name);
-
 	if (nvp == NULL)
 		errno = ENOENT;
 
@@ -257,7 +249,12 @@ bool
 nvlist_exists_type(const nvlist_t *nvl, const char *name, int type)
 {
 
-	return (nvlist_existsf_type(nvl, type, "%s", name));
+	NVLIST_ASSERT(nvl);
+	PJDLOG_ASSERT(nvl->nvl_error == 0);
+	PJDLOG_ASSERT(type == NV_TYPE_NONE ||
+	    (type >= NV_TYPE_FIRST && type <= NV_TYPE_LAST));
+
+	return (nvlist_find(nvl, type, name) != NULL);
 }
 
 bool
@@ -277,20 +274,33 @@ bool
 nvlist_existsv_type(const nvlist_t *nvl, int type, const char *namefmt,
     va_list nameap)
 {
+	char *name;
+	bool exists;
+
+	vasprintf(&name, namefmt, nameap);
+	if (name == NULL)
+		return (false);
+
+	exists = nvlist_exists_type(nvl, name, type);
+	free(name);
+	return (exists);
+}
+
+void
+nvlist_free_type(nvlist_t *nvl, const char *name, int type)
+{
+	nvpair_t *nvp;
 
 	NVLIST_ASSERT(nvl);
 	PJDLOG_ASSERT(nvl->nvl_error == 0);
 	PJDLOG_ASSERT(type == NV_TYPE_NONE ||
 	    (type >= NV_TYPE_FIRST && type <= NV_TYPE_LAST));
 
-	return (nvlist_findv(nvl, type, namefmt, nameap) != NULL);
+	nvp = nvlist_find(nvl, type, name);
-}
+	if (nvp != NULL)
-
+		nvlist_free_nvpair(nvl, nvp);
-void
+	else
-nvlist_free_type(nvlist_t *nvl, const char *name, int type)
+		nvlist_report_missing(type, name);
-{
-
-	nvlist_freef_type(nvl, type, "%s", name);
 }
 
 void
@@ -306,21 +316,13 @@ nvlist_freef_type(nvlist_t *nvl, int type, const char *namefmt, ...)
 void
 nvlist_freev_type(nvlist_t *nvl, int type, const char *namefmt, va_list nameap)
 {
-	va_list cnameap;
+	char *name;
-	nvpair_t *nvp;
 
-	NVLIST_ASSERT(nvl);
+	vasprintf(&name, namefmt, nameap);
-	PJDLOG_ASSERT(nvl->nvl_error == 0);
+	if (name == NULL)
-	PJDLOG_ASSERT(type == NV_TYPE_NONE ||
+		nvlist_report_missing(type, "<unknown>");
-	    (type >= NV_TYPE_FIRST && type <= NV_TYPE_LAST));
+	nvlist_free_type(nvl, name, type);
-
+	free(name);
-	va_copy(cnameap, nameap);
-	nvp = nvlist_findv(nvl, type, namefmt, cnameap);
-	va_end(cnameap);
-	if (nvp != NULL)
-		nvlist_free_nvpair(nvl, nvp);
-	else
-		nvlist_report_missing(type, namefmt, nameap);
 }
 
 nvlist_t *
@@ -1031,24 +1033,24 @@ bool
 nvlist_exists(const nvlist_t *nvl, const char *name)
 {
 
-	return (nvlist_existsf(nvl, "%s", name));
+	return (nvlist_find(nvl, NV_TYPE_NONE, name) != NULL);
 }
 
-#define	NVLIST_EXISTS(type)						\
+#define	NVLIST_EXISTS(type, TYPE)					\
 bool									\
 nvlist_exists_##type(const nvlist_t *nvl, const char *name)		\
 {									\
 									\
-	return (nvlist_existsf_##type(nvl, "%s", name));		\
+	return (nvlist_find(nvl, NV_TYPE_##TYPE, name) != NULL);	\
 }
 
-NVLIST_EXISTS(null)
+NVLIST_EXISTS(null, NULL)
-NVLIST_EXISTS(bool)
+NVLIST_EXISTS(bool, BOOL)
-NVLIST_EXISTS(number)
+NVLIST_EXISTS(number, NUMBER)
-NVLIST_EXISTS(string)
+NVLIST_EXISTS(string, STRING)
-NVLIST_EXISTS(nvlist)
+NVLIST_EXISTS(nvlist, NVLIST)
-NVLIST_EXISTS(descriptor)
+NVLIST_EXISTS(descriptor, DESCRIPTOR)
-NVLIST_EXISTS(binary)
+NVLIST_EXISTS(binary, BINARY)
 
 #undef	NVLIST_EXISTS
 
@@ -1090,27 +1092,41 @@ NVLIST_EXISTSF(binary)
 bool
 nvlist_existsv(const nvlist_t *nvl, const char *namefmt, va_list nameap)
 {
+	char *name;
+	bool exists;
 
-	return (nvlist_findv(nvl, NV_TYPE_NONE, namefmt, nameap) != NULL);
+	vasprintf(&name, namefmt, nameap);
+	if (name == NULL)
+		return (false);
+
+	exists = nvlist_exists(nvl, name);
+	free(name);
+	return (exists);
 }
 
-#define	NVLIST_EXISTSV(type, TYPE)					\
+#define	NVLIST_EXISTSV(type)						\
 bool									\
 nvlist_existsv_##type(const nvlist_t *nvl, const char *namefmt,		\
     va_list nameap)							\
 {									\
+	char *name;							\
+	bool exists;							\
 									\
-	return (nvlist_findv(nvl, NV_TYPE_##TYPE, namefmt, nameap) !=	\
+	vasprintf(&name, namefmt, nameap);				\
-	    NULL);							\
+	if (name == NULL)						\
+		return (false);						\
+	exists = nvlist_exists_##type(nvl, name);			\
+	free(name);							\
+	return (exists);						\
 }
 
-NVLIST_EXISTSV(null, NULL)
+NVLIST_EXISTSV(null)
-NVLIST_EXISTSV(bool, BOOL)
+NVLIST_EXISTSV(bool)
-NVLIST_EXISTSV(number, NUMBER)
+NVLIST_EXISTSV(number)
-NVLIST_EXISTSV(string, STRING)
+NVLIST_EXISTSV(string)
-NVLIST_EXISTSV(nvlist, NVLIST)
+NVLIST_EXISTSV(nvlist)
-NVLIST_EXISTSV(descriptor, DESCRIPTOR)
+NVLIST_EXISTSV(descriptor)
-NVLIST_EXISTSV(binary, BINARY)
+NVLIST_EXISTSV(binary)
 
 #undef	NVLIST_EXISTSV
 
@@ -1561,28 +1577,43 @@ nvlist_movev_binary(nvlist_t *nvl, void *value, size_t size,
 		nvlist_move_nvpair(nvl, nvp);
 }
 
-#define	NVLIST_GET(ftype, type)						\
+const nvpair_t *
+nvlist_get_nvpair(const nvlist_t *nvl, const char *name)
+{
+
+	return (nvlist_find(nvl, NV_TYPE_NONE, name));
+}
+
+#define	NVLIST_GET(ftype, type, TYPE)					\
 ftype									\
 nvlist_get_##type(const nvlist_t *nvl, const char *name)		\
 {									\
+	const nvpair_t *nvp;						\
 									\
-	return (nvlist_getf_##type(nvl, "%s", name));			\
+	nvp = nvlist_find(nvl, NV_TYPE_##TYPE, name);			\
+	if (nvp == NULL)						\
+		nvlist_report_missing(NV_TYPE_##TYPE, name);		\
+	return (nvpair_get_##type(nvp));				\
 }
 
-NVLIST_GET(const nvpair_t *, nvpair)
+NVLIST_GET(bool, bool, BOOL)
-NVLIST_GET(bool, bool)
+NVLIST_GET(uint64_t, number, NUMBER)
-NVLIST_GET(uint64_t, number)
+NVLIST_GET(const char *, string, STRING)
-NVLIST_GET(const char *, string)
+NVLIST_GET(const nvlist_t *, nvlist, NVLIST)
-NVLIST_GET(const nvlist_t *, nvlist)
+NVLIST_GET(int, descriptor, DESCRIPTOR)
-NVLIST_GET(int, descriptor)
 
 #undef	NVLIST_GET
 
 const void *
 nvlist_get_binary(const nvlist_t *nvl, const char *name, size_t *sizep)
 {
+	nvpair_t *nvp;
 
-	return (nvlist_getf_binary(nvl, sizep, "%s", name));
+	nvp = nvlist_find(nvl, NV_TYPE_BINARY, name);
+	if (nvp == NULL)
+		nvlist_report_missing(NV_TYPE_BINARY, name);
+
+	return (nvpair_get_binary(nvp, sizep));
 }
 
 #define	NVLIST_GETF(ftype, type)					\
@@ -1599,7 +1630,6 @@ nvlist_getf_##type(const nvlist_t *nvl, const char *namefmt, ...)	\
 	return (value);							\
 }
 
-NVLIST_GETF(const nvpair_t *, nvpair)
 NVLIST_GETF(bool, bool)
 NVLIST_GETF(uint64_t, number)
 NVLIST_GETF(const char *, string)
@@ -1621,27 +1651,21 @@ nvlist_getf_binary(const nvlist_t *nvl, size_t *sizep, const char *namefmt, ...)
 	return (value);
 }
 
-const nvpair_t *
-nvlist_getv_nvpair(const nvlist_t *nvl, const char *namefmt, va_list nameap)
-{
-
-	return (nvlist_findv(nvl, NV_TYPE_NONE, namefmt, nameap));
-}
-
 #define	NVLIST_GETV(ftype, type, TYPE)					\
 ftype									\
 nvlist_getv_##type(const nvlist_t *nvl, const char *namefmt,		\
     va_list nameap)							\
 {									\
-	va_list cnameap;						\
+	char *name;							\
-	const nvpair_t *nvp;						\
+	ftype value;							\
 									\
-	va_copy(cnameap, nameap);					\
+	vasprintf(&name, namefmt, nameap);				\
-	nvp = nvlist_findv(nvl, NV_TYPE_##TYPE, namefmt, cnameap);	\
+	if (name == NULL)						\
-	va_end(cnameap);						\
+		nvlist_report_missing(NV_TYPE_##TYPE, "<unknown>");	\
-	if (nvp == NULL)						\
+	value = nvlist_get_##type(nvl, name);				\
-		nvlist_report_missing(NV_TYPE_##TYPE, namefmt, nameap);	\
+	free(name);							\
-	return (nvpair_get_##type(nvp));				\
+									\
+	return (value);							\
 }
 
 NVLIST_GETV(bool, bool, BOOL)
@@ -1656,40 +1680,56 @@ const void *
 nvlist_getv_binary(const nvlist_t *nvl, size_t *sizep, const char *namefmt,
     va_list nameap)
 {
-	va_list cnameap;
+	char *name;
-	const nvpair_t *nvp;
+	const void *binary;
 
-	va_copy(cnameap, nameap);
+	vasprintf(&name, namefmt, nameap);
-	nvp = nvlist_findv(nvl, NV_TYPE_BINARY, namefmt, cnameap);
+	if (name == NULL)
-	va_end(cnameap);
+		nvlist_report_missing(NV_TYPE_BINARY, "<unknown>");
-	if (nvp == NULL)
-		nvlist_report_missing(NV_TYPE_BINARY, namefmt, nameap);
 
-	return (nvpair_get_binary(nvp, sizep));
+	binary = nvlist_get_binary(nvl, name, sizep);
+	free(name);
+	return (binary);
 }
 
-#define	NVLIST_TAKE(ftype, type)					\
+#define	NVLIST_TAKE(ftype, type, TYPE)					\
 ftype									\
 nvlist_take_##type(nvlist_t *nvl, const char *name)			\
 {									\
+	nvpair_t *nvp;							\
+	ftype value;							\
 									\
-	return (nvlist_takef_##type(nvl, "%s", name));			\
+	nvp = nvlist_find(nvl, NV_TYPE_##TYPE, name);			\
+	if (nvp == NULL)						\
+		nvlist_report_missing(NV_TYPE_##TYPE, name);		\
+	value = (ftype)(intptr_t)nvpair_get_##type(nvp);		\
+	nvlist_remove_nvpair(nvl, nvp);					\
+	nvpair_free_structure(nvp);					\
+	return (value);							\
 }
 
-NVLIST_TAKE(nvpair_t *, nvpair)
+NVLIST_TAKE(bool, bool, BOOL)
-NVLIST_TAKE(bool, bool)
+NVLIST_TAKE(uint64_t, number, NUMBER)
-NVLIST_TAKE(uint64_t, number)
+NVLIST_TAKE(char *, string, STRING)
-NVLIST_TAKE(char *, string)
+NVLIST_TAKE(nvlist_t *, nvlist, NVLIST)
-NVLIST_TAKE(nvlist_t *, nvlist)
+NVLIST_TAKE(int, descriptor, DESCRIPTOR)
-NVLIST_TAKE(int, descriptor)
 
 #undef	NVLIST_TAKE
 
 void *
 nvlist_take_binary(nvlist_t *nvl, const char *name, size_t *sizep)
 {
+	nvpair_t *nvp;
+	void *value;
 
-	return (nvlist_takef_binary(nvl, sizep, "%s", name));
+	nvp = nvlist_find(nvl, NV_TYPE_BINARY, name);
+	if (nvp == NULL)
+		nvlist_report_missing(NV_TYPE_BINARY, name);
+
+	value = (void *)(intptr_t)nvpair_get_binary(nvp, sizep);
+	nvlist_remove_nvpair(nvl, nvp);
+	nvpair_free_structure(nvp);
+	return (value);
 }
 
 #define	NVLIST_TAKEF(ftype, type)					\
@@ -1706,7 +1746,6 @@ nvlist_takef_##type(nvlist_t *nvl, const char *namefmt, ...)		\
 	return (value);							\
 }
 
-NVLIST_TAKEF(nvpair_t *, nvpair)
 NVLIST_TAKEF(bool, bool)
 NVLIST_TAKEF(uint64_t, number)
 NVLIST_TAKEF(char *, string)
@@ -1728,33 +1767,18 @@ nvlist_takef_binary(nvlist_t *nvl, size_t *sizep, const char *namefmt, ...)
 	return (value);
 }
 
-nvpair_t *
-nvlist_takev_nvpair(nvlist_t *nvl, const char *namefmt, va_list nameap)
-{
-	nvpair_t *nvp;
-
-	nvp = nvlist_findv(nvl, NV_TYPE_NONE, namefmt, nameap);
-	if (nvp != NULL)
-		nvlist_remove_nvpair(nvl, nvp);
-	return (nvp);
-}
-
 #define	NVLIST_TAKEV(ftype, type, TYPE)					\
 ftype									\
 nvlist_takev_##type(nvlist_t *nvl, const char *namefmt, va_list nameap)	\
 {									\
-	va_list cnameap;						\
+	char *name;							\
-	nvpair_t *nvp;							\
 	ftype value;							\
 									\
-	va_copy(cnameap, nameap);					\
+	vasprintf(&name, namefmt, nameap);				\
-	nvp = nvlist_findv(nvl, NV_TYPE_##TYPE, namefmt, cnameap);	\
+	if (name == NULL)						\
-	va_end(cnameap);						\
+		nvlist_report_missing(NV_TYPE_##TYPE, "<unknown>");	\
-	if (nvp == NULL)						\
+	value = nvlist_take_##type(nvl, name);				\
-		nvlist_report_missing(NV_TYPE_##TYPE, namefmt, nameap);	\
+	free(name);							\
-	value = (ftype)(intptr_t)nvpair_get_##type(nvp);		\
-	nvlist_remove_nvpair(nvl, nvp);					\
-	nvpair_free_structure(nvp);					\
 	return (value);							\
 }
 
@@ -1770,20 +1794,16 @@ void *
 nvlist_takev_binary(nvlist_t *nvl, size_t *sizep, const char *namefmt,
     va_list nameap)
 {
-	va_list cnameap;
+	char *name;
-	nvpair_t *nvp;
+	void *binary;
-	void *value;
 
-	va_copy(cnameap, nameap);
+	vasprintf(&name, namefmt, nameap);
-	nvp = nvlist_findv(nvl, NV_TYPE_BINARY, namefmt, cnameap);
+	if (name == NULL)
-	va_end(cnameap);
+		nvlist_report_missing(NV_TYPE_BINARY, "<unknown>");
-	if (nvp == NULL)
-		nvlist_report_missing(NV_TYPE_BINARY, namefmt, nameap);
 
-	value = (void *)(intptr_t)nvpair_get_binary(nvp, sizep);
+	binary = nvlist_take_binary(nvl, name, sizep);
-	nvlist_remove_nvpair(nvl, nvp);
+	free(name);
-	nvpair_free_structure(nvp);
+	return (binary);
-	return (value);
 }
 
 void
@@ -1801,24 +1821,24 @@ void
 nvlist_free(nvlist_t *nvl, const char *name)
 {
 
-	nvlist_freef(nvl, "%s", name);
+	nvlist_free_type(nvl, name, NV_TYPE_NONE);
 }
 
-#define	NVLIST_FREE(type)						\
+#define	NVLIST_FREE(type, TYPE)						\
 void									\
 nvlist_free_##type(nvlist_t *nvl, const char *name)			\
 {									\
 									\
-	nvlist_freef_##type(nvl, "%s", name);				\
+	nvlist_free_type(nvl, name, NV_TYPE_##TYPE);			\
 }
 
-NVLIST_FREE(null)
+NVLIST_FREE(null, NULL)
-NVLIST_FREE(bool)
+NVLIST_FREE(bool, BOOL)
-NVLIST_FREE(number)
+NVLIST_FREE(number, NUMBER)
-NVLIST_FREE(string)
+NVLIST_FREE(string, STRING)
-NVLIST_FREE(nvlist)
+NVLIST_FREE(nvlist, NVLIST)
-NVLIST_FREE(descriptor)
+NVLIST_FREE(descriptor, DESCRIPTOR)
-NVLIST_FREE(binary)
+NVLIST_FREE(binary, BINARY)
 
 #undef	NVLIST_FREE
 
@@ -1864,8 +1884,13 @@ nvlist_freev(nvlist_t *nvl, const char *namefmt, va_list nameap)
 void									\
 nvlist_freev_##type(nvlist_t *nvl, const char *namefmt, va_list nameap)	\
 {									\
+	char *name;							\
 									\
-	nvlist_freev_type(nvl, NV_TYPE_##TYPE, namefmt, nameap);	\
+	vasprintf(&name, namefmt, nameap);				\
+	if (name == NULL)						\
+		nvlist_report_missing(NV_TYPE_##TYPE, "<unknown>");	\
+	nvlist_free_##type(nvl, name);					\
+	free(name);							\
 }
 
 NVLIST_FREEV(null, NULL)