changeset 4328:fe6ac87d8e60

6547176 libtopo leaks memory on failure 6550982 topo_open sets null topo_hdl_t.th_product on some non-Sun x86 platforms 6554353 libtopo dumps core in prop_create() on allocation failure 6554450 libtopo hang due to locked node 6554471 topo_prop_set() has a bogus return value check 6554499 libtopo blew an assertion in topo_snapshot_destroy() 6554648 topo_prop_set_* don't set err = ETOPO_PROP_NOENT when pgroup doesn't exist 6556794 topo_mod_hcfmri() leaks memory on failure 6556795 maybe_di_chars_copy segfaults on allocation failure 6557332 hc_compare is broken 6558936 libtopo stress tests reveals failures 6558942 fmtopo does not close its topo handle on exit 6558947 topo_mod_walk_init() needs to be added to mapfile-vers 6560219 sata topo enumerator is not very robust with memory alloc failures
author cindi
date Fri, 25 May 2007 10:59:41 -0700
parents 4ca5d5f63ada
children 35ac1e4ee309
files usr/src/cmd/fm/fmtopo/common/fmtopo.c usr/src/lib/fm/topo/libtopo/common/hc.c usr/src/lib/fm/topo/libtopo/common/mapfile-vers usr/src/lib/fm/topo/libtopo/common/topo_2xml.c usr/src/lib/fm/topo/libtopo/common/topo_fmri.c usr/src/lib/fm/topo/libtopo/common/topo_mod.c usr/src/lib/fm/topo/libtopo/common/topo_mod.map usr/src/lib/fm/topo/libtopo/common/topo_module.c usr/src/lib/fm/topo/libtopo/common/topo_node.c usr/src/lib/fm/topo/libtopo/common/topo_nvl.c usr/src/lib/fm/topo/libtopo/common/topo_prop.c usr/src/lib/fm/topo/libtopo/common/topo_snap.c usr/src/lib/fm/topo/libtopo/common/topo_tree.c usr/src/lib/fm/topo/modules/common/hostbridge/hostbridge.c usr/src/lib/fm/topo/modules/common/pcibus/did_props.c usr/src/lib/fm/topo/modules/common/pcibus/pcibus.c usr/src/lib/fm/topo/modules/i86pc/hostbridge/hb_i86pc.c usr/src/lib/fm/topo/modules/i86pc/sata/sata.c usr/src/lib/fm/topo/modules/sun4/hostbridge/hb_sun4.c
diffstat 19 files changed, 276 insertions(+), 149 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/cmd/fm/fmtopo/common/fmtopo.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/cmd/fm/fmtopo/common/fmtopo.c	Fri May 25 10:59:41 2007 -0700
@@ -133,9 +133,9 @@
 		nvlist_t *fru = NULL;
 
 		if (topo_node_asru(node, &asru, NULL, &err) == 0)
-		    (void) topo_fmri_nvl2str(thp, asru, &aname, &err);
+			(void) topo_fmri_nvl2str(thp, asru, &aname, &err);
 		if (topo_node_fru(node, &fru, NULL, &err) == 0)
-		    (void) topo_fmri_nvl2str(thp, fru, &fname, &err);
+			(void) topo_fmri_nvl2str(thp, fru, &fname, &err);
 		(void) topo_node_label(node, &lname, &err);
 		if (aname != NULL) {
 			nvlist_free(asru);
@@ -249,7 +249,7 @@
 		    g_pname);
 		return;
 	} else {
-	    (void) nvpair_value_string(pv_nvp, &propn);
+		(void) nvpair_value_string(pv_nvp, &propn);
 	}
 
 	if ((pv_nvp = nvlist_next_nvpair(nvl, pv_nvp)) == NULL ||
@@ -986,9 +986,9 @@
 		nvlist_t *fru = NULL;
 
 		if (topo_fmri_asru(thp, nvl, &asru, &err) == 0)
-		    (void) topo_fmri_nvl2str(thp, asru, &aname, &err);
+			(void) topo_fmri_nvl2str(thp, asru, &aname, &err);
 		if (topo_fmri_fru(thp, nvl, &fru, &err) == 0)
-		    (void) topo_fmri_nvl2str(thp, fru, &fname, &err);
+			(void) topo_fmri_nvl2str(thp, fru, &fname, &err);
 		(void) topo_fmri_label(thp, nvl, &lname, &err);
 
 		nvlist_free(fru);
@@ -1165,7 +1165,5 @@
 		return (fmtopo_exit(thp, uuid, FMTOPO_EXIT_ERROR));
 	}
 
-	topo_hdl_strfree(thp, uuid);
-
-	return (FMTOPO_EXIT_SUCCESS);
+	return (fmtopo_exit(thp, uuid, FMTOPO_EXIT_SUCCESS));
 }
--- a/usr/src/lib/fm/topo/libtopo/common/hc.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/hc.c	Fri May 25 10:59:41 2007 -0700
@@ -353,7 +353,7 @@
 		return (topo_mod_seterrno(mod, EMOD_VER_NEW));
 
 	if (nvlist_lookup_nvlist(in, TOPO_METH_FMRI_ARG_NV1, &nv1) != 0 ||
-	    nvlist_lookup_nvlist(in, TOPO_METH_FMRI_ARG_NV1, &nv2) != 0)
+	    nvlist_lookup_nvlist(in, TOPO_METH_FMRI_ARG_NV2, &nv2) != 0)
 		return (topo_mod_seterrno(mod, EMOD_METHOD_INVAL));
 
 	ret = fmri_compare(mod, nv1, nv2);
@@ -508,8 +508,10 @@
 		return (topo_mod_seterrno(mod, EMOD_FMRI_NVL));
 	}
 
-	if (topo_mod_nvalloc(mod, &fmristr, NV_UNIQUE_NAME) != 0)
+	if (topo_mod_nvalloc(mod, &fmristr, NV_UNIQUE_NAME) != 0) {
+		topo_mod_free(mod, name, len + 1);
 		return (topo_mod_seterrno(mod, EMOD_FMRI_NVL));
+	}
 	if (nvlist_add_string(fmristr, "fmri-string", name) != 0) {
 		topo_mod_free(mod, name, len + 1);
 		nvlist_free(fmristr);
@@ -570,11 +572,13 @@
 	char *cid = NULL;
 	int nslashes = 0;
 	int npairs = 0;
-	int i;
+	int i, hclen;
 
 	if ((hc = topo_mod_strdup(mod, fmri + 5)) == NULL)
 		return (NULL);
 
+	hclen = strlen(hc) + 1;
+
 	/*
 	 * Count equal signs and slashes to determine how many
 	 * hc-pairs will be present in the final FMRI.  There should
@@ -600,7 +604,7 @@
 	 * Do we appear to have a well-formed string version of the FMRI?
 	 */
 	if (nslashes < npairs || npairs == 0) {
-		topo_mod_strfree(mod, hc);
+		topo_mod_free(mod, hc, hclen);
 		return (NULL);
 	}
 
@@ -608,8 +612,8 @@
 
 	find = fromstr;
 
-	if ((pa = topo_mod_alloc(mod, npairs * sizeof (nvlist_t *))) == NULL) {
-		topo_mod_strfree(mod, hc);
+	if ((pa = topo_mod_zalloc(mod, npairs * sizeof (nvlist_t *))) == NULL) {
+		topo_mod_free(mod, hc, hclen);
 		return (NULL);
 	}
 
@@ -623,7 +627,6 @@
 	 * slash from there.
 	 */
 	for (i = 0; i < npairs; i++) {
-		pa[i] = NULL;
 		startn = strchr(find, '/');
 		if (startn == NULL)
 			break;
@@ -669,14 +672,14 @@
 	topo_mod_strfree(mod, cid);
 
 	if (i < npairs) {
-		while (i >= 0)
-			nvlist_free(pa[i + 1]);
+		for (i = 0; i < npairs; i++)
+			nvlist_free(pa[i]);
 		topo_mod_free(mod, pa, npairs * sizeof (nvlist_t *));
-		topo_mod_strfree(mod, hc);
+		topo_mod_free(mod, hc, hclen);
 		return (NULL);
 	}
 
-	topo_mod_strfree(mod, hc);
+	topo_mod_free(mod, hc, hclen);
 
 	return (pa);
 }
--- a/usr/src/lib/fm/topo/libtopo/common/mapfile-vers	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/mapfile-vers	Fri May 25 10:59:41 2007 -0700
@@ -87,6 +87,7 @@
 	topo_mod_strfree;
 	topo_mod_unload;
 	topo_mod_unregister;
+	topo_mod_walk_init;
 	topo_mod_zalloc;
 	topo_node_asru;
 	topo_node_asru_set;
--- a/usr/src/lib/fm/topo/libtopo/common/topo_2xml.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_2xml.c	Fri May 25 10:59:41 2007 -0700
@@ -269,7 +269,14 @@
 static void
 txml_print_topology(topo_hdl_t *thp, FILE *fp, char *scheme, tnode_t *node)
 {
-	begin_element(fp, Topology, Name, thp->th_product, Scheme, scheme,
+	char *name;
+
+	if (thp->th_product != NULL)
+		name = thp->th_product;
+	else
+		name = thp->th_platform;
+
+	begin_element(fp, Topology, Name, name, Scheme, scheme,
 	    NULL);
 	(void) txml_print_range(thp, fp, node, 0);
 	end_element(fp, Topology);
--- a/usr/src/lib/fm/topo/libtopo/common/topo_fmri.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_fmri.c	Fri May 25 10:59:41 2007 -0700
@@ -156,13 +156,14 @@
 	    TOPO_METH_STR2NVL_VERSION, in, &out, err) != 0)
 		return (set_error(thp, *err, err, TOPO_METH_STR2NVL, in));
 
+	nvlist_free(in);
+
 	if (out == NULL ||
 	    topo_hdl_nvdup(thp, out, fmri) != 0)
 		return (set_error(thp, ETOPO_FMRI_NVL, err,
-		    TOPO_METH_STR2NVL, in));
+		    TOPO_METH_STR2NVL, out));
 
 	nvlist_free(out);
-	nvlist_free(in);
 
 	return (0);
 }
--- a/usr/src/lib/fm/topo/libtopo/common/topo_mod.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_mod.c	Fri May 25 10:59:41 2007 -0700
@@ -310,8 +310,10 @@
 	}
 
 	if (pnode != NULL) {
-		if (topo_node_resource(pnode, &pfmri, &err) < 0)
+		if (topo_node_resource(pnode, &pfmri, &err) < 0) {
+			nvlist_free(args);
 			return (set_fmri_err(mod, EMOD_NVL_INVAL));
+		}
 
 		if (nvlist_add_nvlist(args, TOPO_METH_FMRI_ARG_PARENT,
 		    pfmri) != 0) {
@@ -699,6 +701,11 @@
 	char *server = NULL;
 	nvlist_t *auth;
 
+	if ((err = topo_mod_nvalloc(mod, &auth, NV_UNIQUE_NAME)) != 0) {
+		(void) topo_mod_seterrno(mod, EMOD_FMRI_NVL);
+		return (NULL);
+	}
+
 	(void) topo_prop_get_string(pnode, FM_FMRI_AUTHORITY,
 	    FM_FMRI_AUTH_PRODUCT, &prod, &err);
 	(void) topo_prop_get_string(pnode, FM_FMRI_AUTHORITY,
@@ -720,14 +727,12 @@
 	/*
 	 * No luck, return NULL
 	 */
-	if (!prod && !server && !csn)
-		return (NULL);
-
-	if ((err = topo_mod_nvalloc(mod, &auth, NV_UNIQUE_NAME)) != 0) {
-		(void) topo_mod_seterrno(mod, EMOD_FMRI_NVL);
+	if (!prod && !server && !csn) {
+		nvlist_free(auth);
 		return (NULL);
 	}
 
+	err = 0;
 	if (prod != NULL) {
 		err |= nvlist_add_string(auth, FM_FMRI_AUTH_PRODUCT, prod);
 		topo_mod_strfree(mod, prod);
--- a/usr/src/lib/fm/topo/libtopo/common/topo_mod.map	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_mod.map	Fri May 25 10:59:41 2007 -0700
@@ -65,6 +65,8 @@
 	topo_mod_str2nvl = FUNCTION extern;
 	topo_mod_auth = FUNCTION extern;
 
+	topo_mod_walk_init = FUNCTION extern;
+
 	topo_method_register = FUNCTION extern;
 	topo_method_unregister = FUNCTION extern;
 	topo_method_unregister_all = FUNCTION extern;
--- a/usr/src/lib/fm/topo/libtopo/common/topo_module.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_module.c	Fri May 25 10:59:41 2007 -0700
@@ -200,8 +200,12 @@
 static topo_mod_t *
 set_create_error(topo_hdl_t *thp, topo_mod_t *mod, const char *path, int err)
 {
-	topo_dprintf(thp, TOPO_DBG_ERR, "unable to load module %s: %s\n",
-	    path, topo_strerror(err));
+	if (path != NULL)
+		topo_dprintf(thp, TOPO_DBG_ERR, "unable to load module %s: "
+		    "%s\n", path, topo_strerror(err));
+	else
+		topo_dprintf(thp, TOPO_DBG_ERR, "unable to load module: "
+		    "%s\n", topo_strerror(err));
 
 	if (mod != NULL)
 		topo_mod_destroy(mod);
--- a/usr/src/lib/fm/topo/libtopo/common/topo_node.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_node.c	Fri May 25 10:59:41 2007 -0700
@@ -131,6 +131,9 @@
 	if (node == NULL)
 		return;
 
+	topo_dprintf(mod->tm_hdl, TOPO_DBG_MODSVC, "destroying node %s=%d\n",
+	    topo_node_name(node), topo_node_instance(node));
+
 	assert(node->tn_refs == 0);
 
 	/*
@@ -344,16 +347,12 @@
 		return;
 	}
 
+	for (i = 0; i < nhp->th_arrlen; i++)
+		assert(nhp->th_nodearr[i] == NULL);
+
 	topo_list_delete(&pnode->tn_children, nhp);
 	topo_node_unlock(pnode);
 
-	/*
-	 * Should be an empty node range
-	 */
-	for (i = 0; i < nhp->th_arrlen; i++) {
-		topo_node_unbind(nhp->th_nodearr[i]);
-	}
-
 	mod = nhp->th_enum;
 	if (nhp->th_name != NULL)
 		topo_mod_strfree(mod, nhp->th_name);
@@ -480,7 +479,8 @@
 		return (node_bind_seterror(mod, pnode, node, err));
 
 	topo_dprintf(mod->tm_hdl, TOPO_DBG_MODSVC,
-	    "node bound %s=%d\n", node->tn_name, node->tn_instance);
+	    "node bound %s=%d/%s=%d\n", topo_node_name(pnode),
+	    topo_node_instance(pnode), node->tn_name, node->tn_instance);
 
 	node->tn_state |= TOPO_NODE_BOUND;
 
@@ -516,6 +516,12 @@
 	node->tn_state &= ~TOPO_NODE_BOUND;
 	topo_node_unlock(node);
 
+	topo_dprintf(node->tn_hdl, TOPO_DBG_MODSVC,
+	    "node unbound %s=%d/%s=%d refs = %d\n",
+	    topo_node_name(node->tn_parent),
+	    topo_node_instance(node->tn_parent), node->tn_name,
+	    node->tn_instance, node->tn_refs);
+
 	topo_node_rele(node);
 }
 
@@ -573,7 +579,7 @@
 		wp->tw_node = child;
 	} else {
 		topo_node_unlock(node);
-		topo_node_hold(child);
+		topo_node_hold(node); /* rele at walk end */
 		wp->tw_node = node;
 	}
 
--- a/usr/src/lib/fm/topo/libtopo/common/topo_nvl.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_nvl.c	Fri May 25 10:59:41 2007 -0700
@@ -2,9 +2,8 @@
  * CDDL HEADER START
  *
  * The contents of this file are subject to the terms of the
- * Common Development and Distribution License, Version 1.0 only
- * (the "License").  You may not use this file except in compliance
- * with the License.
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
  *
  * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  * or http://www.opensolaris.org/os/licensing.
@@ -20,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -47,24 +46,36 @@
 int
 topo_mod_nvalloc(topo_mod_t *mod, nvlist_t **nvlp, uint_t nvflag)
 {
-	return (nvlist_xalloc(nvlp, nvflag, &mod->tm_alloc->ta_nva));
+	if (nvlist_xalloc(nvlp, nvflag, &mod->tm_alloc->ta_nva) != 0)
+		return (-1);
+
+	return (0);
 }
 
 int
 topo_mod_nvdup(topo_mod_t *mod, nvlist_t *nvl, nvlist_t **nvlp)
 {
-	return (nvlist_xdup(nvl, nvlp, &mod->tm_alloc->ta_nva));
+	if (nvlist_xdup(nvl, nvlp, &mod->tm_alloc->ta_nva) != 0)
+		return (-1);
+
+	return (0);
 }
 
 int
 topo_hdl_nvalloc(topo_hdl_t *thp, nvlist_t **nvlp, uint_t nvflag)
 {
 
-	return (nvlist_xalloc(nvlp, nvflag, &thp->th_alloc->ta_nva));
+	if (nvlist_xalloc(nvlp, nvflag, &thp->th_alloc->ta_nva) != 0)
+		return (-1);
+
+	return (0);
 }
 
 int
 topo_hdl_nvdup(topo_hdl_t *thp, nvlist_t *nvl, nvlist_t **nvlp)
 {
-	return (nvlist_xdup(nvl, nvlp, &thp->th_alloc->ta_nva));
+	if (nvlist_xdup(nvl, nvlp, &thp->th_alloc->ta_nva) != 0)
+		return (-1);
+
+	return (0);
 }
--- a/usr/src/lib/fm/topo/libtopo/common/topo_prop.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_prop.c	Fri May 25 10:59:41 2007 -0700
@@ -498,8 +498,11 @@
 	/*
 	 * Replace existing prop value with new one
 	 */
-	if ((pg = pgroup_get(node, pgname)) == NULL)
+	if ((pg = pgroup_get(node, pgname)) == NULL) {
+		topo_node_unlock(node);
+		*err = ETOPO_PROP_NOENT;
 		return (NULL);
+	}
 
 	if ((pv = propval_get(pg, pname)) != NULL) {
 		if (pv->tp_type != type)
@@ -517,6 +520,8 @@
 		if ((pv = topo_hdl_zalloc(thp, sizeof (topo_propval_t)))
 		    == NULL)
 			return (set_seterror(node, pvl, err, ETOPO_NOMEM));
+
+		pv->tp_hdl = thp;
 		pvl->tp_pval = pv;
 
 		if ((pv->tp_name = topo_hdl_strdup(thp, pname))
@@ -524,7 +529,6 @@
 			return (set_seterror(node, pvl, err, ETOPO_NOMEM));
 		pv->tp_flag = flag;
 		pv->tp_type = type;
-		pv->tp_hdl = thp;
 		topo_prop_hold(pv);
 		topo_list_append(&pg->tpg_pvals, pvl);
 	}
@@ -1007,7 +1011,7 @@
 
 			pip = pg->tpg_info;
 			if ((info->tpi_name =
-				topo_hdl_strdup(thp, pip->tpi_name)) == NULL) {
+			    topo_hdl_strdup(thp, pip->tpi_name)) == NULL) {
 				*err = ETOPO_PROP_NOMEM;
 				topo_hdl_free(thp, info,
 				    sizeof (topo_pgroup_info_t));
--- a/usr/src/lib/fm/topo/libtopo/common/topo_snap.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_snap.c	Fri May 25 10:59:41 2007 -0700
@@ -196,7 +196,8 @@
 		thp->th_product = topo_hdl_strdup(thp, thp->th_platform);
 	}
 
-	if (thp->th_rootdir == NULL)
+	if (thp->th_rootdir == NULL || thp->th_platform == NULL ||
+	    thp->th_machine == NULL)
 		return (set_open_errno(thp, errp, ETOPO_NOMEM));
 
 	dbflags	 = getenv("TOPO_DEBUG");
--- a/usr/src/lib/fm/topo/libtopo/common/topo_tree.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/libtopo/common/topo_tree.c	Fri May 25 10:59:41 2007 -0700
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -154,6 +154,7 @@
 static int
 topo_tree_enum(topo_hdl_t *thp, ttree_t *tp)
 {
+	int rv = 0;
 	char *pp;
 
 	/*
@@ -184,13 +185,11 @@
 			if (topo_file_load(tp->tt_root->tn_enum, tp->tt_root,
 			    thp->th_machine, tp->tt_scheme) < 0) {
 
-				if (topo_file_load(tp->tt_root->tn_enum,
-				    tp->tt_root, NULL, tp->tt_scheme) < 0) {
+				if ((rv = topo_file_load(tp->tt_root->tn_enum,
+				    tp->tt_root, NULL, tp->tt_scheme)) < 0) {
 					topo_dprintf(thp, TOPO_DBG_ERR, "no "
 					    "topology map found for the %s "
 					    "FMRI set\n", tp->tt_scheme);
-					return (topo_hdl_seterrno(thp,
-					    ETOPO_ENUM_NOMAP));
 				}
 			}
 		}
@@ -210,6 +209,8 @@
 		thp->th_pi = DI_PROM_HANDLE_NIL;
 	}
 
+	if (rv != 0)
+		return (topo_hdl_seterrno(thp, ETOPO_ENUM_NOMAP));
 
 	return (0);
 }
--- a/usr/src/lib/fm/topo/modules/common/hostbridge/hostbridge.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/common/hostbridge/hostbridge.c	Fri May 25 10:59:41 2007 -0700
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -140,6 +140,7 @@
 hb_enum(topo_mod_t *mp, tnode_t *pn, const char *name, topo_instance_t imin,
     topo_instance_t imax, void *notused, void *data)
 {
+	int rv;
 	topo_mod_t *pcimod;
 
 	if (strcmp(name, HOSTBRIDGE) != 0) {
@@ -161,7 +162,6 @@
 	 * did_t structures we can use to enumerate the singled out hostbridge.
 	 */
 	if (imin != imax) {
-		int rv;
 
 		if (did_hash_init(mp) < 0) {
 			topo_mod_dprintf(mp,
@@ -170,14 +170,13 @@
 			topo_mod_unload(pcimod);
 			return (-1);
 		}
-		if ((rv = platform_hb_enum(mp, pn, name, imin, imax)) < 0)
-			topo_mod_seterrno(mp, EMOD_PARTIAL_ENUM);
+		rv = platform_hb_enum(mp, pn, name, imin, imax);
 		did_hash_fini(mp);
-		return (rv);
 	} else {
-		return (specific_hb_enum(mp, pn, name, imin, imax,
-		    data));
+		rv = specific_hb_enum(mp, pn, name, imin, imax, data);
 	}
+
+	return (rv);
 }
 
 /*ARGSUSED*/
@@ -185,12 +184,6 @@
 hb_release(topo_mod_t *mp, tnode_t *node)
 {
 	topo_method_unregister_all(mp, node);
-
-	/*
-	 * node private data (did_t) for this node is destroyed in
-	 * did_hash_destroy()
-	 */
-
 }
 
 static tnode_t *
--- a/usr/src/lib/fm/topo/modules/common/pcibus/did_props.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/common/pcibus/did_props.c	Fri May 25 10:59:41 2007 -0700
@@ -323,6 +323,9 @@
 	char *lastslash;
 	char *newpath;
 	char *comma;
+	int plen;
+
+	plen = strlen(path) + 1;
 
 	/*
 	 * We only care about the last component of the dev path. If
@@ -339,10 +342,13 @@
 	assert(comma != NULL);
 
 	*comma = '\0';
-	if ((newpath = topo_mod_strdup(mp, path)) == NULL)
-		return (path);
+	if ((newpath = topo_mod_strdup(mp, path)) == NULL) {
+		topo_mod_free(mp, path, plen);
+		return (NULL);
+	}
+
 	*comma = ',';
-	topo_mod_strfree(mp, path);
+	topo_mod_free(mp, path, plen);
 	return (newpath);
 }
 
@@ -421,7 +427,7 @@
 	if (topo_node_resource(tn, &fmri, &err) < 0 ||
 	    fmri == NULL) {
 		topo_mod_dprintf(mp, "FRU_fmri_set error: %s\n",
-			topo_strerror(topo_mod_errno(mp)));
+		    topo_strerror(topo_mod_errno(mp)));
 		return (topo_mod_seterrno(mp, err));
 	}
 	e = topo_node_fru_set(tn, fmri, 0, &err);
@@ -462,7 +468,7 @@
 		e = FRU_fmri_set(mp, tn);
 		return (e);
 	} else if (strcmp(nm, PCI_DEVICE) == 0 ||
-		strcmp(nm, PCIEX_DEVICE) == 0) {
+	    strcmp(nm, PCIEX_DEVICE) == 0) {
 		nvlist_t *in, *out;
 
 		mp = did_mod(pd);
@@ -473,8 +479,8 @@
 			return (topo_mod_seterrno(mp, EMOD_NOMEM));
 		}
 		if (topo_method_invoke(tn,
-			TOPO_METH_FRU_COMPUTE, TOPO_METH_FRU_COMPUTE_VERSION,
-			in, &out, &err) != 0) {
+		    TOPO_METH_FRU_COMPUTE, TOPO_METH_FRU_COMPUTE_VERSION,
+		    in, &out, &err) != 0) {
 			nvlist_free(in);
 			return (topo_mod_seterrno(mp, err));
 		}
@@ -664,7 +670,10 @@
 	if (di_bytes_get(did_mod(pd), did_dinode(pd), dpnm, &sz, &typbuf) < 0)
 		return (0);
 	mp = did_mod(pd);
-	tmpbuf = topo_mod_alloc(mp, sz + 1);
+
+	if ((tmpbuf = topo_mod_alloc(mp, sz + 1)) == NULL)
+		return (topo_mod_seterrno(mp, EMOD_NOMEM));
+
 	bcopy(typbuf, tmpbuf, sz);
 	tmpbuf[sz] = 0;
 	e = topo_prop_set_string(tn,
--- a/usr/src/lib/fm/topo/modules/common/pcibus/pcibus.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/common/pcibus/pcibus.c	Fri May 25 10:59:41 2007 -0700
@@ -149,8 +149,10 @@
 
 	if ((dev32 = pcidev_declare(mod, bus, di, 32)) == NULL)
 		return (-1);
-	if (pcifn_declare(mod, dev32, di, 0) == NULL)
+	if (pcifn_declare(mod, dev32, di, 0) == NULL) {
+		topo_node_unbind(dev32);
 		return (-1);
+	}
 	return (0);
 }
 
@@ -174,11 +176,12 @@
 	 * We may find pci-express buses or plain-pci buses beneath a function
 	 */
 	if (child_range_add(mod, ntn, PCIEX_BUS, 0, MAX_HB_BUSES) < 0) {
-		topo_node_range_destroy(ntn, PCIEX_BUS);
+		topo_node_unbind(ntn);
 		return (NULL);
 	}
 	if (child_range_add(mod, ntn, PCI_BUS, 0, MAX_HB_BUSES) < 0) {
-		topo_node_range_destroy(ntn, PCI_BUS);
+		topo_node_range_destroy(ntn, PCIEX_BUS);
+		topo_node_unbind(ntn);
 		return (NULL);
 	}
 	return (ntn);
@@ -207,7 +210,7 @@
 	 */
 	if (child_range_add(mod,
 	    ntn, PCIEX_FUNCTION, 0, MAX_PCIDEV_FNS) < 0) {
-		topo_node_range_destroy(ntn, PCIEX_FUNCTION);
+		topo_node_unbind(ntn);
 		return (NULL);
 	}
 	return (ntn);
@@ -225,7 +228,6 @@
 	if ((ntn = pci_tnode_create(mod, parent, PCIEX_BUS, i, dn)) == NULL)
 		return (NULL);
 	if (did_props_set(ntn, pd, Bus_common_props, Bus_propcnt) < 0) {
-		topo_node_range_destroy(ntn, PCI_DEVICE);
 		topo_node_unbind(ntn);
 		return (NULL);
 	}
@@ -234,7 +236,7 @@
 	 */
 	if (child_range_add(mod,
 	    ntn, PCIEX_DEVICE, 0, MAX_PCIBUS_DEVS) < 0) {
-		topo_node_range_destroy(ntn, PCIEX_DEVICE);
+		topo_node_unbind(ntn);
 		return (NULL);
 	}
 	return (ntn);
@@ -301,7 +303,6 @@
 	 * We can expect to find pci functions beneath the device
 	 */
 	if (child_range_add(mod, ntn, PCI_FUNCTION, 0, MAX_PCIDEV_FNS) < 0) {
-		topo_node_range_destroy(ntn, PCI_FUNCTION);
 		topo_node_unbind(ntn);
 		return (NULL);
 	}
@@ -372,11 +373,11 @@
 	return (err);
 }
 
-static int
+static void
 declare_dev_and_fn(topo_mod_t *mod, tnode_t *bus, tnode_t **dev, di_node_t din,
     int board, int bridge, int rc, int devno, int fnno, int depth)
 {
-	int err = 0;
+	int dcnt = 0;
 	tnode_t *fn;
 	uint_t class, subclass;
 
@@ -386,16 +387,28 @@
 		else
 			*dev = pcidev_declare(mod, bus, din, devno);
 		if (*dev == NULL)
-			return (-1);
+			return;
+		++dcnt;
 	}
 	if (rc >= 0)
 		fn = pciexfn_declare(mod, *dev, din, fnno);
 	else
 		fn = pcifn_declare(mod, *dev, din, fnno);
-	if (fn == NULL)
-		return (-1);
-	if (pci_classcode_get(mod, din, &class, &subclass) < 0)
-		return (-1);
+
+	if (fn == NULL) {
+		if (dcnt) {
+			topo_node_unbind(*dev);
+			*dev = NULL;
+		}
+		return;
+	}
+
+	if (pci_classcode_get(mod, din, &class, &subclass) < 0) {
+		topo_node_unbind(fn);
+		if (dcnt)
+			topo_node_unbind(*dev);
+		return;
+	}
 
 	/*
 	 * This function may be a bridge.  If not, check for a possible
@@ -403,12 +416,10 @@
 	 * devices.
 	 */
 	if (class == PCI_CLASS_BRIDGE && subclass == PCI_BRIDGE_PCI)
-		err = pci_bridge_declare(mod, fn, din, board, bridge, rc,
+		(void) pci_bridge_declare(mod, fn, din, board, bridge, rc,
 		    depth);
 	else if (class == PCI_CLASS_MASS)
 		(void) topo_mod_enummap(mod, fn, "storage", FM_FMRI_SCHEME_HC);
-
-	return (err);
 }
 
 int
@@ -459,15 +470,18 @@
 			if (pps[d][f] == NULL)
 				continue;
 			din = did_dinode(pps[d][f]);
+
 			/*
-			 * Ignore error and try to enumerate as much as
-			 * possible.  If we ever need to check for an
-			 * error all declared buses, devices and functions
-			 * need to be cleaned up
+			 * Try to enumerate as many devices and functions as
+			 * possible.  If we fail to declare a device, break
+			 * out of the function loop.
 			 */
-			(void) declare_dev_and_fn(mod, bn,
+			declare_dev_and_fn(mod, bn,
 			    &dn, din, board, bridge, rc, d, f, depth);
 			did_rele(pps[d][f]);
+
+			if (dn == NULL)
+				break;
 		}
 		dn = NULL;
 	}
--- a/usr/src/lib/fm/topo/modules/i86pc/hostbridge/hb_i86pc.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/i86pc/hostbridge/hb_i86pc.c	Fri May 25 10:59:41 2007 -0700
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -45,8 +45,13 @@
 		return (-1);
 	if ((hb = pcihostbridge_declare(mod, ptn, bn, hbi)) == NULL)
 		return (-1);
-	return (topo_mod_enumerate(mod,
-	    hb, PCI_BUS, PCI_BUS, 0, MAX_HB_BUSES, (void *)hbdid));
+	if (topo_mod_enumerate(mod,
+	    hb, PCI_BUS, PCI_BUS, 0, MAX_HB_BUSES, (void *)hbdid) < 0) {
+		topo_node_unbind(hb);
+		return (-1);
+	}
+
+	return (0);
 }
 
 static int
@@ -62,8 +67,14 @@
 		return (-1);
 	if ((rc = pciexrc_declare(mod, hb, bn, hbi)) == NULL)
 		return (-1);
-	return (topo_mod_enumerate(mod,
-	    rc, PCI_BUS, PCIEX_BUS, 0, MAX_HB_BUSES, (void *)hbdid));
+	if (topo_mod_enumerate(mod,
+	    rc, PCI_BUS, PCIEX_BUS, 0, MAX_HB_BUSES, (void *)hbdid) < 0) {
+		topo_node_unbind(hb);
+		topo_node_unbind(rc);
+		return (-1);
+	}
+
+	return (0);
 }
 
 
@@ -84,11 +95,12 @@
 
 	pnode = di_drv_first_node(PCI, devtree);
 	while (pnode != DI_NODE_NIL) {
-		if (hb_process(mod, ptn, hbcnt++, pnode)
-		    < 0) {
-			topo_node_range_destroy(ptn, HOSTBRIDGE);
+		if (hb_process(mod, ptn, hbcnt, pnode) < 0) {
+			if (hbcnt == 0)
+				topo_node_range_destroy(ptn, HOSTBRIDGE);
 			return (topo_mod_seterrno(mod, EMOD_PARTIAL_ENUM));
 		}
+		hbcnt++;
 		pnode = di_drv_next_node(pnode);
 	}
 
@@ -99,20 +111,24 @@
 			if (di_driver_name(cnode) == NULL)
 				continue;
 			if (strcmp(di_driver_name(cnode), PCI_PCI) == 0) {
-				if (hb_process(mod, ptn, hbcnt++, cnode) < 0) {
-					topo_node_range_destroy(ptn,
-					    HOSTBRIDGE);
+				if (hb_process(mod, ptn, hbcnt, cnode) < 0) {
+					if (hbcnt == 0)
+						topo_node_range_destroy(ptn,
+						    HOSTBRIDGE);
 					return (topo_mod_seterrno(mod,
 					    EMOD_PARTIAL_ENUM));
 				}
+				hbcnt++;
 			}
 			if (strcmp(di_driver_name(cnode), PCIE_PCI) == 0) {
-				if (rc_process(mod, ptn, hbcnt++, cnode) < 0) {
-					topo_node_range_destroy(ptn,
-					    HOSTBRIDGE);
+				if (rc_process(mod, ptn, hbcnt, cnode) < 0) {
+					if (hbcnt == 0)
+						topo_node_range_destroy(ptn,
+						    HOSTBRIDGE);
 					return (topo_mod_seterrno(mod,
 					    EMOD_PARTIAL_ENUM));
 				}
+				hbcnt++;
 			}
 		}
 		pnode = di_drv_next_node(pnode);
--- a/usr/src/lib/fm/topo/modules/i86pc/sata/sata.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/i86pc/sata/sata.c	Fri May 25 10:59:41 2007 -0700
@@ -245,13 +245,18 @@
 
 		sata_info_to_fru(cfgap->ap_info, &model, &model_len, &manuf,
 		    &manuf_len, &serial, &serial_len, &firm, &firm_len, mod);
-		if ((s = strchr(model, ' ')) != NULL)
-			*s = '-';
-		len = manuf_len + model_len + 1;
-		if ((mm = topo_mod_alloc(mod, len)) != NULL)
-			(void) snprintf(mm, len, "%s-%s", manuf, model);
-		else
-			mm = model;
+
+		if (model != NULL && manuf != NULL) {
+			if ((s = strchr(model, ' ')) != NULL)
+				*s = '-';
+			len = manuf_len + model_len + 1;
+			if ((mm = topo_mod_alloc(mod, len)) != NULL) {
+				(void) snprintf(mm, len, "%s-%s", manuf, model);
+			} else {
+				len = 0;
+				mm = model;
+			}
+		}
 	}
 
 	auth = topo_mod_auth(mod, pnode);
@@ -443,7 +448,9 @@
 
 		assert(nlist == 1);
 		*apbuflen = PATH_MAX;
-		*ap = topo_mod_alloc(mod, *apbuflen);
+		if ((*ap = topo_mod_alloc(mod, *apbuflen)) == NULL)
+			return (-1);
+
 		strncpy(*ap, (*list_array)[0].ap_log_id, PATH_MAX);
 		/*
 		 * The logical id is:
@@ -503,7 +510,10 @@
 	dentlen = pathconf(physpath, _PC_NAME_MAX);
 	dentlen = ((dentlen <= 0) ? MAXNAMELEN : dentlen) +
 	    sizeof (struct dirent);
-	dent = topo_mod_alloc(mod, dentlen);
+	if ((dent = topo_mod_alloc(mod, dentlen)) == NULL) {
+		closedir(dp);
+		return (-1);
+	}
 
 	errno = 0;
 	while (!found && readdir_r(dp, dent, &dep) == 0 && dep != NULL) {
@@ -700,7 +710,7 @@
     topo_mod_t *mod)
 {
 	int infolen;
-	char *sata_info = tp_strdup(mod, info, &infolen);
+	char *sata_info;
 	char *modlp, *revp, *snp, *manup;
 
 	/*
@@ -710,10 +720,12 @@
 	 * Mod: <model> FRev: <revision> SN: <serialno>
 	 */
 
+	if ((sata_info = tp_strdup(mod, info, &infolen)) == NULL)
+		return;
+
 	if ((modlp = strstr(sata_info, "Mod: ")) == NULL ||
 	    (revp = strstr(sata_info, " FRev: ")) == NULL ||
 	    (snp = strstr(sata_info, " SN: ")) == NULL) {
-		*model = *manuf = *serial = *firm = NULL;
 		topo_mod_free(mod, sata_info, infolen);
 		return;
 	}
@@ -778,8 +790,8 @@
 		    ldev, err);
 	}
 
-	sata_info_to_fru(cfgap->ap_info, &model, &model_len, &manuf, &manuf_len,
-	    &serial, &serial_len, &firm, &firm_len, mod);
+	sata_info_to_fru(cfgap->ap_info, &model, &model_len, &manuf,
+	    &manuf_len, &serial, &serial_len, &firm, &firm_len, mod);
 	if (model) {
 		(void) topo_prop_set_string(cnode, TOPO_STORAGE_PGROUP,
 		    TOPO_STORAGE_MODEL, TOPO_PROP_IMMUTABLE, model, err);
@@ -943,10 +955,8 @@
 		}
 	}
 
-	if (nerrs) {
-		topo_node_range_destroy(pnode, name);
+	if (nerrs)
 		return (-1);
-	}
 
 	return (0);
 }
@@ -1048,10 +1058,9 @@
 
 	topo_mod_free(mod, dpath, dpathlen);
 
-	if (nerr != 0) {
-		topo_node_range_destroy(pnode, name);
+	if (nerr != 0)
 		return (-1);
-	} else
+	else
 		return (0);
 }
 
--- a/usr/src/lib/fm/topo/modules/sun4/hostbridge/hb_sun4.c	Fri May 25 09:30:08 2007 -0700
+++ b/usr/src/lib/fm/topo/modules/sun4/hostbridge/hb_sun4.c	Fri May 25 10:59:41 2007 -0700
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -156,6 +156,9 @@
 		return (NULL);
 	if (topo_mod_enumerate(mod, hb, PCI_BUS, PCI_BUS, bi, bi, hbdid) == 0)
 		return (hb);
+
+	topo_node_unbind(hb);
+
 	return (NULL);
 }
 
@@ -169,6 +172,9 @@
 	if (topo_mod_enumerate(mod,
 	    rc, PCI_BUS, PCIEX_BUS, 0, MAX_HB_BUSES, NULL) == 0)
 		return (rc);
+
+	topo_node_unbind(rc);
+
 	return (NULL);
 }
 
@@ -197,6 +203,7 @@
 declare_exbuses(topo_mod_t *mod, busorrc_t *list, tnode_t *ptn, int nhb,
     int nrc)
 {
+	int err = 0;
 	tnode_t **rcs;
 	tnode_t **hb;
 	busorrc_t *p;
@@ -211,32 +218,42 @@
 	/*
 	 * Allocate an array to point at the root complex tnode_t pointers.
 	 */
-	if ((rcs = topo_mod_zalloc(mod, nrc * sizeof (tnode_t *))) == NULL)
+	if ((rcs = topo_mod_zalloc(mod, nrc * sizeof (tnode_t *))) == NULL) {
+		topo_mod_free(mod, hb, nhb * sizeof (tnode_t *));
 		return (topo_mod_seterrno(mod, EMOD_NOMEM));
+	}
 
 	br = rc = 0;
 	for (p = list; p != NULL; p = p->br_nextbus) {
 		topo_mod_dprintf(mod,
 		    "declaring (%x,%x)\n", p->br_ba_bc, p->br_ba_ac);
 
-		if (did_create(mod, p->br_din, 0, br, rc, rc) == NULL)
-			return (-1);
+		if (did_create(mod, p->br_din, 0, br, rc, rc) == NULL) {
+			err = -1;
+			break;
+		}
 
 		if (hb[br] == NULL) {
 			hb[br] = pciexhostbridge_declare(mod, ptn, p->br_din,
 			    br);
-			if (hb[br] == NULL)
-				return (-1);
+			if (hb[br] == NULL) {
+				err = -1;
+				break;
+			}
 		}
 		if (rcs[rc] == NULL) {
 			rcs[rc] = rc_process(mod, hb[br], rc, p->br_din);
-			if (rcs[rc] == NULL)
-				return (-1);
+			if (rcs[rc] == NULL) {
+				err = -1;
+				break;
+			}
 		} else {
 			if (topo_mod_enumerate(mod,
 			    rcs[rc], PCI_BUS, PCIEX_BUS, 0, MAX_HB_BUSES,
-			    NULL) < 0)
-				return (-1);
+			    NULL) < 0) {
+				err = -1;
+				break;
+			}
 		}
 		rc++;
 		if (rc == nrc) {
@@ -246,9 +263,20 @@
 				br = 0;
 		}
 	}
+
+	if (err != 0) {
+		int i;
+
+		for (i = 0; i < nhb; ++i)
+			topo_node_unbind(hb[br]);
+		for (i = 0; i < nrc; ++i)
+			topo_node_unbind(rcs[rc]);
+	}
+
 	topo_mod_free(mod, rcs, nrc * sizeof (tnode_t *));
 	topo_mod_free(mod, hb, nhb * sizeof (tnode_t *));
-	return (0);
+
+	return (err);
 }
 
 /*
@@ -267,6 +295,7 @@
 int
 declare_buses(topo_mod_t *mod, busorrc_t *list, tnode_t *ptn, int nhb)
 {
+	int err = 0;
 	busorrc_t *p;
 	tnode_t **hb;
 	did_t *link;
@@ -284,18 +313,23 @@
 		    "declaring (%x,%x)\n", p->br_ba_bc, p->br_ba_ac);
 
 		if ((link =
-		    did_create(mod, p->br_din, 0, br, NO_RC, bus)) == NULL)
-			return (-1);
+		    did_create(mod, p->br_din, 0, br, NO_RC, bus)) == NULL) {
+			err = -1;
+			break;
+		}
 
 		if (hb[br] == NULL) {
 			hb[br] = hb_process(mod, ptn, br, bus, p->br_din, link);
-			if (hb[br] == NULL)
-				return (-1);
+			if (hb[br] == NULL) {
+				err = -1;
+				break;
+			}
 		} else {
 			did_link_set(mod, hb[br], link);
 			if (topo_mod_enumerate(mod,
 			    hb[br], PCI_BUS, PCI_BUS, bus, bus, link) < 0) {
-				return (-1);
+				err = -1;
+				break;
 			}
 		}
 		br++;
@@ -304,6 +338,14 @@
 			bus++;
 		}
 	}
+
+	if (err != 0) {
+		int i;
+
+		for (i = 0; i < nhb; ++i)
+			topo_node_unbind(hb[br]);
+	}
+
 	topo_mod_free(mod, hb, nhb * sizeof (tnode_t *));
-	return (0);
+	return (err);
 }