--- a/usr/src/lib/libshare/common/libshare.c	Tue May 29 15:28:30 2007 -0700
+++ b/usr/src/lib/libshare/common/libshare.c	Tue May 29 15:54:57 2007 -0700
@@ -974,11 +974,23 @@
 	if (impl_handle != NULL && impl_handle->tree != NULL) {
 		if (groupname != NULL) {
 			group = strdup(groupname);
-			subgroup = strchr(group, '/');
-			if (subgroup != NULL)
-				*subgroup++ = '\0';
+			if (group != NULL) {
+				subgroup = strchr(group, '/');
+				if (subgroup != NULL)
+					*subgroup++ = '\0';
+			}
 		}
-		node = find_group_by_name(impl_handle->tree, (xmlChar *)group);
+		/*
+		 * We want to find the, possibly, named group. If
+		 * group is not NULL, then lookup the name. If it is
+		 * NULL, we only do the find if groupname is also
+		 * NULL. This allows lookup of the "first" group in
+		 * the internal list.
+		 */
+		if (group != NULL || groupname == NULL)
+			node = find_group_by_name(impl_handle->tree,
+			    (xmlChar *)group);
+
 		/* if a subgroup, find it before returning */
 		if (subgroup != NULL && node != NULL)
 			node = find_group_by_name(node, (xmlChar *)subgroup);

--- a/usr/src/lib/libshare/common/libshare_zfs.c	Tue May 29 15:28:30 2007 -0700
+++ b/usr/src/lib/libshare/common/libshare_zfs.c	Tue May 29 15:54:57 2007 -0700
@@ -28,6 +28,7 @@
 
 #include <libzfs.h>
 #include <string.h>
+#include <strings.h>
 #include <libshare.h>
 #include "libshare_impl.h"
 #include <libintl.h>
@@ -48,6 +49,7 @@
 	zfs_handle_t	**cb_handles;
 	size_t		cb_alloc;
 	size_t		cb_used;
+	uint_t		cb_types;
 } get_all_cbdata_t;
 
 /*
@@ -80,17 +82,24 @@
 sa_zfs_fini(sa_handle_impl_t impl_handle)
 {
 	if (impl_handle->zfs_libhandle != NULL) {
-		libzfs_fini(impl_handle->zfs_libhandle);
-		impl_handle->zfs_libhandle = NULL;
 		if (impl_handle->zfs_list != NULL) {
+			zfs_handle_t **zhp = impl_handle->zfs_list;
+			size_t i;
+
 			/*
-			 * contents of zfs_list were already freed by
-			 * the call to libzfs_fini().
+			 * Contents of zfs_list need to be freed so we
+			 * don't lose ZFS handles.
 			 */
+			for (i = 0; i < impl_handle->zfs_list_count; i++) {
+				zfs_close(zhp[i]);
+			}
 			free(impl_handle->zfs_list);
 			impl_handle->zfs_list = NULL;
 			impl_handle->zfs_list_count = 0;
 		}
+
+		libzfs_fini(impl_handle->zfs_libhandle);
+		impl_handle->zfs_libhandle = NULL;
 	}
 }
 
@@ -100,17 +109,32 @@
  * an interator function called while iterating through the ZFS
  * root. It accumulates into an array of file system handles that can
  * be used to derive info about those file systems.
+ *
+ * Note that as this function is called, we close all zhp handles that
+ * are not going to be places into the cp_handles list. We don't want
+ * to close the ones we are keeping, but all others would be leaked if
+ * not closed here.
  */
 
 static int
 get_one_filesystem(zfs_handle_t *zhp, void *data)
 {
 	get_all_cbdata_t *cbp = data;
+	zfs_type_t type = zfs_get_type(zhp);
 
 	/*
-	 * Skip any zvols
+	 * Interate over any nested datasets.
 	 */
-	if (zfs_get_type(zhp) != ZFS_TYPE_FILESYSTEM) {
+	if (type == ZFS_TYPE_FILESYSTEM &&
+	    zfs_iter_filesystems(zhp, get_one_filesystem, data) != 0) {
+		zfs_close(zhp);
+		return (1);
+	}
+
+	/*
+	 * Skip any datasets whose type does not match.
+	 */
+	if ((type & cbp->cb_types) == 0) {
 		zfs_close(zhp);
 		return (0);
 	}
@@ -123,13 +147,15 @@
 		else
 			cbp->cb_alloc *= 2;
 
-		handles = calloc(1, cbp->cb_alloc * sizeof (void *));
+		handles = (zfs_handle_t **)calloc(1,
+		    cbp->cb_alloc * sizeof (void *));
+
 		if (handles == NULL) {
+			zfs_close(zhp);
 			return (0);
 		}
-
 		if (cbp->cb_handles) {
-			(void) memcpy(handles, cbp->cb_handles,
+			bcopy(cbp->cb_handles, handles,
 			    cbp->cb_used * sizeof (void *));
 			free(cbp->cb_handles);
 		}
@@ -139,7 +165,7 @@
 
 	cbp->cb_handles[cbp->cb_used++] = zhp;
 
-	return (zfs_iter_filesystems(zhp, get_one_filesystem, data));
+	return (0);
 }
 
 /*
@@ -156,6 +182,7 @@
 			zfs_handle_t ***fslist, size_t *count)
 {
 	get_all_cbdata_t cb = { 0 };
+	cb.cb_types = ZFS_TYPE_FILESYSTEM;
 
 	if (impl_handle->zfs_list != NULL) {
 		*fslist = impl_handle->zfs_list;
@@ -512,11 +539,11 @@
 				 * put it on the share.
 				 */
 				options = strdup(shareopts);
-				if (options != NULL)
+				if (options != NULL) {
 					err = sa_parse_legacy_options(share,
 					    options, "nfs");
-				if (options != NULL)
 					free(options);
+				}
 			}
 			/* unmark the share's changed state */
 			set_node_attr(share, "changed", NULL);
@@ -644,14 +671,14 @@
 						    zfsgroup, dataset);
 						if (group == NULL) {
 							static int err = 0;
-							/*
-							 * there is a problem,
-							 * but we can't do
-							 * anything about it
-							 * at this point so we
-							 * issue a warning an
-							 * move on.
-							 */
+						/*
+						 * there is a problem,
+						 * but we can't do
+						 * anything about it
+						 * at this point so we
+						 * issue a warning an
+						 * move on.
+						 */
 							zfs_grp_error(err);
 							err = 1;
 							continue;

--- a/usr/src/lib/libshare/common/scfutil.c	Tue May 29 15:28:30 2007 -0700
+++ b/usr/src/lib/libshare/common/scfutil.c	Tue May 29 15:54:57 2007 -0700
@@ -72,8 +72,10 @@
 		unbind = 1;
 		scf_scope_destroy(handle->scope);
 	    }
+	    if (handle->instance != NULL)
+		scf_instance_destroy(handle->instance);
 	    if (handle->service != NULL)
-		    scf_service_destroy(handle->service);
+		scf_service_destroy(handle->service);
 	    if (handle->pg != NULL)
 		scf_pg_destroy(handle->pg);
 	    if (handle->handle != NULL) {
@@ -112,7 +114,13 @@
 		    handle->scope = scf_scope_create(handle->handle);
 		    handle->service = scf_service_create(handle->handle);
 		    handle->pg = scf_pg_create(handle->handle);
+
+		    /* make sure we have sufficient SMF running */
 		    handle->instance = scf_instance_create(handle->handle);
+		    if (handle->scope == NULL || handle->service == NULL ||
+			handle->pg == NULL || handle->instance == NULL)
+				goto err;
+
 		    if (scf_handle_get_scope(handle->handle,
 					SCF_SCOPE_LOCAL, handle->scope) == 0) {
 			if (scf_scope_get_service(handle->scope,

--- a/usr/src/lib/libshare/nfs/libshare_nfs.c	Tue May 29 15:28:30 2007 -0700
+++ b/usr/src/lib/libshare/nfs/libshare_nfs.c	Tue May 29 15:54:57 2007 -0700
@@ -179,10 +179,10 @@
 {
 	int i;
 	if (name != NULL) {
-	    for (i = 0; optdefs[i].tag != NULL; i++) {
-		if (strcmp(optdefs[i].tag, name) == 0)
-		    return (i);
-	    }
+		for (i = 0; optdefs[i].tag != NULL; i++) {
+			if (strcmp(optdefs[i].tag, name) == 0)
+				return (i);
+		}
 	}
 	return (-1);
 }
@@ -200,7 +200,7 @@
 
 	optdef = findopt(name);
 	if (optdef != -1)
-	    return (optdefs[optdef].type);
+		return (optdefs[optdef].type);
 	return (OPT_TYPE_ANY);
 }
 
@@ -219,7 +219,7 @@
 	(void) memset(&secinfo, '\0', sizeof (secinfo));
 	err = nfs_getseconfig_byname(mode, &secinfo);
 	if (err == SC_NOERROR)
-	    return (1);
+		return (1);
 	return (0);
 }
 
@@ -236,8 +236,8 @@
 	int i;
 
 	for (i = 0; seclist[i] != NULL; i++) {
-	    if (strcmp(tok, seclist[i]) == 0)
-		return (1);
+		if (strcmp(tok, seclist[i]) == 0)
+			return (1);
 	}
 	return (0);
 }
@@ -253,9 +253,9 @@
 find_security(struct securities *seclist, sa_security_t sec)
 {
 	while (seclist != NULL) {
-	    if (seclist->security == sec)
-		return (1);
-	    seclist = seclist->next;
+		if (seclist->security == sec)
+			return (1);
+		seclist = seclist->next;
 	}
 	return (0);
 }
@@ -277,52 +277,52 @@
 	int freetok = 0;
 
 	for (tok = securitymodes; tok != NULL; tok = next) {
-	    next = strchr(tok, ':');
-	    if (next != NULL)
-		*next++ = '\0';
-	    if (strcmp(tok, "default") == 0) {
-		/* resolve default into the real type */
-		tok = nfs_space_alias(tok);
-		freetok = 1;
-	    }
-	    check = sa_get_security(group, tok, proto);
+		next = strchr(tok, ':');
+		if (next != NULL)
+			*next++ = '\0';
+		if (strcmp(tok, "default") == 0) {
+			/* resolve default into the real type */
+			tok = nfs_space_alias(tok);
+			freetok = 1;
+		}
+		check = sa_get_security(group, tok, proto);
 
-	    /* add to the security list if it isn't there already */
-	    if (check == NULL || !find_security(headp, check)) {
-		curp = (struct securities *)calloc(1,
-						sizeof (struct securities));
-		if (curp != NULL) {
-		    if (check == NULL) {
-			curp->security = sa_create_security(group, tok,
-								proto);
-		    } else {
-			curp->security = check;
-		    }
-			/*
-			 * note that the first time through the loop,
-			 * headp will be NULL and prev will be
-			 * undefined.  Since headp is NULL, we set
-			 * both it and prev to the curp (first
-			 * structure to be allocated).
-			 *
-			 * later passes through the loop will have
-			 * headp not being NULL and prev will be used
-			 * to allocate at the end of the list.
-			 */
-		    if (headp == NULL) {
-			headp = curp;
-			prev = curp;
-		    } else {
-			prev->next = curp;
-			prev = curp;
-		    }
+		/* add to the security list if it isn't there already */
+		if (check == NULL || !find_security(headp, check)) {
+			curp = (struct securities *)calloc(1,
+			    sizeof (struct securities));
+			if (curp != NULL) {
+				if (check == NULL) {
+					curp->security = sa_create_security(
+					    group, tok, proto);
+				} else {
+					curp->security = check;
+				}
+				/*
+				 * note that the first time through the loop,
+				 * headp will be NULL and prev will be
+				 * undefined.  Since headp is NULL, we set
+				 * both it and prev to the curp (first
+				 * structure to be allocated).
+				 *
+				 * later passes through the loop will have
+				 * headp not being NULL and prev will be used
+				 * to allocate at the end of the list.
+				 */
+				if (headp == NULL) {
+					headp = curp;
+					prev = curp;
+				} else {
+					prev->next = curp;
+					prev = curp;
+				}
+			}
 		}
-	    }
 
-	    if (freetok) {
-		freetok = 0;
-		sa_free_attr_string(tok);
-	    }
+		if (freetok) {
+			freetok = 0;
+			sa_free_attr_string(tok);
+		}
 	}
 	return (headp);
 }
@@ -332,10 +332,10 @@
 {
 	struct securities *next;
 	if (sec != NULL) {
-	    for (next = sec->next; sec != NULL; sec = next) {
-		next = sec->next;
-		free(sec);
-	    }
+		for (next = sec->next; sec != NULL; sec = next) {
+			next = sec->next;
+			free(sec);
+		}
 	}
 }
 
@@ -355,7 +355,7 @@
 	len = strlen(str1) + strlen(str2) + 2;
 	newstr = (char *)malloc(len);
 	if (newstr != NULL)
-	    (void) snprintf(newstr, len, "%s%c%s", str1, sep, str2);
+		(void) snprintf(newstr, len, "%s%c%s", str1, sep, str2);
 	return (newstr);
 }
 
@@ -374,12 +374,13 @@
 	int ret = SA_OK;
 
 	for (; sec != NULL; sec = sec->next) {
-	    if (value == NULL) {
-		if (strcmp(name, SHOPT_RW) == 0 || strcmp(name, SHOPT_RO) == 0)
-		    value = "*";
-		else
-		    value = "true";
-	    }
+		if (value == NULL) {
+			if (strcmp(name, SHOPT_RW) == 0 ||
+			    strcmp(name, SHOPT_RO) == 0)
+				value = "*";
+			else
+				value = "true";
+		}
 
 		/*
 		 * Get the existing property, if it exists, so we can
@@ -390,56 +391,65 @@
 		 * rw="host2" is seen, the values are merged into a
 		 * single rw="host1:host2".
 		 */
-	    prop = sa_get_property(sec->security, name);
+		prop = sa_get_property(sec->security, name);
 
-	    if (prop != NULL) {
-		char *oldvalue;
-		char *newvalue;
+		if (prop != NULL) {
+			char *oldvalue;
+			char *newvalue;
 
-		/*
-		 * The security options of ro/rw/root might appear
-		 * multiple times. If they do, the values need to be
-		 * merged into an access list. If it was previously
-		 * empty, the new value alone is added.
-		 */
-		oldvalue = sa_get_property_attr(prop, "value");
-		if (oldvalue != NULL) {
 			/*
-			 * The general case is to concatenate the new
-			 * value onto the old value for multiple
-			 * rw(ro/root) properties. A special case
-			 * exists when either the old or new is the
-			 * "all" case. In the special case, if both
-			 * are "all", then it is "all", else if one is
-			 * an access-list, that replaces the "all".
+			 * The security options of ro/rw/root might appear
+			 * multiple times. If they do, the values need to be
+			 * merged into an access list. If it was previously
+			 * empty, the new value alone is added.
 			 */
-		    if (strcmp(oldvalue, "*") == 0) {
-			/* Replace old value with new value. */
-			newvalue = strdup(value);
-		    } else if (strcmp(value, "*") == 0) {
-			/* Keep old value and ignore the new value. */
-			newvalue = NULL;
-		    } else {
-			/* Make a new list of old plus new access-list. */
-			newvalue = nfs_alistcat(oldvalue, value, ':');
-		    }
+			oldvalue = sa_get_property_attr(prop, "value");
+			if (oldvalue != NULL) {
+				/*
+				 * The general case is to concatenate the new
+				 * value onto the old value for multiple
+				 * rw(ro/root) properties. A special case
+				 * exists when either the old or new is the
+				 * "all" case. In the special case, if both
+				 * are "all", then it is "all", else if one is
+				 * an access-list, that replaces the "all".
+				 */
+				if (strcmp(oldvalue, "*") == 0) {
+					/* Replace old value with new value. */
+					newvalue = strdup(value);
+				} else if (strcmp(value, "*") == 0) {
+					/*
+					 * Keep old value and ignore
+					 * the new value.
+					 */
+					newvalue = NULL;
+				} else {
+					/*
+					 * Make a new list of old plus new
+					 * access-list.
+					 */
+					newvalue = nfs_alistcat(oldvalue,
+					    value, ':');
+				}
 
-		    if (newvalue != NULL) {
-			(void) sa_remove_property(prop);
-			prop = sa_create_property(name, newvalue);
+				if (newvalue != NULL) {
+					(void) sa_remove_property(prop);
+					prop = sa_create_property(name,
+					    newvalue);
+					ret = sa_add_property(sec->security,
+					    prop);
+					free(newvalue);
+				}
+				if (oldvalue != NULL)
+					sa_free_attr_string(oldvalue);
+			}
+		} else {
+			prop = sa_create_property(name, value);
 			ret = sa_add_property(sec->security, prop);
-			free(newvalue);
-		    }
-		    if (oldvalue != NULL)
-			sa_free_attr_string(oldvalue);
 		}
-	    } else {
-		prop = sa_create_property(name, value);
-		ret = sa_add_property(sec->security, prop);
-	    }
-	    if (ret == SA_OK && !iszfs) {
-		ret = sa_commit_properties(sec->security, !persist);
-	    }
+		if (ret == SA_OK && !iszfs) {
+			ret = sa_commit_properties(sec->security, !persist);
+		}
 	}
 	return (ret);
 }
@@ -455,9 +465,9 @@
 
 	type = sa_get_group_attr(group, "type");
 	if (type != NULL && strcmp(type, "persist") != 0)
-	    persist = 0;
+		persist = 0;
 	if (type != NULL)
-	    sa_free_attr_string(type);
+		sa_free_attr_string(type);
 	return (persist);
 }
 
@@ -476,28 +486,29 @@
 	copy = strdup(options);
 	token = base = copy;
 	while (token != NULL && ret == 0) {
-	    token = strtok(base, ",");
-	    base = NULL;
-	    if (token != NULL) {
-		value = strchr(token, '=');
-		if (value != NULL)
-		    *value++ = '\0';
-		if (strcmp(token, "sec") == 0) {
-		    /* have security flavors so check them */
-		    char *tok, *next;
-		    for (next = NULL, tok = value; tok != NULL; tok = next) {
-			next = strchr(tok, ':');
-			if (next != NULL)
-			    *next++ = '\0';
-			ret = !nfs_validate_security_mode(tok);
-			if (ret)
-			    break;
-		    }
+		token = strtok(base, ",");
+		base = NULL;
+		if (token != NULL) {
+			value = strchr(token, '=');
+			if (value != NULL)
+				*value++ = '\0';
+			if (strcmp(token, "sec") == 0) {
+				/* HAVE security flavors so check them */
+				char *tok, *next;
+				for (next = NULL, tok = value; tok != NULL;
+				    tok = next) {
+					next = strchr(tok, ':');
+					if (next != NULL)
+						*next++ = '\0';
+					ret = !nfs_validate_security_mode(tok);
+					if (ret)
+						break;
+				}
+			}
 		}
-	    }
 	}
 	if (copy != NULL)
-	    free(copy);
+		free(copy);
 	return (ret);
 }
 
@@ -526,8 +537,8 @@
 	/* do we have an existing optionset? */
 	optionset = sa_get_optionset(group, "nfs");
 	if (optionset == NULL) {
-	    /* didn't find existing optionset so create one */
-	    optionset = sa_create_optionset(group, "nfs");
+		/* didn't find existing optionset so create one */
+		optionset = sa_create_optionset(group, "nfs");
 	} else {
 		/*
 		 * have an existing optionset so we need to compare
@@ -536,7 +547,7 @@
 		 * and the others will be the same. This needs to be
 		 * fixed before the final code is ready.
 		 */
-	    return (ret);
+		return (ret);
 	}
 
 	if (strcmp(options, SHOPT_RW) == 0) {
@@ -545,7 +556,7 @@
 		 * being the default option. We don't have to do
 		 * anything.
 		 */
-	    return (ret);
+		return (ret);
 	}
 
 	/*
@@ -554,7 +565,7 @@
 	 */
 
 	if (invalid_security(options)) {
-	    return (SA_INVALID_SECURITY);
+		return (SA_INVALID_SECURITY);
 	}
 
 	/*
@@ -562,17 +573,17 @@
 	 * absolutely necessary, we never do it in the legacy parsing.
 	 */
 	if (sa_is_share(group)) {
-	    char *zfs;
-	    parent = sa_get_parent_group(group);
-	    if (parent != NULL) {
-		zfs = sa_get_group_attr(parent, "zfs");
-		if (zfs != NULL) {
-		    sa_free_attr_string(zfs);
-		    iszfs++;
+		char *zfs;
+		parent = sa_get_parent_group(group);
+		if (parent != NULL) {
+			zfs = sa_get_group_attr(parent, "zfs");
+			if (zfs != NULL) {
+				sa_free_attr_string(zfs);
+				iszfs++;
+			}
 		}
-	    }
 	} else {
-	    iszfs = sa_group_is_zfs(group);
+		iszfs = sa_group_is_zfs(group);
 	}
 
 	/*
@@ -587,96 +598,116 @@
 	token = dup;
 	lasts = NULL;
 	while (token != NULL && ret == SA_OK) {
-	    ret = SA_OK;
-	    token = strtok_r(base, ",", &lasts);
-	    base = NULL;
-	    if (token != NULL) {
-		char *value;
-		/*
-		 * if the option has a value, it will have an '=' to
-		 * separate the name from the value. The following
-		 * code will result in value != NULL and token
-		 * pointing to just the name if there is a value.
-		 */
-		value = strchr(token, '=');
-		if (value != NULL) {
-		    *value++ = '\0';
-		}
-		if (strcmp(token, "sec") == 0 || strcmp(token, "secure") == 0) {
-			/*
-			 * Once in security parsing, we only
-			 * do security. We do need to move
-			 * between the security node and the
-			 * toplevel. The security tag goes on
-			 * the root while the following ones
-			 * go on the security.
-			 */
-		    if (security_list != NULL) {
-			/* have an old list so close it and start the new */
-			free_security_list(security_list);
-		    }
-		    if (strcmp(token, "secure") == 0) {
-			value = "dh";
-		    } else {
-			if (value == NULL) {
-			    ret = SA_SYNTAX_ERR;
-			    break;
-			}
-		    }
-		    security_list = make_security_list(group, value, "nfs");
-		} else {
+		ret = SA_OK;
+		token = strtok_r(base, ",", &lasts);
+		base = NULL;
+		if (token != NULL) {
+			char *value;
 			/*
-			 * Note that the "old" syntax allowed a
-			 * default security model This must be
-			 * accounted for and internally converted to
-			 * "standard" security structure.
+			 * if the option has a value, it will have an '=' to
+			 * separate the name from the value. The following
+			 * code will result in value != NULL and token
+			 * pointing to just the name if there is a value.
 			 */
-		    if (nfs_is_security_opt(token)) {
-			if (security_list == NULL) {
+			value = strchr(token, '=');
+			if (value != NULL) {
+				*value++ = '\0';
+			}
+			if (strcmp(token, "sec") == 0 ||
+			    strcmp(token, "secure") == 0) {
 				/*
-				 * need to have a security option. This
-				 * will be "closed" when a defined "sec="
-				 * option is seen. This is technically an
-				 * error but will be allowed with warning.
+				 * Once in security parsing, we only
+				 * do security. We do need to move
+				 * between the security node and the
+				 * toplevel. The security tag goes on
+				 * the root while the following ones
+				 * go on the security.
 				 */
-			    security_list = make_security_list(group,
-								"default",
-								"nfs");
-			}
-			if (security_list != NULL) {
-			    ret = add_security_prop(security_list, token,
-							value, persist,
-							iszfs);
+				if (security_list != NULL) {
+					/*
+					 * have an old list so close it and
+					 * start the new
+					 */
+					free_security_list(security_list);
+				}
+				if (strcmp(token, "secure") == 0) {
+					value = "dh";
+				} else {
+					if (value == NULL) {
+						ret = SA_SYNTAX_ERR;
+						break;
+					}
+				}
+				security_list = make_security_list(group,
+				    value, "nfs");
 			} else {
-			    ret = SA_NO_MEMORY;
+				/*
+				 * Note that the "old" syntax allowed a
+				 * default security model This must be
+				 * accounted for and internally converted to
+				 * "standard" security structure.
+				 */
+				if (nfs_is_security_opt(token)) {
+					if (security_list == NULL) {
+						/*
+						 * need to have a
+						 * security
+						 * option. This will
+						 * be "closed" when a
+						 * defined "sec="
+						 * option is
+						 * seen. This is
+						 * technically an
+						 * error but will be
+						 * allowed with
+						 * warning.
+						 */
+						security_list =
+						    make_security_list(group,
+						    "default",
+						    "nfs");
+					}
+					if (security_list != NULL) {
+						ret = add_security_prop(
+						    security_list, token,
+						    value, persist, iszfs);
+					} else {
+						ret = SA_NO_MEMORY;
+					}
+				} else {
+					/* regular options */
+					if (value == NULL) {
+						if (strcmp(token, SHOPT_RW) ==
+						    0 || strcmp(token,
+						    SHOPT_RO) == 0) {
+							value = "*";
+						} else {
+							value = "global";
+							if (strcmp(token,
+							    SHOPT_LOG) != 0) {
+								value = "true";
+							}
+						}
+						prop = sa_create_property(
+						    token, value);
+						ret =
+						    sa_add_property(optionset,
+						    prop);
+						if (ret != SA_OK)
+							break;
+					}
+					if (!iszfs) {
+						ret = sa_commit_properties(
+						    optionset, !persist);
+					}
+				}
 			}
-		    } else {
-			/* regular options */
-			if (value == NULL) {
-			    if (strcmp(token, SHOPT_RW) == 0 ||
-				strcmp(token, SHOPT_RO) == 0)
-				value = "*";
-			    else if (strcmp(token, SHOPT_LOG) == 0)
-				value = "global";
-			    else
-				value = "true";
-			}
-			prop = sa_create_property(token, value);
-			ret = sa_add_property(optionset, prop);
-			if (ret != SA_OK) {
-			    break;
-			}
-			if (!iszfs) {
-			    ret = sa_commit_properties(optionset, !persist);
-			}
-		    }
 		}
-	    }
 	}
 	if (security_list != NULL)
-	    free_security_list(security_list);
+		free_security_list(security_list);
 	if (dup != NULL)
-	    free(dup);
+		free(dup);
 	return (ret);
 }
 
@@ -693,17 +724,17 @@
 	int hex = 0;
 
 	if (strncmp(number, "0x", 2) == 0) {
-	    number += 2;
-	    hex = 1;
-	} else if (*number == '-')
-	    number++; /* skip the minus */
-
+		number += 2;
+		hex = 1;
+	} else if (*number == '-') {
+		number++; /* skip the minus */
+	}
 	while (ret == 1 && *number != '\0') {
-	    if (hex) {
-		ret = isxdigit(*number++);
-	    } else {
-		ret = isdigit(*number++);
-	    }
+		if (hex) {
+			ret = isxdigit(*number++);
+		} else {
+			ret = isdigit(*number++);
+		}
 	}
 	return (ret);
 }
@@ -730,9 +761,8 @@
 	error = nfsl_getconfig_list(&configlist);
 	if (error) {
 		(void) fprintf(stderr,
-				dgettext(TEXT_DOMAIN,
-					"Cannot get log configuration: %s\n"),
-				strerror(error));
+		    dgettext(TEXT_DOMAIN, "Cannot get log configuration: %s\n"),
+		    strerror(error));
 	}
 
 	if (tag == NULL)
@@ -740,8 +770,7 @@
 	if ((configp = nfsl_findconfig(configlist, tag, &error)) == NULL) {
 		nfsl_freeconfig_list(&configlist);
 		(void) fprintf(stderr,
-				dgettext(TEXT_DOMAIN,
-					"No tags matching \"%s\"\n"), tag);
+		    dgettext(TEXT_DOMAIN, "No tags matching \"%s\"\n"), tag);
 		/* bad configuration */
 		error = ENOENT;
 		goto err;
@@ -760,7 +789,7 @@
 		exp->ex_flags |= EX_LOG_ALLOPS;
 out:
 	if (configlist != NULL)
-	    nfsl_freeconfig_list(&configlist);
+		nfsl_freeconfig_list(&configlist);
 
 err:
 	if (error != 0) {
@@ -769,9 +798,8 @@
 		if (exp->ex_log_buffer != NULL)
 			free(exp->ex_log_buffer);
 		(void) fprintf(stderr,
-				dgettext(TEXT_DOMAIN,
-					"Cannot set log configuration: %s\n"),
-				strerror(error));
+		    dgettext(TEXT_DOMAIN, "Cannot set log configuration: %s\n"),
+		    strerror(error));
 	}
 }
 
@@ -791,111 +819,109 @@
 	int ret = SA_OK;
 
 	for (option = sa_get_property(optionset, NULL);
-		option != NULL; option = sa_get_next_property(option)) {
-	    char *name;
-	    char *value;
-	    uint32_t val;
+	    option != NULL; option = sa_get_next_property(option)) {
+		char *name;
+		char *value;
+		uint32_t val;
 
-	/*
-	 * since options may be set/reset multiple times, always do an
-	 * explicit set or clear of the option. This allows defaults
-	 * to be set and then the protocol specifici to override.
-	 */
+		/*
+		 * since options may be set/reset multiple times, always do an
+		 * explicit set or clear of the option. This allows defaults
+		 * to be set and then the protocol specifici to override.
+		 */
 
-	    name = sa_get_property_attr(option, "type");
-	    value = sa_get_property_attr(option, "value");
-	    switch (findopt(name)) {
-	    case OPT_ANON:
-		if (value != NULL && is_a_number(value)) {
-		    val = strtoul(value, NULL, 0);
-		} else {
-		    struct passwd *pw;
-		    pw = getpwnam(value != NULL ? value : "nobody");
-		    if (pw != NULL) {
-			val = pw->pw_uid;
-		    } else {
-			val = UID_NOBODY;
-		    }
-		    endpwent();
+		name = sa_get_property_attr(option, "type");
+		value = sa_get_property_attr(option, "value");
+		switch (findopt(name)) {
+		case OPT_ANON:
+			if (value != NULL && is_a_number(value)) {
+				val = strtoul(value, NULL, 0);
+			} else {
+				struct passwd *pw;
+				pw = getpwnam(value != NULL ? value : "nobody");
+				if (pw != NULL) {
+					val = pw->pw_uid;
+				} else {
+					val = UID_NOBODY;
+				}
+				endpwent();
+			}
+			export->ex_anon = val;
+			break;
+		case OPT_NOSUID:
+			if (value != NULL && (strcasecmp(value, "true") == 0 ||
+			    strcmp(value, "1") == 0))
+				export->ex_flags |= EX_NOSUID;
+			else
+				export->ex_flags &= ~EX_NOSUID;
+			break;
+		case OPT_ACLOK:
+			if (value != NULL && (strcasecmp(value, "true") == 0 ||
+			    strcmp(value, "1") == 0))
+				export->ex_flags |= EX_ACLOK;
+			else
+				export->ex_flags &= ~EX_ACLOK;
+			break;
+		case OPT_NOSUB:
+			if (value != NULL && (strcasecmp(value, "true") == 0 ||
+			    strcmp(value, "1") == 0))
+				export->ex_flags |= EX_NOSUB;
+			else
+				export->ex_flags &= ~EX_NOSUB;
+			break;
+		case OPT_PUBLIC:
+			if (value != NULL && (strcasecmp(value, "true") == 0 ||
+			    strcmp(value, "1") == 0))
+				export->ex_flags |= EX_PUBLIC;
+			else
+				export->ex_flags &= ~EX_PUBLIC;
+			break;
+		case OPT_INDEX:
+			if (value != NULL && (strcmp(value, "..") == 0 ||
+			    strchr(value, '/') != NULL)) {
+				/* this is an error */
+				(void) printf(dgettext(TEXT_DOMAIN,
+				    "NFS: index=\"%s\" not valid;"
+				    "must be a filename.\n"),
+				    value);
+				break;
+			}
+			if (value != NULL && *value != '\0' &&
+			    strcmp(value, ".") != 0) {
+				/* valid index file string */
+				if (export->ex_index != NULL) {
+					/* left over from "default" */
+					free(export->ex_index);
+				}
+				/* remember to free */
+				export->ex_index = strdup(value);
+				if (export->ex_index == NULL) {
+					(void) printf(dgettext(TEXT_DOMAIN,
+					    "NFS: out of memory setting "
+					    "index property\n"));
+					break;
+				}
+				export->ex_flags |= EX_INDEX;
+			}
+			break;
+		case OPT_LOG:
+			if (value == NULL)
+				value = strdup("global");
+			if (value != NULL)
+				configlog(export,
+				    strlen(value) ? value : "global");
+			break;
+		default:
+			/* have a syntactic error */
+			(void) printf(dgettext(TEXT_DOMAIN,
+			    "NFS: unrecognized option %s=%s\n"),
+			    name, value != NULL ? value : "");
+			break;
 		}
-		export->ex_anon = val;
-		break;
-	    case OPT_NOSUID:
-		if (value != NULL &&
-		    (strcasecmp(value, "true") == 0 || strcmp(value, "1") == 0))
-		    export->ex_flags |= EX_NOSUID;
-		else
-		    export->ex_flags &= ~EX_NOSUID;
-		break;
-	    case OPT_ACLOK:
-		if (value != NULL &&
-		    (strcasecmp(value, "true") == 0 ||
-			strcmp(value, "1") == 0))
-		    export->ex_flags |= EX_ACLOK;
-		else
-		    export->ex_flags &= ~EX_ACLOK;
-		break;
-	    case OPT_NOSUB:
-		if (value != NULL &&
-		    (strcasecmp(value, "true") == 0 || strcmp(value, "1") == 0))
-		    export->ex_flags |= EX_NOSUB;
-		else
-		    export->ex_flags &= ~EX_NOSUB;
-		break;
-	    case OPT_PUBLIC:
-		if (value != NULL &&
-		    (strcasecmp(value, "true") == 0 || strcmp(value, "1") == 0))
-		    export->ex_flags |= EX_PUBLIC;
-		else
-		    export->ex_flags &= ~EX_PUBLIC;
-		break;
-	    case OPT_INDEX:
-		if (value != NULL &&
-		    (strcmp(value, "..") == 0 || strchr(value, '/') != NULL)) {
-		    /* this is an error */
-		    (void) printf(dgettext(TEXT_DOMAIN,
-					    "NFS: index=\"%s\" not valid;"
-					    "must be a filename.\n"),
-			    value);
-		    break;
-		}
-		if (value != NULL && *value != '\0' &&
-			strcmp(value, ".") != 0) {
-		    /* valid index file string */
-		    if (export->ex_index != NULL) {
-			/* left over from "default" */
-			free(export->ex_index);
-		    }
-		    export->ex_index = strdup(value); /* remember to free */
-		    if (export->ex_index == NULL) {
-			(void) printf(dgettext(TEXT_DOMAIN,
-						"NFS: out of memory setting "
-						"index property\n"));
-			break;
-		    }
-		    export->ex_flags |= EX_INDEX;
-		}
-		break;
-	    case OPT_LOG:
-		if (value == NULL)
-		    value = strdup("global");
+		if (name != NULL)
+			sa_free_attr_string(name);
 		if (value != NULL)
-		    configlog(export, strlen(value) ? value : "global");
-		break;
-	    case OPT_CKSUM:
-		/* TBD: not ready yet */
-		break;
-	    default:
-		/* have a syntactic error */
-		(void) printf(dgettext(TEXT_DOMAIN,
-					"NFS: unrecognized option %s=%s\n"),
-			name, value != NULL ? value : "");
-		break;
-	    }
-	    if (name != NULL)
-		sa_free_attr_string(name);
-	    if (value != NULL)
-		sa_free_attr_string(value);
+			sa_free_attr_string(value);
 	}
 	return (ret);
 }
@@ -912,13 +938,12 @@
 	int i;
 
 	if (export->ex_index != NULL)
-	    free(export->ex_index);
+		free(export->ex_index);
 	if (export->ex_secinfo != NULL) {
-	    for (i = 0; i < export->ex_seccnt; i++)
-		if (export->ex_secinfo[i].s_rootnames != NULL) {
-		    free(export->ex_secinfo[i].s_rootnames);
-		}
-	    free(export->ex_secinfo);
+		for (i = 0; i < export->ex_seccnt; i++)
+			if (export->ex_secinfo[i].s_rootnames != NULL)
+				free(export->ex_secinfo[i].s_rootnames);
+		free(export->ex_secinfo);
 	}
 }
 
@@ -950,17 +975,17 @@
 	a = (caddr_t *)malloc(c * sizeof (char *));
 	if (a == NULL) {
 		(void) printf(dgettext(TEXT_DOMAIN,
-					"get_rootnames: no memory\n"));
+		    "get_rootnames: no memory\n"));
 	} else {
-	    for (i = 0; i < c; i++) {
-		host = strtok(list, ":");
-		if (!nfs_get_root_principal(sec, host, &a[i])) {
-		    free(a);
-		    a = NULL;
-		    break;
+		for (i = 0; i < c; i++) {
+			host = strtok(list, ":");
+			if (!nfs_get_root_principal(sec, host, &a[i])) {
+				free(a);
+				a = NULL;
+				break;
+			}
+			list = NULL;
 		}
-		list = NULL;
-	    }
 	}
 
 	return (a);
@@ -982,74 +1007,76 @@
 
 	type = sa_get_security_attr(secopts, "sectype");
 	if (type != NULL) {
-	    /* named security type needs secinfo to be filled in */
-	    err = nfs_getseconfig_byname(type, &sp->s_secinfo);
-	    sa_free_attr_string(type);
-	    if (err != SC_NOERROR)
-		return (err);
+		/* named security type needs secinfo to be filled in */
+		err = nfs_getseconfig_byname(type, &sp->s_secinfo);
+		sa_free_attr_string(type);
+		if (err != SC_NOERROR)
+			return (err);
 	} else {
-	    /* default case */
-	    err = nfs_getseconfig_default(&sp->s_secinfo);
-	    if (err != SC_NOERROR)
-		return (err);
+		/* default case */
+		err = nfs_getseconfig_default(&sp->s_secinfo);
+		if (err != SC_NOERROR)
+			return (err);
 	}
 
 	err = SA_OK;
 	for (prop = sa_get_property(secopts, NULL);
-		prop != NULL && err == SA_OK;
-		prop = sa_get_next_property(prop)) {
-	    char *name;
-	    char *value;
+	    prop != NULL && err == SA_OK;
+	    prop = sa_get_next_property(prop)) {
+		char *name;
+		char *value;
 
-	    name = sa_get_property_attr(prop, "type");
-	    value = sa_get_property_attr(prop, "value");
+		name = sa_get_property_attr(prop, "type");
+		value = sa_get_property_attr(prop, "value");
 
-	    longform = value != NULL && strcmp(value, "*") != 0;
+		longform = value != NULL && strcmp(value, "*") != 0;
 
-	    switch (findopt(name)) {
-	    case OPT_RO:
-		sp->s_flags |= longform ? M_ROL : M_RO;
-		break;
-	    case OPT_RW:
-		sp->s_flags |= longform ? M_RWL : M_RW;
-		break;
-	    case OPT_ROOT:
-		sp->s_flags |= M_ROOT;
-		/*
-		 * if we are using AUTH_UNIX, handle like other things
-		 * such as RO/RW
-		 */
-		if (sp->s_secinfo.sc_rpcnum == AUTH_UNIX)
-		    continue;
-		/* not AUTH_UNIX */
-		if (value != NULL) {
-		    sp->s_rootnames = get_rootnames(&sp->s_secinfo, value,
-							&sp->s_rootcnt);
-		    if (sp->s_rootnames == NULL) {
-			err = SA_BAD_VALUE;
-			(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-							"Bad root list\n"));
-		    }
+		switch (findopt(name)) {
+		case OPT_RO:
+			sp->s_flags |= longform ? M_ROL : M_RO;
+			break;
+		case OPT_RW:
+			sp->s_flags |= longform ? M_RWL : M_RW;
+			break;
+		case OPT_ROOT:
+			sp->s_flags |= M_ROOT;
+			/*
+			 * if we are using AUTH_UNIX, handle like other things
+			 * such as RO/RW
+			 */
+			if (sp->s_secinfo.sc_rpcnum == AUTH_UNIX)
+				continue;
+			/* not AUTH_UNIX */
+			if (value != NULL) {
+				sp->s_rootnames = get_rootnames(&sp->s_secinfo,
+				    value, &sp->s_rootcnt);
+				if (sp->s_rootnames == NULL) {
+					err = SA_BAD_VALUE;
+					(void) fprintf(stderr,
+					    dgettext(TEXT_DOMAIN,
+					    "Bad root list\n"));
+				}
+			}
+			break;
+		case OPT_WINDOW:
+			if (value != NULL) {
+				sp->s_window = atoi(value);
+				/* just in case */
+				if (sp->s_window < 0)
+					sp->s_window = DEF_WIN;
+			}
+			break;
+		default:
+			break;
 		}
-		break;
-	    case OPT_WINDOW:
-		if (value != NULL) {
-		    sp->s_window = atoi(value);
-		    if (sp->s_window < 0)
-			sp->s_window = DEF_WIN; /* just in case */
-		}
-		break;
-	    default:
-		break;
-	    }
-	    if (name != NULL)
-		sa_free_attr_string(name);
-	    if (value != NULL)
-		sa_free_attr_string(value);
+		if (name != NULL)
+			sa_free_attr_string(name);
+		if (value != NULL)
+			sa_free_attr_string(value);
 	}
 	/* if rw/ro options not set, use default of RW */
 	if ((sp->s_flags & NFS_RWMODES) == 0)
-	    sp->s_flags |= M_RW;
+		sp->s_flags |= M_RW;
 	return (err);
 }
 
@@ -1065,7 +1092,7 @@
 	struct secinfo *sp;
 
 	if (debug == 0)
-	    return;
+		return;
 
 	(void) printf("%s:\n", path);
 	(void) printf("\tex_version = %d\n", ep->ex_version);
@@ -1089,9 +1116,9 @@
 	(void) 	printf("\n");
 	if (ep->ex_flags & EX_LOG) {
 		(void) printf("\tex_log_buffer = %s\n",
-			(ep->ex_log_buffer ? ep->ex_log_buffer : "(NULL)"));
+		    (ep->ex_log_buffer ? ep->ex_log_buffer : "(NULL)"));
 		(void) printf("\tex_tag = %s\n",
-				(ep->ex_tag ? ep->ex_tag : "(NULL)"));
+		    (ep->ex_tag ? ep->ex_tag : "(NULL)"));
 	}
 	(void) printf("\tex_anon = %d\n", ep->ex_anon);
 	(void) printf("\tex_seccnt = %d\n", ep->ex_seccnt);
@@ -1112,7 +1139,7 @@
 		(void) fflush(stdout);
 		for (j = 0; j < sp->s_rootcnt; j++)
 			(void) printf("%s ", sp->s_rootnames[j] ?
-					sp->s_rootnames[j] : "<null>");
+			    sp->s_rootnames[j] : "<null>");
 		(void) printf("\n\n");
 	}
 }
@@ -1131,10 +1158,10 @@
 	int count = 0;
 	sa_property_t prop;
 	if (opts != NULL) {
-	    for (prop = sa_get_property(opts, NULL); prop != NULL;
-		prop = sa_get_next_property(prop)) {
-		count++;
-	    }
+		for (prop = sa_get_property(opts, NULL); prop != NULL;
+		    prop = sa_get_next_property(prop)) {
+			count++;
+		}
 	}
 	return (count);
 }
@@ -1162,79 +1189,79 @@
 	name = sa_get_property_attr(prop, "type");
 	value = sa_get_property_attr(prop, "value");
 	if (buff != NULL)
-	    curlen = strlen(buff);
+		curlen = strlen(buff);
 	else
-	    curlen = 0;
+		curlen = 0;
 	if (name != NULL) {
-	    int len;
-	    len = strlen(name) + sep;
+		int len;
+		len = strlen(name) + sep;
 
 		/*
 		 * A future RFE would be to replace this with more
 		 * generic code and to possibly handle more types.
 		 */
-	    switch (gettype(name)) {
-	    case OPT_TYPE_BOOLEAN:
-		if (value != NULL && strcasecmp(value, "false") == 0) {
-		    *name = '\0';
-		}
-		if (value != NULL)
-		    sa_free_attr_string(value);
-		value = NULL;
-		break;
-	    case OPT_TYPE_ACCLIST:
-		if (value != NULL && strcmp(value, "*") == 0) {
-		    sa_free_attr_string(value);
-		    value = NULL;
-		} else {
-		    if (value != NULL)
-			len += 1 + strlen(value);
-		}
-		break;
-	    case OPT_TYPE_LOGTAG:
-		if (value != NULL && strlen(value) == 0) {
-		    sa_free_attr_string(value);
-		    value = NULL;
-		} else {
-		    if (value != NULL)
-			len += 1 + strlen(value);
+		switch (gettype(name)) {
+		case OPT_TYPE_BOOLEAN:
+			if (value != NULL && strcasecmp(value, "false") == 0)
+				*name = '\0';
+			if (value != NULL)
+				sa_free_attr_string(value);
+			value = NULL;
+			break;
+		case OPT_TYPE_ACCLIST:
+			if (value != NULL && strcmp(value, "*") == 0) {
+				sa_free_attr_string(value);
+				value = NULL;
+			} else {
+				if (value != NULL)
+					len += 1 + strlen(value);
+			}
+			break;
+		case OPT_TYPE_LOGTAG:
+			if (value != NULL && strlen(value) == 0) {
+				sa_free_attr_string(value);
+				value = NULL;
+			} else {
+				if (value != NULL)
+					len += 1 + strlen(value);
+			}
+			break;
+		default:
+			if (value != NULL)
+				len += 1 + strlen(value);
+			break;
 		}
-		break;
-	    default:
-		if (value != NULL)
-		    len += 1 + strlen(value);
-		break;
-	    }
-	    while (buffsize <= (curlen + len)) {
-		/* need more room */
-		buffsize += incr;
-		buff = realloc(buff, buffsize);
-		if (buff == NULL) {
-		    /* realloc failed so free everything */
-		    if (*rbuff != NULL)
-			free(*rbuff);
+		while (buffsize <= (curlen + len)) {
+			/* need more room */
+			buffsize += incr;
+			buff = realloc(buff, buffsize);
+			if (buff == NULL) {
+				/* realloc failed so free everything */
+				if (*rbuff != NULL)
+					free(*rbuff);
+			}
+			*rbuff = buff;
+			*rbuffsize = buffsize;
+			if (buff == NULL) {
+				return;
+			}
 		}
-		*rbuff = buff;
-		*rbuffsize = buffsize;
-		if (buff == NULL) {
-		    return;
+		if (buff == NULL)
+			return;
+		if (value == NULL) {
+			(void) snprintf(buff + curlen, buffsize - curlen,
+			    "%s%s", sep ? "," : "",
+			    name, value != NULL ? value : "");
+		} else {
+			(void) snprintf(buff + curlen, buffsize - curlen,
+			    "%s%s=%s", sep ? "," : "",
+			    name, value != NULL ? value : "");
 		}
-	    }
-	    if (buff == NULL)
-		return;
-	    if (value == NULL)
-		(void) snprintf(buff + curlen, buffsize - curlen,
-				"%s%s", sep ? "," : "",
-				name, value != NULL ? value : "");
-	    else
-		(void) snprintf(buff + curlen, buffsize - curlen,
-				"%s%s=%s", sep ? "," : "",
-				name, value != NULL ? value : "");
 	}
 	if (name != NULL)
-	    sa_free_attr_string(name);
+		sa_free_attr_string(name);
 	if (value != NULL)
-	    sa_free_attr_string(value);
+		sa_free_attr_string(value);
 }
 
 /*
@@ -1249,99 +1276,121 @@
 nfs_format_options(sa_group_t group, int hier)
 {
 	sa_optionset_t options = NULL;
-	sa_optionset_t secoptions;
+	sa_optionset_t secoptions = NULL;
 	sa_property_t prop, secprop;
-	sa_security_t security;
+	sa_security_t security = NULL;
 	char *buff;
 	size_t buffsize;
+	char *sectype = NULL;
+	int sep = 0;
+
+
+	buff = malloc(OPT_CHUNK);
+	if (buff == NULL) {
+		return (NULL);
+	}
+
+	buff[0] = '\0';
+	buffsize = OPT_CHUNK;
+
+	/*
+	 * We may have a an optionset relative to this item. format
+	 * these if we find them and then add any security definitions.
+	 */
 
 	options = sa_get_derived_optionset(group, "nfs", hier);
 
 	/*
-	 * have a an optionset relative to this item, if any. format
-	 * these then add any security definitions.
+	 * do the default set first but skip any option that is also
+	 * in the protocol specific optionset.
 	 */
-	buff = malloc(OPT_CHUNK);
-	if (buff != NULL) {
-	    int sep = 0;
-	    buff[0] = '\0';
-	    buffsize = OPT_CHUNK;
-		/*
-		 * do the default set first but skip any option that is also
-		 * in the protocol specific optionset.
-		 */
-	    if (options != NULL) {
-		for (prop = sa_get_property(options, NULL); prop != NULL;
-			prop = sa_get_next_property(prop)) {
+	if (options != NULL) {
+		for (prop = sa_get_property(options, NULL);
+		    prop != NULL; prop = sa_get_next_property(prop)) {
 			/*
-			 * use this one since we skipped any of these that
-			 * were also in optdefault
+			 * use this one since we skipped any
+			 * of these that were also in
+			 * optdefault
 			 */
-		    nfs_sprint_option(&buff, &buffsize, OPT_CHUNK, prop, sep);
-		    if (buff == NULL) {
-			/*
-			 * buff could become NULL if there isn't
-			 * enough memory for nfs_sprint_option to
-			 * realloc() as necessary. We can't really do
-			 * anything about it at this point so we
-			 * return NULL.  The caller should handle the
-			 * failure. Note that this
-			 */
-			return (buff);
-		    }
-		    sep = 1;
+			nfs_sprint_option(&buff, &buffsize, OPT_CHUNK,
+			    prop, sep);
+			if (buff == NULL) {
+				/*
+				 * buff could become NULL if there
+				 * isn't enough memory for
+				 * nfs_sprint_option to realloc()
+				 * as necessary. We can't really
+				 * do anything about it at this
+				 * point so we return NULL.  The
+				 * caller should handle the
+				 * failure.
+				 */
+				if (options != NULL)
+					sa_free_derived_optionset(
+					    options);
+				return (buff);
+			}
+			sep = 1;
 		}
-	    }
-	    secoptions = (sa_optionset_t)sa_get_all_security_types(group,
-								"nfs", hier);
-	    if (secoptions != NULL) {
+	}
+	secoptions = (sa_optionset_t)sa_get_all_security_types(group,
+	    "nfs", hier);
+	if (secoptions != NULL) {
 		for (secprop = sa_get_property(secoptions, NULL);
-		    secprop != NULL; secprop = sa_get_next_property(secprop)) {
-		    char *sectype;
-
-		    sectype = sa_get_property_attr(secprop, "type");
-		    security = (sa_security_t)sa_get_derived_security(group,
-								sectype,
-								"nfs", hier);
-		    if (security != NULL) {
-			if (sectype != NULL) {
-			    prop = sa_create_property("sec", sectype);
-			    nfs_sprint_option(&buff, &buffsize, OPT_CHUNK,
-						prop, sep);
-			    (void) sa_remove_property(prop);
-			sep = 1;
+		    secprop != NULL;
+		    secprop = sa_get_next_property(secprop)) {
+			sectype = sa_get_property_attr(secprop, "type");
+			security =
+			    (sa_security_t)sa_get_derived_security(
+			    group, sectype, "nfs", hier);
+			if (security != NULL) {
+				if (sectype != NULL) {
+					prop = sa_create_property(
+					    "sec", sectype);
+					nfs_sprint_option(&buff,
+					    &buffsize, OPT_CHUNK,
+					    prop, sep);
+					(void) sa_remove_property(prop);
+					sep = 1;
+				}
+				for (prop = sa_get_property(security,
+				    NULL); prop != NULL;
+				    prop = sa_get_next_property(prop)) {
+					nfs_sprint_option(&buff,
+					    &buffsize, OPT_CHUNK, prop,
+					    sep);
+					if (buff == NULL)
+						goto err;
+					sep = 1;
+				}
+				sa_free_derived_optionset(security);
 			}
-			for (prop = sa_get_property(security, NULL);
-			    prop != NULL;
-			    prop = sa_get_next_property(prop)) {
-
-			    nfs_sprint_option(&buff, &buffsize, OPT_CHUNK,
-						prop, sep);
-			    if (buff == NULL) {
-				/* catastrophic memory failure */
-				sa_free_derived_optionset(secoptions);
-				if (security != NULL)
-				    sa_free_derived_optionset(security);
-				if (sectype != NULL)
-				    sa_free_attr_string(sectype);
-				if (options != NULL)
-				    sa_free_derived_optionset(options);
-				return (buff);
-			    }
-			    sep = 1;
-			}
-			sa_free_derived_optionset(security);
-		    }
-		    if (sectype != NULL)
-			sa_free_attr_string(sectype);
+			if (sectype != NULL)
+				sa_free_attr_string(sectype);
 		}
 		sa_free_derived_optionset(secoptions);
-	    }
 	}
+
 	if (options != NULL)
-	    sa_free_derived_optionset(options);
+		sa_free_derived_optionset(options);
+	return (buff);
+
+err:
+	/*
+	 * If we couldn't allocate memory for option printing, we need
+	 * to break out of the nested loops, cleanup and return NULL.
+	 */
+	if (secoptions != NULL)
+		sa_free_derived_optionset(secoptions);
+	if (security != NULL)
+		sa_free_derived_optionset(security);
+	if (sectype != NULL)
+		sa_free_attr_string(sectype);
+	if (options != NULL)
+		sa_free_derived_optionset(options);
 	return (buff);
 }
+
 /*
  * Append an entry to the nfslogtab file
  */
@@ -1367,16 +1416,16 @@
 
 	if (lockf(fileno(f), F_LOCK, 0L) < 0) {
 		(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however failed to lock %s "
-			"for update: %s\n"), NFSLOGTAB, strerror(errno));
+		    "share complete, however failed to lock %s "
+		    "for update: %s\n"), NFSLOGTAB, strerror(errno));
 		error = -1;
 		goto out;
 	}
 
 	if (logtab_deactivate_after_boot(f) == -1) {
 		(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however could not deactivate "
-			"entries in %s\n"), NFSLOGTAB);
+		    "share complete, however could not deactivate "
+		    "entries in %s\n"), NFSLOGTAB);
 		error = -1;
 		goto out;
 	}
@@ -1387,8 +1436,8 @@
 	 */
 	if (logtab_rement(f, buffer, dir, NULL, -1)) {
 		(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however could not remove matching "
-			"entries in %s\n"), NFSLOGTAB);
+		    "share complete, however could not remove matching "
+		    "entries in %s\n"), NFSLOGTAB);
 		error = -1;
 		goto out;
 	}
@@ -1398,8 +1447,8 @@
 	 */
 	if (logtab_deactivate(f, NULL, dir, NULL)) {
 		(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however could not deactivate matching "
-			"entries in %s\n"), NFSLOGTAB);
+		    "share complete, however could not deactivate matching "
+		    "entries in %s\n"), NFSLOGTAB);
 		error = -1;
 		goto out;
 	}
@@ -1414,8 +1463,8 @@
 	 */
 	if (logtab_putent(f, &lep) < 0) {
 		(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however could not add %s to %s\n"),
-			dir, NFSLOGTAB);
+		    "share complete, however could not add %s to %s\n"),
+		    dir, NFSLOGTAB);
 		error = -1;
 	}
 
@@ -1443,16 +1492,16 @@
 	if (lockf(fileno(f), F_LOCK, 0L) < 0) {
 		error = errno;
 		(void)  fprintf(stderr, dgettext(TEXT_DOMAIN,
-			"share complete, however could not lock %s for "
-			"update: %s\n"), NFSLOGTAB, strerror(error));
+		    "share complete, however could not lock %s for "
+		    "update: %s\n"), NFSLOGTAB, strerror(error));
 		goto out;
 	}
 	if (logtab_deactivate(f, NULL, path, NULL) == -1) {
 		error = -1;
 		(void) fprintf(stderr,
-			dgettext(TEXT_DOMAIN,
-				"share complete, however could not "
-				"deactivate %s in %s\n"), path, NFSLOGTAB);
+		    dgettext(TEXT_DOMAIN,
+		    "share complete, however could not "
+		    "deactivate %s in %s\n"), path, NFSLOGTAB);
 		goto out;
 	}
 
@@ -1480,32 +1529,34 @@
 
 	group = sa_get_parent_group(skipshare);
 	if (group == NULL)
-	    return (SA_NO_SUCH_GROUP);
+		return (SA_NO_SUCH_GROUP);
 
 	handle = sa_find_group_handle(group);
 	if (handle == NULL)
-	    return (SA_SYSTEM_ERR);
+		return (SA_SYSTEM_ERR);
 
 	for (group = sa_get_group(handle, NULL); group != NULL;
 	    group = sa_get_next_group(group)) {
-	    for (share = sa_get_share(group, NULL); share != NULL;
-		share = sa_get_next_share(share)) {
-		if (share == skipshare)
-		    continue;
-		opt = sa_get_optionset(share, "nfs");
-		if (opt != NULL) {
-		    prop = sa_get_property(opt, "public");
-		    if (prop != NULL) {
-			char *shared;
-			shared = sa_get_share_attr(share, "shared");
-			if (shared != NULL) {
-			    exists = strcmp(shared, "true") == 0;
-			    sa_free_attr_string(shared);
-			    goto out;
+		for (share = sa_get_share(group, NULL); share != NULL;
+		    share = sa_get_next_share(share)) {
+			if (share == skipshare)
+				continue;
+			opt = sa_get_optionset(share, "nfs");
+			if (opt != NULL) {
+				prop = sa_get_property(opt, "public");
+				if (prop != NULL) {
+					char *shared;
+					shared = sa_get_share_attr(share,
+					    "shared");
+					if (shared != NULL) {
+						exists = strcmp(shared,
+						    "true") == 0;
+						sa_free_attr_string(shared);
+						goto out;
+					}
+				}
 			}
-		    }
 		}
-	    }
 	}
 out:
 	return (exists);
@@ -1537,7 +1588,7 @@
 	/* get the path since it is important in several places */
 	path = sa_get_share_attr(share, "path");
 	if (path == NULL)
-	    return (SA_NO_SUCH_PATH);
+		return (SA_NO_SUCH_PATH);
 
 	/*
 	 * find the optionsets and security sets.  There may not be
@@ -1548,9 +1599,9 @@
 	opt = sa_get_derived_optionset(share, "nfs", 1);
 	secoptlist = (sa_optionset_t)sa_get_all_security_types(share, "nfs", 1);
 	if (secoptlist != NULL)
-	    num_secinfo = MAX(1, count_security(secoptlist));
+		num_secinfo = MAX(1, count_security(secoptlist));
 	else
-	    num_secinfo = 1;
+		num_secinfo = 1;
 
 	/*
 	 * walk through the options and fill in the structure
@@ -1572,7 +1623,7 @@
 	sp = calloc(num_secinfo, sizeof (struct secinfo));
 
 	if (opt != NULL)
-	    err = fill_export_from_optionset(&export, opt);
+		err = fill_export_from_optionset(&export, opt);
 
 	/*
 	 * check to see if "public" is set. If it is, then make sure
@@ -1580,22 +1631,22 @@
 	 */
 
 	if (export.ex_flags & EX_PUBLIC && public_exists(share)) {
-	    (void) printf(dgettext(TEXT_DOMAIN,
-					"NFS: Cannot share more than one file "
-			    "system with 'public' property\n"));
-	    err = SA_NOT_ALLOWED;
-	    goto out;
+		(void) printf(dgettext(TEXT_DOMAIN,
+		    "NFS: Cannot share more than one file "
+		    "system with 'public' property\n"));
+		err = SA_NOT_ALLOWED;
+		goto out;
 	}
 
 	if (sp == NULL) {
 		/* failed to alloc memory */
-	    (void) printf("NFS: no memory for security\n");
-	    err = SA_NO_MEMORY;
+		(void) printf("NFS: no memory for security\n");
+		err = SA_NO_MEMORY;
 	} else {
-	    int i;
-	    export.ex_secinfo = sp;
-	    /* get default secinfo */
-	    export.ex_seccnt = num_secinfo;
+		int i;
+		export.ex_secinfo = sp;
+		/* get default secinfo */
+		export.ex_seccnt = num_secinfo;
 		/*
 		 * since we must have one security option defined, we
 		 * init to the default and then override as we find
@@ -1603,39 +1654,49 @@
 		 * where we have no defined options but we need to set
 		 * up one.
 		 */
-	    sp[0].s_window = DEF_WIN;
-	    sp[0].s_rootnames = NULL;
-	    /* setup a default in case no properties defined */
-	    if (nfs_getseconfig_default(&sp[0].s_secinfo)) {
-		(void) printf(dgettext(TEXT_DOMAIN,
-				    "NFS: nfs_getseconfig_default: failed to "
-				    "get default security mode\n"));
-		err = SA_CONFIG_ERR;
-	    }
-	    if (secoptlist != NULL) {
-		for (i = 0, prop = sa_get_property(secoptlist, NULL);
-		    prop != NULL && i < num_secinfo;
-		    prop = sa_get_next_property(prop), i++) {
-		    char *sectype;
+		sp[0].s_window = DEF_WIN;
+		sp[0].s_rootnames = NULL;
+		/* setup a default in case no properties defined */
+		if (nfs_getseconfig_default(&sp[0].s_secinfo)) {
+			(void) printf(dgettext(TEXT_DOMAIN,
+			    "NFS: nfs_getseconfig_default: failed to "
+			    "get default security mode\n"));
+			err = SA_CONFIG_ERR;
+		}
+		if (secoptlist != NULL) {
+			for (i = 0, prop = sa_get_property(secoptlist, NULL);
+			    prop != NULL && i < num_secinfo;
+			    prop = sa_get_next_property(prop), i++) {
+				char *sectype;
 
-		    sectype = sa_get_property_attr(prop, "type");
-		    /* if sectype is NULL, we can't do anything so skip */
-		    if (sectype == NULL)
-			continue;
-		    sec = (sa_security_t)sa_get_derived_security(share,
-								sectype,
-								"nfs", 1);
-		    sp[i].s_window = DEF_WIN;
-		    sp[i].s_rootcnt = 0;
-		    sp[i].s_rootnames = NULL;
+				sectype = sa_get_property_attr(prop, "type");
+				/*
+				 * if sectype is NULL, we probably
+				 * have a memory problem and can't get
+				 * the correct values. Rather than
+				 * exporting with incorrect security,
+				 * don't share it.
+				 */
+				if (sectype == NULL) {
+					err = SA_NO_MEMORY;
+					(void) printf(dgettext(TEXT_DOMAIN,
+					    "NFS: Cannot share %s: "
+					    "no memory\n"), path);
+					goto out;
+				}
+				sec = (sa_security_t)sa_get_derived_security(
+				    share, sectype, "nfs", 1);
+				sp[i].s_window = DEF_WIN;
+				sp[i].s_rootcnt = 0;
+				sp[i].s_rootnames = NULL;
 
-		    (void) fill_security_from_secopts(&sp[i], sec);
-		    if (sec != NULL)
-			sa_free_derived_security(sec);
-		    if (sectype != NULL)
-			sa_free_attr_string(sectype);
+				(void) fill_security_from_secopts(&sp[i], sec);
+				if (sec != NULL)
+					sa_free_derived_security(sec);
+				if (sectype != NULL)
+					sa_free_attr_string(sectype);
+			}
 		}
-	    }
 		/*
 		 * when we get here, we can do the exportfs system call and
 		 * initiate thinsg. We probably want to enable the nfs.server
@@ -1644,37 +1705,36 @@
 		/* check nfs.server status and start if needed */
 
 		/* now add the share to the internal tables */
-	    printarg(path, &export);
+		printarg(path, &export);
 		/*
 		 * call the exportfs system call which is implemented
 		 * via the nfssys() call as the EXPORTFS subfunction.
 		 */
-	    if ((err = exportfs(path, &export)) < 0) {
-		err = SA_SYSTEM_ERR;
-		switch (errno) {
-		case EREMOTE:
-		    (void) printf(dgettext(TEXT_DOMAIN,
-						"NFS: Cannot share remote file"
-						"system: %s\n"),
-					path);
-		    break;
-		case EPERM:
-		    if (getzoneid() != GLOBAL_ZONEID) {
-			(void) printf(dgettext(TEXT_DOMAIN,
-					"NFS: Cannot share file systems "
-					"in non-global zones: %s\n"), path);
-			err = SA_NOT_SUPPORTED;
-			break;
-		    }
-		    err = SA_NO_PERMISSION;
-		    /* FALLTHROUGH */
-		default:
-		    break;
+		if ((err = exportfs(path, &export)) < 0) {
+			err = SA_SYSTEM_ERR;
+			switch (errno) {
+			case EREMOTE:
+				(void) printf(dgettext(TEXT_DOMAIN,
+				    "NFS: Cannot share remote "
+				    "filesystem: %s\n"), path);
+				break;
+			case EPERM:
+				if (getzoneid() != GLOBAL_ZONEID) {
+					(void) printf(dgettext(TEXT_DOMAIN,
+					    "NFS: Cannot share filesystems "
+					    "in non-global zones: %s\n"), path);
+					err = SA_NOT_SUPPORTED;
+					break;
+				}
+				err = SA_NO_PERMISSION;
+				/* FALLTHROUGH */
+			default:
+				break;
+			}
+		} else {
+			/* update sharetab with an add/modify */
+			(void) sa_update_sharetab(share, "nfs");
 		}
-	    } else {
-		/* update sharetab with an add/modify */
-		(void) sa_update_sharetab(share, "nfs");
-	    }
 	}
 
 	if (err == SA_OK) {
@@ -1689,35 +1749,34 @@
 		 * error. This is probably where sharetab should be
 		 * updated with the NFS specific entry.
 		 */
-	    if (export.ex_flags & EX_LOG) {
-		/* enable logging */
-		if (nfslogtab_add(path, export.ex_log_buffer,
-		    export.ex_tag) != 0) {
-		    (void) fprintf(stderr,
-				dgettext(TEXT_DOMAIN,
-					"Could not enable logging for %s\n"),
-				path);
+		if (export.ex_flags & EX_LOG) {
+			/* enable logging */
+			if (nfslogtab_add(path, export.ex_log_buffer,
+			    export.ex_tag) != 0) {
+				(void) fprintf(stderr, dgettext(TEXT_DOMAIN,
+				    "Could not enable logging for %s\n"),
+				    path);
+			}
+			_check_services(service_list_logging);
+		} else {
+			/*
+			 * don't have logging so remove it from file. It might
+			 * not be thre, but that doesn't matter.
+			 */
+			(void) nfslogtab_deactivate(path);
+			_check_services(service_list_default);
 		}
-		_check_services(service_list_logging);
-	    } else {
-		/*
-		 * don't have logging so remove it from file. It might
-		 * not be thre, but that doesn't matter.
-		 */
-		(void) nfslogtab_deactivate(path);
-		_check_services(service_list_default);
-	    }
 	}
 
 out:
 	if (path != NULL)
-	    free(path);
+		free(path);
 
 	cleanup_export(&export);
 	if (opt != NULL)
-	    sa_free_derived_optionset(opt);
+		sa_free_derived_optionset(opt);
 	if (secoptlist != NULL)
-	    (void) sa_destroy_optionset(secoptlist);
+		(void) sa_destroy_optionset(secoptlist);
 	return (err);
 }
 
@@ -1734,31 +1793,33 @@
 	int ret = SA_OK;
 
 	if (share != NULL) {
-	    err = exportfs(share, NULL);
-	    if (err < 0) {
-		/* TBD: only an error in some cases - need better analysis */
-		switch (errno) {
-		case EPERM:
-		case EACCES:
-		    ret = SA_NO_PERMISSION;
-		    if (getzoneid() != GLOBAL_ZONEID) {
-			ret = SA_NOT_SUPPORTED;
-		    }
-		    break;
-		case EINVAL:
-		case ENOENT:
-		    ret = SA_NO_SUCH_PATH;
-		    break;
-		default:
-		    ret = SA_SYSTEM_ERR;
-		    break;
+		err = exportfs(share, NULL);
+		if (err < 0) {
+			/*
+			 * TBD: only an error in some cases - need
+			 * better analysis
+			 */
+			switch (errno) {
+			case EPERM:
+			case EACCES:
+				ret = SA_NO_PERMISSION;
+				if (getzoneid() != GLOBAL_ZONEID)
+					ret = SA_NOT_SUPPORTED;
+				break;
+			case EINVAL:
+			case ENOENT:
+				ret = SA_NO_SUCH_PATH;
+				break;
+			default:
+				ret = SA_SYSTEM_ERR;
+				break;
+			}
 		}
-	    }
-	    if (ret == SA_OK || ret == SA_NO_SUCH_PATH) {
-		(void) sa_delete_sharetab(share, "nfs");
-		/* just in case it was logged */
-		(void) nfslogtab_deactivate(share);
-	    }
+		if (ret == SA_OK || ret == SA_NO_SUCH_PATH) {
+			(void) sa_delete_sharetab(share, "nfs");
+			/* just in case it was logged */
+			(void) nfslogtab_deactivate(share);
+		}
 	}
 	return (ret);
 }
@@ -1773,7 +1834,7 @@
 {
 	int ret = SA_OK;
 	if (strcmp(v1, v2) == 0)
-	    ret = SA_VALUE_CONFLICT;
+		ret = SA_VALUE_CONFLICT;
 	return (ret);
 }
 
@@ -1797,126 +1858,143 @@
 	propname = sa_get_property_attr(property, "type");
 
 	if ((optindex = findopt(propname)) < 0)
-	    ret = SA_NO_SUCH_PROP;
+		ret = SA_NO_SUCH_PROP;
 
 	/* need to validate value range here as well */
 
 	if (ret == SA_OK) {
-	    parent_group = sa_get_parent_group((sa_share_t)parent);
-	    if (optdefs[optindex].share && !sa_is_share(parent_group)) {
-		ret = SA_PROP_SHARE_ONLY;
-	    }
+		parent_group = sa_get_parent_group((sa_share_t)parent);
+		if (optdefs[optindex].share && !sa_is_share(parent_group))
+			ret = SA_PROP_SHARE_ONLY;
 	}
 	if (ret == SA_OK) {
-	    value = sa_get_property_attr(property, "value");
-	    if (value != NULL) {
-		/* first basic type checking */
-		switch (optdefs[optindex].type) {
-		case OPT_TYPE_NUMBER:
-		    /* check that the value is all digits */
-		    if (!is_a_number(value))
-			ret = SA_BAD_VALUE;
-		    break;
-		case OPT_TYPE_BOOLEAN:
-		    if (strlen(value) == 0 ||
-			strcasecmp(value, "true") == 0 ||
-			strcmp(value, "1") == 0 ||
-			strcasecmp(value, "false") == 0 ||
-			strcmp(value, "0") == 0) {
-			ret = SA_OK;
-		    } else {
-			ret = SA_BAD_VALUE;
-		    }
-		    break;
-		case OPT_TYPE_USER:
-		    if (!is_a_number(value)) {
-			struct passwd *pw;
-			/* in this case it would have to be a user name */
-			pw = getpwnam(value);
-			if (pw == NULL)
-			    ret = SA_BAD_VALUE;
-			endpwent();
-		    } else {
-			uint64_t intval;
-			intval = strtoull(value, NULL, 0);
-			if (intval > UID_MAX && intval != ~0)
-			    ret = SA_BAD_VALUE;
-		    }
-		    break;
-		case OPT_TYPE_FILE:
-		    if (strcmp(value, "..") == 0 ||
-			strchr(value, '/') != NULL) {
-			ret = SA_BAD_VALUE;
-		    }
-		    break;
-		case OPT_TYPE_ACCLIST:
-			/*
-			 * access list handling. Should eventually
-			 * validate that all the values make sense.
-			 * Also, ro and rw may have cross value
-			 * conflicts.
-			 */
-		    if (strcmp(propname, SHOPT_RO) == 0)
-			other = SHOPT_RW;
-		    else if (strcmp(propname, SHOPT_RW) == 0)
-			other = SHOPT_RO;
-		    else
-			other = NULL;
-		    if (other != NULL && parent != NULL) {
-			/* compare rw(ro) with ro(rw) */
-			sa_property_t oprop;
-			oprop = sa_get_property(parent, other);
-			if (oprop != NULL) {
-			    /* only potential confusion if other exists */
-			    char *ovalue;
-			    ovalue = sa_get_property_attr(oprop, "value");
-			    if (ovalue != NULL) {
-				ret = check_rorw(value, ovalue);
-				sa_free_attr_string(ovalue);
-			    }
+		value = sa_get_property_attr(property, "value");
+		if (value != NULL) {
+			/* first basic type checking */
+			switch (optdefs[optindex].type) {
+			case OPT_TYPE_NUMBER:
+				/* check that the value is all digits */
+				if (!is_a_number(value))
+					ret = SA_BAD_VALUE;
+				break;
+			case OPT_TYPE_BOOLEAN:
+				if (strlen(value) == 0 ||
+				    strcasecmp(value, "true") == 0 ||
+				    strcmp(value, "1") == 0 ||
+				    strcasecmp(value, "false") == 0 ||
+				    strcmp(value, "0") == 0) {
+					ret = SA_OK;
+				} else {
+					ret = SA_BAD_VALUE;
+				}
+				break;
+			case OPT_TYPE_USER:
+				if (!is_a_number(value)) {
+					struct passwd *pw;
+					/*
+					 * in this case it would have to be a
+					 * user name
+					 */
+					pw = getpwnam(value);
+					if (pw == NULL)
+						ret = SA_BAD_VALUE;
+					endpwent();
+				} else {
+					uint64_t intval;
+					intval = strtoull(value, NULL, 0);
+					if (intval > UID_MAX && intval != ~0)
+						ret = SA_BAD_VALUE;
+				}
+				break;
+			case OPT_TYPE_FILE:
+				if (strcmp(value, "..") == 0 ||
+				    strchr(value, '/') != NULL) {
+					ret = SA_BAD_VALUE;
+				}
+				break;
+			case OPT_TYPE_ACCLIST:
+				/*
+				 * access list handling. Should eventually
+				 * validate that all the values make sense.
+				 * Also, ro and rw may have cross value
+				 * conflicts.
+				 */
+				if (strcmp(propname, SHOPT_RO) == 0)
+					other = SHOPT_RW;
+				else if (strcmp(propname, SHOPT_RW) == 0)
+					other = SHOPT_RO;
+				else
+					other = NULL;
+
+				if (other != NULL && parent != NULL) {
+					/* compare rw(ro) with ro(rw) */
+					sa_property_t oprop;
+					oprop = sa_get_property(parent, other);
+					if (oprop != NULL) {
+						/*
+						 * only potential
+						 * confusion if other
+						 * exists
+						 */
+						char *ovalue;
+						ovalue = sa_get_property_attr(
+						    oprop, "value");
+						if (ovalue != NULL) {
+							ret = check_rorw(value,
+							    ovalue);
+							sa_free_attr_string(
+							    ovalue);
+						}
+					}
+				}
+				break;
+			case OPT_TYPE_LOGTAG:
+				if (nfsl_getconfig_list(&configlist) == 0) {
+					int error;
+					if (value == NULL ||
+					    strlen(value) == 0) {
+						if (value != NULL)
+							sa_free_attr_string(
+							    value);
+						value = strdup("global");
+					}
+					if (value != NULL &&
+					    nfsl_findconfig(configlist, value,
+					    &error) == NULL) {
+						ret = SA_BAD_VALUE;
+					} else {
+						nfsl_freeconfig_list(
+						    &configlist);
+					}
+				} else {
+					ret = SA_CONFIG_ERR;
+				}
+				break;
+			case OPT_TYPE_STRING:
+				/* whatever is here should be ok */
+				break;
+			case OPT_TYPE_SECURITY:
+				/*
+				 * The "sec" property isn't used in the
+				 * non-legacy parts of sharemgr. We need to
+				 * reject it here. For legacy, it is pulled
+				 * out well before we get here.
+				 */
+				ret = SA_NO_SUCH_PROP;
+				break;
+			default:
+				break;
 			}
-		    }
-		    break;
-		case OPT_TYPE_LOGTAG:
-		    if (nfsl_getconfig_list(&configlist) == 0) {
-			int error;
-			if (value == NULL || strlen(value) == 0) {
-			    if (value != NULL)
-				sa_free_attr_string(value);
-			    value = strdup("global");
+			sa_free_attr_string(value);
+			if (ret == SA_OK && optdefs[optindex].check != NULL) {
+				/* do the property specific check */
+				ret = optdefs[optindex].check(property);
 			}
-			if (nfsl_findconfig(configlist, value, &error) == NULL)
-			    ret = SA_BAD_VALUE;
-			nfsl_freeconfig_list(&configlist);
-		    } else {
-			ret = SA_CONFIG_ERR;
-		    }
-		    break;
-		case OPT_TYPE_STRING:
-		    /* whatever is here should be ok */
-		    break;
-		case OPT_TYPE_SECURITY:
-			/*
-			 * The "sec" property isn't used in the
-			 * non-legacy parts of sharemgr. We need to
-			 * reject it here. For legacy, it is pulled
-			 * out well before we get here.
-			 */
-		    ret = SA_NO_SUCH_PROP;
-		    break;
-		default:
-		    break;
 		}
-		sa_free_attr_string(value);
-		if (ret == SA_OK && optdefs[optindex].check != NULL) {
-		    /* do the property specific check */
-		    ret = optdefs[optindex].check(property);
-		}
-	    }
 	}
 
 	if (propname != NULL)
-	    sa_free_attr_string(propname);
+		sa_free_attr_string(propname);
 	return (ret);
 }
 
@@ -2027,13 +2105,13 @@
 {
 	int i;
 	for (i = 0; proto_options[i].tag != NULL; i++) {
-	    if (whichname == 1) {
-		if (strcasecmp(proto_options[i].name, name) == 0)
+		if (whichname == 1) {
+			if (strcasecmp(proto_options[i].name, name) == 0)
 			return (i);
-	    } else {
-		if (strcasecmp(proto_options[i].tag, name) == 0)
-			return (i);
-	    }
+		} else {
+			if (strcasecmp(proto_options[i].tag, name) == 0)
+				return (i);
+		}
 	}
 	return (-1);
 }
@@ -2048,8 +2126,8 @@
 fixcaselower(char *str)
 {
 	while (*str) {
-	    *str = tolower(*str);
-	    str++;
+		*str = tolower(*str);
+		str++;
 	}
 }
 
@@ -2063,8 +2141,8 @@
 fixcaseupper(char *str)
 {
 	while (*str) {
-	    *str = toupper(*str);
-	    str++;
+		*str = toupper(*str);
+		str++;
 	}
 }
 
@@ -2085,6 +2163,32 @@
 }
 
 /*
+ * extractprop()
+ *
+ * Extract the property and value out of the line and create the
+ * property in the optionset.
+ */
+static void
+extractprop(char *name, char *value)
+{
+	sa_property_t prop;
+	int index;
+	/*
+	 * Remove any leading
+	 * white space.
+	 */
+	name = skipwhitespace(name);
+
+	index = findprotoopt(name, 0);
+	if (index >= 0) {
+		fixcaselower(name);
+		prop = sa_create_property(proto_options[index].name, value);
+		if (prop != NULL)
+			(void) sa_add_protocol_property(protoset, prop);
+	}
+}
+
+/*
  * initprotofromdefault()
  *
  * read the default file(s) and add the defined values to the
@@ -2099,50 +2203,39 @@
 	char buff[BUFSIZ];
 	char *name;
 	char *value;
-	sa_property_t prop;
-	int index;
 
 	protoset = sa_create_protocol_properties("nfs");
 
 	if (protoset != NULL) {
-	    nfs = fopen(NFSADMIN, "r");
-	    if (nfs != NULL) {
-		while (fgets(buff, sizeof (buff), nfs) != NULL) {
-		    switch (buff[0]) {
-		    case '\n':
-		    case '#':
-			/* skip */
-			break;
-		    default:
-			name = buff;
-			buff[strlen(buff) - 1] = '\0';
-			value = strchr(name, '=');
-			if (value != NULL) {
-			    *value++ = '\0';
-			    /* Remove any leading white space. */
-			    name = skipwhitespace(name);
-
-			    if ((index = findprotoopt(name, 0)) >= 0) {
-				fixcaselower(name);
-				prop = sa_create_property(
-					proto_options[index].name,
-					value);
-				(void) sa_add_protocol_property(protoset, prop);
-			    }
+		nfs = fopen(NFSADMIN, "r");
+		if (nfs != NULL) {
+			while (fgets(buff, sizeof (buff), nfs) != NULL) {
+				switch (buff[0]) {
+				case '\n':
+				case '#':
+					/* skip */
+					break;
+				default:
+					name = buff;
+					buff[strlen(buff) - 1] = '\0';
+					value = strchr(name, '=');
+					if (value != NULL) {
+						*value++ = '\0';
+						extractprop(name, value);
+					}
+				}
 			}
-		    }
+			if (nfs != NULL)
+				(void) fclose(nfs);
 		}
-		if (nfs != NULL)
-		    (void) fclose(nfs);
-	    }
 	}
 	if (protoset == NULL)
-	    return (SA_NO_MEMORY);
+		return (SA_NO_MEMORY);
 	return (SA_OK);
 }
 
 /*
- * add_default()
+ * add_defaults()
  *
  * Add the default values for any property not defined in the parsing
  * of the default files. Values are set according to their defined
@@ -2156,37 +2249,40 @@
 	char number[MAXDIGITS];
 
 	for (i = 0; proto_options[i].tag != NULL; i++) {
-	    sa_property_t prop;
-	    prop = sa_get_protocol_property(protoset, proto_options[i].name);
-	    if (prop == NULL) {
-		/* add the default value */
-		switch (proto_options[i].type) {
-		case OPT_TYPE_NUMBER:
-		    (void) snprintf(number, sizeof (number), "%d",
-				proto_options[i].defvalue.intval);
-		    prop = sa_create_property(proto_options[i].name, number);
-		    break;
+		sa_property_t prop;
+		prop = sa_get_protocol_property(protoset,
+		    proto_options[i].name);
+		if (prop == NULL) {
+			/* add the default value */
+			switch (proto_options[i].type) {
+			case OPT_TYPE_NUMBER:
+				(void) snprintf(number, sizeof (number), "%d",
+				    proto_options[i].defvalue.intval);
+				prop = sa_create_property(proto_options[i].name,
+				    number);
+				break;
 
-		case OPT_TYPE_BOOLEAN:
-		    prop = sa_create_property(proto_options[i].name,
-					proto_options[i].defvalue.intval ?
-					"true" : "false");
-		    break;
+			case OPT_TYPE_BOOLEAN:
+				prop = sa_create_property(proto_options[i].name,
+				    proto_options[i].defvalue.intval ?
+				    "true" : "false");
+				break;
 
-		case OPT_TYPE_ONOFF:
-		    prop = sa_create_property(proto_options[i].name,
-					proto_options[i].defvalue.intval ?
-					"on" : "off");
-		    break;
+			case OPT_TYPE_ONOFF:
+				prop = sa_create_property(proto_options[i].name,
+				    proto_options[i].defvalue.intval ?
+				    "on" : "off");
+				break;
 
-		default:
-		    /* treat as strings of zero length */
-		    prop = sa_create_property(proto_options[i].name, "");
-		    break;
+			default:
+				/* treat as strings of zero length */
+				prop = sa_create_property(proto_options[i].name,
+				    "");
+				break;
+			}
+			if (prop != NULL)
+				(void) sa_add_protocol_property(protoset, prop);
 		}
-		if (prop != NULL)
-		    (void) sa_add_protocol_property(protoset, prop);
-	    }
 	}
 }
 
@@ -2208,11 +2304,12 @@
 	int ret = SA_OK;
 
 	if (sa_plugin_ops.sa_init != nfs_init)
-	    (void) printf(dgettext(TEXT_DOMAIN,
-				    "NFS plugin not properly initialized\n"));
+		(void) printf(dgettext(TEXT_DOMAIN,
+		    "NFS plugin not properly initialized\n"));
 
 	ret = initprotofromdefault();
-	add_defaults();
+	if (ret == SA_OK)
+		add_defaults();
 
 	return (ret);
 }
@@ -2265,19 +2362,20 @@
 
 	file = fopen(fname, "r");
 	if (file != NULL) {
-	    while (fgets(buff, sizeof (buff), file) != NULL) {
-		newdef = (struct deffile *)calloc(1, sizeof (struct deffile));
-		if (newdef != NULL) {
-		    /* Make sure we skip any leading whitespace. */
-		    newdef->line = strdup(skipwhitespace(buff));
-		    if (defs == NULL) {
-			prevdef = defs = newdef;
-		    } else {
-			prevdef->next = newdef;
-			prevdef = newdef;
-		    }
+		while (fgets(buff, sizeof (buff), file) != NULL) {
+			newdef = (struct deffile *)calloc(1,
+			    sizeof (struct deffile));
+			if (newdef != NULL) {
+				/* Make sure we skip any leading whitespace. */
+				newdef->line = strdup(skipwhitespace(buff));
+				if (defs == NULL) {
+					prevdef = defs = newdef;
+				} else {
+					prevdef->next = newdef;
+					prevdef = newdef;
+				}
+			}
 		}
-	    }
 	}
 	(void) fclose(file);
 	return (defs);
@@ -2289,11 +2387,11 @@
 	struct deffile *curdefs = NULL;
 
 	while (defs != NULL) {
-	    curdefs = defs;
-	    defs = defs->next;
-	    if (curdefs->line != NULL)
-		free(curdefs->line);
-	    free(curdefs);
+		curdefs = defs;
+		defs = defs->next;
+		if (curdefs->line != NULL)
+			free(curdefs->line);
+		free(curdefs);
 	}
 }
 
@@ -2312,28 +2410,28 @@
 
 	file = fopen(fname, "w+");
 	if (file != NULL) {
-	    (void) sigprocmask(SIG_BLOCK, NULL, &new);
-	    (void) sigaddset(&new, SIGHUP);
-	    (void) sigaddset(&new, SIGINT);
-	    (void) sigaddset(&new, SIGQUIT);
-	    (void) sigaddset(&new, SIGTSTP);
-	    (void) sigprocmask(SIG_SETMASK, &new, &old);
-	    while (defs != NULL) {
-		(void) fputs(defs->line, file);
-		defs = defs->next;
-	    }
-	    (void) fsync(fileno(file));
-	    (void) sigprocmask(SIG_SETMASK, &old, NULL);
-	    (void) fclose(file);
+		(void) sigprocmask(SIG_BLOCK, NULL, &new);
+		(void) sigaddset(&new, SIGHUP);
+		(void) sigaddset(&new, SIGINT);
+		(void) sigaddset(&new, SIGQUIT);
+		(void) sigaddset(&new, SIGTSTP);
+		(void) sigprocmask(SIG_SETMASK, &new, &old);
+		while (defs != NULL) {
+			(void) fputs(defs->line, file);
+			defs = defs->next;
+		}
+		(void) fsync(fileno(file));
+		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		(void) fclose(file);
 	} else {
-	    switch (errno) {
-	    case EPERM:
-	    case EACCES:
-		ret = SA_NO_PERMISSION;
-		break;
-	    default:
-		ret = SA_SYSTEM_ERR;
-	    }
+		switch (errno) {
+		case EPERM:
+		case EACCES:
+			ret = SA_NO_PERMISSION;
+			break;
+		default:
+			ret = SA_SYSTEM_ERR;
+		}
 	}
 	return (ret);
 }
@@ -2363,64 +2461,64 @@
 
 	root = defs = read_default_file(NFSADMIN);
 	if (root == NULL) {
-	    if (errno == EPERM || errno == EACCES)
-		ret = SA_NO_PERMISSION;
-	    else
-		ret = SA_SYSTEM_ERR;
+		if (errno == EPERM || errno == EACCES)
+			ret = SA_NO_PERMISSION;
+		else
+			ret = SA_SYSTEM_ERR;
 	} else {
-	    while (defs != NULL) {
-		if (defs->line != NULL &&
-		    strncasecmp(defs->line, string, len) == 0) {
-		    /* replace with the new value */
-		    free(defs->line);
-		    fixcaseupper(tag);
-		    (void) snprintf(string, sizeof (string), "%s=%s\n",
-					tag, value);
-		    string[MAX_STRING_LENGTH - 1] = '\0';
-		    defs->line = strdup(string);
-		    update = 1;
-		    break;
-		}
-		defs = defs->next;
-	    }
-	    if (!update) {
-		defs = root;
-		/* didn't find, so see if it is a comment */
-		(void) snprintf(string, MAX_STRING_LENGTH, "#%s=", tag);
-		len = strlen(string);
 		while (defs != NULL) {
-		    if (strncasecmp(defs->line, string, len) == 0) {
-			/* replace with the new value */
-			free(defs->line);
-			fixcaseupper(tag);
-			(void) snprintf(string, sizeof (string),
+			if (defs->line != NULL &&
+			    strncasecmp(defs->line, string, len) == 0) {
+				/* replace with the new value */
+				free(defs->line);
+				fixcaseupper(tag);
+				(void) snprintf(string, sizeof (string),
 				    "%s=%s\n", tag, value);
-			string[MAX_STRING_LENGTH - 1] = '\0';
-			defs->line = strdup(string);
-			update = 1;
-			break;
-		    }
-		    defs = defs->next;
+				string[MAX_STRING_LENGTH - 1] = '\0';
+				defs->line = strdup(string);
+				update = 1;
+				break;
+			}
+			defs = defs->next;
 		}
-	    }
-	    if (!update) {
-		fixcaseupper(tag);
-		(void) snprintf(string, sizeof (string), "%s=%s\n",
-					tag, value);
-		prev = root;
-		while (prev->next != NULL)
-		    prev = prev->next;
-		defs = malloc(sizeof (struct deffile));
-		prev->next = defs;
-		if (defs != NULL) {
-		    defs->next = NULL;
-		    defs->line = strdup(string);
+		if (!update) {
+			defs = root;
+			/* didn't find, so see if it is a comment */
+			(void) snprintf(string, MAX_STRING_LENGTH, "#%s=", tag);
+			len = strlen(string);
+			while (defs != NULL) {
+				if (strncasecmp(defs->line, string, len) == 0) {
+					/* replace with the new value */
+					free(defs->line);
+					fixcaseupper(tag);
+					(void) snprintf(string, sizeof (string),
+					    "%s=%s\n", tag, value);
+					string[MAX_STRING_LENGTH - 1] = '\0';
+					defs->line = strdup(string);
+					update = 1;
+					break;
+				}
+				defs = defs->next;
+			}
 		}
-	    }
-	    if (update) {
-		ret = write_default_file(NFSADMIN, root);
-	    }
-	    free_default_file(root);
+		if (!update) {
+			fixcaseupper(tag);
+			(void) snprintf(string, sizeof (string), "%s=%s\n",
+			    tag, value);
+			prev = root;
+			while (prev->next != NULL)
+				prev = prev->next;
+			defs = malloc(sizeof (struct deffile));
+			prev->next = defs;
+			if (defs != NULL) {
+				defs->next = NULL;
+				defs->line = strdup(string);
+			}
+		}
+		if (update) {
+			ret = write_default_file(NFSADMIN, root);
+		}
+		free_default_file(root);
 	}
 	return (ret);
 }
@@ -2440,9 +2538,9 @@
 
 	state = smf_get_state(service);
 	if (state != NULL) {
-	    /* got the state so get the equality for the return value */
-	    ret = strcmp(state, chkstate) == 0 ? B_TRUE : B_FALSE;
-	    free(state);
+		/* got the state so get the equality for the return value */
+		ret = strcmp(state, chkstate) == 0 ? B_TRUE : B_FALSE;
+		free(state);
 	}
 	return (ret);
 }
@@ -2464,34 +2562,34 @@
 	char *service;
 
 	for (mask = 1; svcs != 0; mask <<= 1) {
-	    switch (svcs & mask) {
-	    case SVC_LOCKD:
-		service = LOCKD;
-		break;
-	    case SVC_STATD:
-		service = STATD;
-		break;
-	    case SVC_NFSD:
-		service = NFSD;
-		break;
-	    case SVC_MOUNTD:
-		service = MOUNTD;
-		break;
-	    case SVC_NFS4CBD:
-		service = NFS4CBD;
-		break;
-	    case SVC_NFSMAPID:
-		service = NFSMAPID;
-		break;
-	    case SVC_RQUOTAD:
-		service = RQUOTAD;
-		break;
-	    case SVC_NFSLOGD:
-		service = NFSLOGD;
-		break;
-	    default:
-		continue;
-	    }
+		switch (svcs & mask) {
+		case SVC_LOCKD:
+			service = LOCKD;
+			break;
+		case SVC_STATD:
+			service = STATD;
+			break;
+		case SVC_NFSD:
+			service = NFSD;
+			break;
+		case SVC_MOUNTD:
+			service = MOUNTD;
+			break;
+		case SVC_NFS4CBD:
+			service = NFS4CBD;
+			break;
+		case SVC_NFSMAPID:
+			service = NFSMAPID;
+			break;
+		case SVC_RQUOTAD:
+			service = RQUOTAD;
+			break;
+		case SVC_NFSLOGD:
+			service = NFSLOGD;
+			break;
+		default:
+			continue;
+		}
 
 		/*
 		 * Only attempt to restart the service if it is
@@ -2499,34 +2597,35 @@
 		 * desirable to use smf_refresh_instance if the NFS
 		 * services ever implement the refresh method.
 		 */
-	    if (service_in_state(service, SCF_STATE_STRING_ONLINE)) {
-		ret = smf_restart_instance(service);
-		/*
-		 * There are only a few SMF errors at this point, but
-		 * it is also possible that a bad value may have put
-		 * the service into maintenance if there wasn't an
-		 * SMF level error.
-		 */
-		if (ret != 0) {
-		    (void) fprintf(stderr,
-				dgettext(TEXT_DOMAIN,
-					"%s failed to restart: %s\n"),
-				scf_strerror(scf_error()));
-		} else {
+		if (service_in_state(service, SCF_STATE_STRING_ONLINE)) {
+			ret = smf_restart_instance(service);
 			/*
-			 * Check whether it has gone to "maintenance"
-			 * mode or not. Maintenance implies something
-			 * went wrong.
+			 * There are only a few SMF errors at this point, but
+			 * it is also possible that a bad value may have put
+			 * the service into maintenance if there wasn't an
+			 * SMF level error.
 			 */
-		    if (service_in_state(service, SCF_STATE_STRING_MAINT)) {
-			(void) fprintf(stderr,
-					dgettext(TEXT_DOMAIN,
-						"%s failed to restart\n"),
-					service);
-		    }
+			if (ret != 0) {
+				(void) fprintf(stderr,
+				    dgettext(TEXT_DOMAIN,
+				    "%s failed to restart: %s\n"),
+				    scf_strerror(scf_error()));
+			} else {
+				/*
+				 * Check whether it has gone to "maintenance"
+				 * mode or not. Maintenance implies something
+				 * went wrong.
+				 */
+				if (service_in_state(service,
+				    SCF_STATE_STRING_MAINT)) {
+					(void) fprintf(stderr,
+					    dgettext(TEXT_DOMAIN,
+					    "%s failed to restart\n"),
+					    service);
+				}
+			}
 		}
-	    }
-	    svcs &= ~mask;
+		svcs &= ~mask;
 	}
 }
 
@@ -2549,25 +2648,27 @@
 	int ret = B_TRUE;
 
 	if (proto_options[index].other != NULL) {
-	    /* have a property to compare against */
-	    opts = nfs_get_proto_set();
-	    prop = sa_get_property(opts, proto_options[index].other);
+		/* have a property to compare against */
+		opts = nfs_get_proto_set();
+		prop = sa_get_property(opts, proto_options[index].other);
 		/*
 		 * If we don't find the property, assume default
 		 * values which will work since the max will be at the
 		 * max and the min at the min.
 		 */
-	    if (prop != NULL) {
-		pval = sa_get_property_attr(prop, "value");
-		if (pval != NULL) {
-		    val = strtoul(pval, NULL, 0);
-		    if (proto_options[index].compare == OPT_CMP_LE) {
-			ret = value <= val ? B_TRUE : B_FALSE;
-		    } else if (proto_options[index].compare == OPT_CMP_GE) {
-			ret = value >= val ? B_TRUE : B_FALSE;
-		    }
+		if (prop != NULL) {
+			pval = sa_get_property_attr(prop, "value");
+			if (pval != NULL) {
+				val = strtoul(pval, NULL, 0);
+				if (proto_options[index].compare ==
+				    OPT_CMP_LE) {
+					ret = value <= val ? B_TRUE : B_FALSE;
+				} else if (proto_options[index].compare ==
+				    OPT_CMP_GE) {
+					ret = value >= val ? B_TRUE : B_FALSE;
+				}
+			}
 		}
-	    }
 	}
 	return (ret);
 }
@@ -2598,25 +2699,25 @@
 
 	switch (proto_options[index].type) {
 	case OPT_TYPE_NUMBER:
-	    if (!is_a_number(value))
-		ret = SA_BAD_VALUE;
-	    else {
-		int val;
-		val = strtoul(value, NULL, 0);
-		if (val < proto_options[index].minval ||
-		    val > proto_options[index].maxval)
-		    ret = SA_BAD_VALUE;
-		/*
-		 * For server_versmin/server_versmax and
-		 * client_versmin/client_versmax, the value of the
-		 * min(max) should be checked to be correct relative
-		 * to the current max(min).
-		 */
-		if (!nfs_minmax_check(index, val)) {
-		    ret = SA_BAD_VALUE;
+		if (!is_a_number(value))
+			ret = SA_BAD_VALUE;
+		else {
+			int val;
+			val = strtoul(value, NULL, 0);
+			if (val < proto_options[index].minval ||
+			    val > proto_options[index].maxval)
+				ret = SA_BAD_VALUE;
+			/*
+			 * For server_versmin/server_versmax and
+			 * client_versmin/client_versmax, the value of the
+			 * min(max) should be checked to be correct relative
+			 * to the current max(min).
+			 */
+			if (!nfs_minmax_check(index, val)) {
+				ret = SA_BAD_VALUE;
+			}
 		}
-	    }
-	    break;
+		break;
 
 	case OPT_TYPE_DOMAIN:
 		/*
@@ -2625,42 +2726,42 @@
 		 * side of it.  A zero length string is also allowed
 		 * and is the way to turn off the override.
 		 */
-	    if (strlen(value) == 0)
+		if (strlen(value) == 0)
+			break;
+		cp = strchr(value, '.');
+		if (cp == NULL || cp == value || strchr(value, '@') != NULL)
+			ret = SA_BAD_VALUE;
 		break;
-	    cp = strchr(value, '.');
-	    if (cp == NULL || cp == value || strchr(value, '@') != NULL)
-		ret = SA_BAD_VALUE;
-	    break;
 
 	case OPT_TYPE_BOOLEAN:
-	    if (strlen(value) == 0 ||
-		strcasecmp(value, "true") == 0 ||
-		strcmp(value, "1") == 0 ||
-		strcasecmp(value, "false") == 0 ||
-		strcmp(value, "0") == 0) {
-		ret = SA_OK;
-	    } else {
-		ret = SA_BAD_VALUE;
-	    }
-	    break;
+		if (strlen(value) == 0 ||
+		    strcasecmp(value, "true") == 0 ||
+		    strcmp(value, "1") == 0 ||
+		    strcasecmp(value, "false") == 0 ||
+		    strcmp(value, "0") == 0) {
+			ret = SA_OK;
+		} else {
+			ret = SA_BAD_VALUE;
+		}
+		break;
 
 	case OPT_TYPE_ONOFF:
-	    if (strcasecmp(value, "on") != 0 &&
-		strcasecmp(value, "off") != 0) {
-		ret = SA_BAD_VALUE;
-	    }
-	    break;
+		if (strcasecmp(value, "on") != 0 &&
+		    strcasecmp(value, "off") != 0) {
+			ret = SA_BAD_VALUE;
+		}
+		break;
 
 	case OPT_TYPE_PROTOCOL:
-	    if (strcasecmp(value, "all") != 0 &&
-		strcasecmp(value, "tcp") != 0 &&
-		strcasecmp(value, "udp") != 0)
-		ret = SA_BAD_VALUE;
-	    break;
+		if (strcasecmp(value, "all") != 0 &&
+		    strcasecmp(value, "tcp") != 0 &&
+		    strcasecmp(value, "udp") != 0)
+			ret = SA_BAD_VALUE;
+		break;
 
 	default:
-	    /* treat as a string */
-	    break;
+		/* treat as a string */
+		break;
 	}
 	return (ret);
 }
@@ -2681,21 +2782,21 @@
 	name = sa_get_property_attr(prop, "type");
 	value = sa_get_property_attr(prop, "value");
 	if (name != NULL && value != NULL) {
-	    int index = findprotoopt(name, 1);
-	    if (index >= 0) {
-		/* should test for valid value */
-		ret = nfs_validate_proto_prop(index, name, value);
-		if (ret == SA_OK)
-		    ret = set_default_file_value(proto_options[index].tag,
-							value);
-		if (ret == SA_OK)
-		    restart_service(proto_options[index].svcs);
-	    }
+		int index = findprotoopt(name, 1);
+		if (index >= 0) {
+			/* should test for valid value */
+			ret = nfs_validate_proto_prop(index, name, value);
+			if (ret == SA_OK)
+				ret = set_default_file_value(
+				    proto_options[index].tag, value);
+			if (ret == SA_OK)
+				restart_service(proto_options[index].svcs);
+		}
 	}
 	if (name != NULL)
-	    sa_free_attr_string(name);
+		sa_free_attr_string(name);
 	if (value != NULL)
-	    sa_free_attr_string(value);
+		sa_free_attr_string(value);
 	return (ret);
 }
 
@@ -2735,8 +2836,8 @@
 	 */
 	if (strcmp(space, "default") == 0 &&
 	    nfs_getseconfig_default(&secconf) == 0) {
-	    if (nfs_getseconfig_bynumber(secconf.sc_nfsnum, &secconf) == 0)
-		name = secconf.sc_name;
+		if (nfs_getseconfig_bynumber(secconf.sc_nfsnum, &secconf) == 0)
+			name = secconf.sc_name;
 	}
 	return (strdup(name));
 }