6549814 libshare has no sense of style

3 files changed, 1680 insertions, 1607 deletions

diff --git a/usr/src/lib/libshare/common/libsharecore.c b/usr/src/lib/libshare/common/libsharecore.c
index cbef53feab..ba9d7f4b97 100644
--- a/usr/src/lib/libshare/common/libsharecore.c
+++ b/usr/src/lib/libshare/common/libsharecore.c
@@ -107,12 +107,12 @@ sablocksigs(sigset_t *sigs)
 	sigset_t new;
 
 	if (sigs != 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, sigs);
+		(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, sigs);
 	}
 }
 
@@ -125,7 +125,7 @@ void
 saunblocksigs(sigset_t *sigs)
 {
 	if (sigs != NULL)
-	    (void) sigprocmask(SIG_SETMASK, sigs, NULL);
+		(void) sigprocmask(SIG_SETMASK, sigs, NULL);
 }
 
 /*
@@ -141,7 +141,7 @@ alloc_sharelist()
 
 	item = (xfs_sharelist_t *)malloc(sizeof (xfs_sharelist_t));
 	if (item != NULL)
-	    (void) memset(item, '\0', sizeof (xfs_sharelist_t));
+		(void) memset(item, '\0', sizeof (xfs_sharelist_t));
 	return (item);
 }
 
@@ -159,29 +159,29 @@ fix_notice(xfs_sharelist_t *list)
 	int i;
 
 	if (list == NULL) {
-	    /* zero length dfstab */
-	    list = alloc_sharelist();
-	    if (list == NULL)
-		return (NULL);
-	    list->description = strdup("#\n");
+		/* zero length dfstab */
+		list = alloc_sharelist();
+		if (list == NULL)
+			return (NULL);
+		list->description = strdup("#\n");
 	}
 	if (list->path == NULL && list->description != NULL &&
 	    strcmp(list->description, notice[0]) != 0) {
-	    for (prev = NULL, i = 0; i < DFSTAB_NOTICE_LINES; i++) {
-		item = alloc_sharelist();
-		if (item != NULL) {
-		    item->description = strdup(notice[i]);
-		    if (prev == NULL) {
-			item->next = list;
-			prev = item;
-			list = item;
-		    } else {
-			item->next = prev->next;
-			prev->next = item;
-			prev = item;
-		    }
+		for (prev = NULL, i = 0; i < DFSTAB_NOTICE_LINES; i++) {
+			item = alloc_sharelist();
+			if (item != NULL) {
+				item->description = strdup(notice[i]);
+				if (prev == NULL) {
+					item->next = list;
+					prev = item;
+					list = item;
+				} else {
+					item->next = prev->next;
+					prev->next = item;
+					prev = item;
+				}
+			}
 		}
-	    }
 	}
 	return (list);
 }
@@ -208,94 +208,94 @@ getdfstab(FILE *dfs)
 	xfs_sharelist_t *item = NULL, *first = NULL, *last;
 
 	if (dfs != NULL) {
-	    first = NULL;
-	    line = 0;
-	    while (fgets(buff, sizeof (buff), dfs) != NULL) {
-		line++;
-		bp = buff;
-		if (buff[0] == '#') {
-		    item = alloc_sharelist();
-		    if (item != NULL) {
-			/* if no path, then comment */
-			item->lineno = line;
-			item->description = strdup(buff);
-			if (first == NULL) {
-			    first = item;
-			    last = item;
+		first = NULL;
+		line = 0;
+		while (fgets(buff, sizeof (buff), dfs) != NULL) {
+			line++;
+			bp = buff;
+			if (buff[0] == '#') {
+				item = alloc_sharelist();
+				if (item != NULL) {
+					/* if no path, then comment */
+					item->lineno = line;
+					item->description = strdup(buff);
+					if (first == NULL) {
+						first = item;
+						last = item;
+					} else {
+						last->next = item;
+						last = item;
+					}
+				} else {
+					break;
+				}
+				continue;
+			} else if (buff[0] == '\n') {
+				continue;
+			}
+			optind = 1;
+			item = alloc_sharelist();
+			if (item == NULL) {
+				break;
+			} else if (first == NULL) {
+				first = item;
+				last = item;
 			} else {
-			    last->next = item;
-			    last = item;
+				last->next = item;
+				last = item;
 			}
-		    } else {
-			break;
-		    }
-		    continue;
-		} else if (buff[0] == '\n') {
-		    continue;
-		}
-		optind = 1;
-		item = alloc_sharelist();
-		if (item == NULL) {
-		    break;
-		} else if (first == NULL) {
-		    first = item;
-		    last = item;
-		} else {
-		    last->next = item;
-		    last = item;
-		}
-		item->lineno = line;
-		item->origline = strdup(buff);
-		(void) get_token(NULL); /* reset to new pointers */
-		argc = 0;
-		while ((token = get_token(bp)) != NULL) {
-		    if (argc < MAXARGSFORSHARE) {
-			args[argc++] = token;
-		    }
-		}
-		while ((c = getopt(argc, args, "F:o:d:pg:")) != -1) {
-		    switch (c) {
-		    case 'p':
-			item->persist = 1;
-			break;
-		    case 'F':
-			item->fstype = strdup(optarg);
-			break;
-		    case 'o':
-			item->options = strdup(optarg);
-			break;
-		    case 'd':
-			item->description = strdup(optarg);
-			break;
-		    case 'g':
-			item->group = strdup(optarg);
-			break;
-		    default:
-			break;
-		    }
-		}
-		if (optind < argc) {
-		    item->path = strdup(args[optind]);
-		    optind++;
-		    if (optind < argc) {
-			char *resource;
-			char *optgroup;
-			/* resource and/or groupname */
-			resource = args[optind];
-			optgroup = strchr(resource, '@');
-			if (optgroup != NULL) {
-			    *optgroup++ = '\0';
+			item->lineno = line;
+			item->origline = strdup(buff);
+			(void) get_token(NULL); /* reset to new pointers */
+			argc = 0;
+			while ((token = get_token(bp)) != NULL) {
+				if (argc < MAXARGSFORSHARE)
+					args[argc++] = token;
 			}
-			if (optgroup != NULL)
-			    item->group = strdup(optgroup);
-			if (resource != NULL && strlen(resource) > 0)
-			    item->resource = strdup(resource);
-		    }
-		}
-		if (item != NULL && item->fstype == NULL) {
-		    item->fstype = strdup("nfs"); /* this is the default */
+			while ((c = getopt(argc, args, "F:o:d:pg:")) != -1) {
+				switch (c) {
+				case 'p':
+					item->persist = 1;
+					break;
+				case 'F':
+					item->fstype = strdup(optarg);
+					break;
+				case 'o':
+					item->options = strdup(optarg);
+					break;
+				case 'd':
+					item->description = strdup(optarg);
+					break;
+				case 'g':
+					item->group = strdup(optarg);
+					break;
+				default:
+					break;
+				}
+			}
+			if (optind < argc) {
+				item->path = strdup(args[optind]);
+				optind++;
+				if (optind < argc) {
+					char *resource;
+					char *optgroup;
+					/* resource and/or groupname */
+					resource = args[optind];
+					optgroup = strchr(resource, '@');
+					if (optgroup != NULL)
+						*optgroup++ = '\0';
+					if (optgroup != NULL)
+						item->group = strdup(optgroup);
+					if (resource != NULL &&
+					    strlen(resource) > 0)
+						item->resource =
+						    strdup(resource);
+				}
+			}
+			/* NFS is the default if none defined */
+			if (item != NULL && item->fstype == NULL)
+				item->fstype = strdup("nfs");
 		}
-	    }
 	}
 	first = fix_notice(first);
 	return (first);
@@ -314,7 +314,7 @@ finddfsentry(xfs_sharelist_t *list, char *path)
 	xfs_sharelist_t *item;
 
 	for (item = list; item != NULL; item = item->next) {
-	    if (item->path != NULL && strcmp(item->path, path) == 0)
+		if (item->path != NULL && strcmp(item->path, path) == 0)
 		return (item);
 	}
 	return (NULL);
@@ -335,27 +335,26 @@ remdfsentry(xfs_sharelist_t *list, char *path, char *proto)
 
 	for (item = prev = list; item != NULL; item = item->next) {
 	    /* skip comment entry but don't lose it */
-	    if (item->path == NULL) {
+		if (item->path == NULL) {
+			prev = item;
+			continue;
+		}
+		/* if proto is NULL, remove all protocols */
+		if (proto == NULL || (strcmp(item->path, path) == 0 &&
+		    (item->fstype != NULL && strcmp(item->fstype, proto) == 0)))
+			break;
+		if (item->fstype == NULL &&
+		    (proto == NULL || strcmp(proto, "nfs") == 0))
+			break;
 		prev = item;
-		continue;
-	    }
-	    /* if proto is NULL, remove all protocols */
-	    if (proto == NULL || (strcmp(item->path, path) == 0 &&
-		(item->fstype != NULL && strcmp(item->fstype, proto) == 0)))
-		break;
-	    if (item->fstype == NULL &&
-		(proto == NULL || strcmp(proto, "nfs") == 0))
-		break;
-	    prev = item;
 	}
 	if (item != NULL) {
-	    if (item == prev) {
-		list = item->next; /* this must be the first one */
-	    } else {
-		prev->next = item->next;
-	    }
-	    item->next = NULL;
-	    dfs_free_list(item);
+		if (item == prev)
+			list = item->next; /* this must be the first one */
+		else
+			prev->next = item->next;
+		item->next = NULL;
+		dfs_free_list(item);
 	}
 	return (list);
 }
@@ -372,24 +371,22 @@ remdfsline(xfs_sharelist_t *list, char *line)
 	xfs_sharelist_t *item, *prev = NULL;
 
 	for (item = prev = list; item != NULL; item = item->next) {
-	    /* skip comment entry but don't lose it */
-	    if (item->path == NULL) {
+		/* skip comment entry but don't lose it */
+		if (item->path == NULL) {
 		prev = item;
 		continue;
-	    }
-	    if (strcmp(item->origline, line) == 0) {
-		break;
-	    }
-	    prev = item;
+		}
+		if (strcmp(item->origline, line) == 0)
+			break;
+		prev = item;
 	}
 	if (item != NULL) {
-	    if (item == prev) {
-		list = item->next; /* this must be the first one */
-	    } else {
-		prev->next = item->next;
-	    }
-	    item->next = NULL;
-	    dfs_free_list(item);
+		if (item == prev)
+			list = item->next; /* this must be the first one */
+		else
+			prev->next = item->next;
+		item->next = NULL;
+		dfs_free_list(item);
 	}
 	return (list);
 }
@@ -410,33 +407,34 @@ adddfsentry(xfs_sharelist_t *list, sa_share_t share, char *proto)
 
 	item = alloc_sharelist();
 	if (item != NULL) {
-	    parent = sa_get_parent_group(share);
-	    groupname = sa_get_group_attr(parent, "name");
-	    if (strcmp(groupname, "default") == 0) {
-		sa_free_attr_string(groupname);
-		groupname = NULL;
-	    }
-	    item->path = sa_get_share_attr(share, "path");
-	    item->resource = sa_get_share_attr(share, "resource");
-	    item->group = groupname;
-	    item->fstype = strdup(proto);
-	    item->options = sa_proto_legacy_format(proto, share, 1);
-	    if (item->options != NULL && strlen(item->options) == 0) {
-		free(item->options);
-		item->options = NULL;
-	    }
-	    item->description = sa_get_share_description(share);
-	    if (item->description != NULL && strlen(item->description) == 0) {
-		sa_free_share_description(item->description);
-		item->description = NULL;
-	    }
-	    if (list == NULL) {
-		list = item;
-	    } else {
-		for (tmp = list; tmp->next != NULL; tmp = tmp->next)
-		    /* do nothing */;
-		tmp->next = item;
-	    }
+		parent = sa_get_parent_group(share);
+		groupname = sa_get_group_attr(parent, "name");
+		if (strcmp(groupname, "default") == 0) {
+			sa_free_attr_string(groupname);
+			groupname = NULL;
+		}
+		item->path = sa_get_share_attr(share, "path");
+		item->resource = sa_get_share_attr(share, "resource");
+		item->group = groupname;
+		item->fstype = strdup(proto);
+		item->options = sa_proto_legacy_format(proto, share, 1);
+		if (item->options != NULL && strlen(item->options) == 0) {
+			free(item->options);
+			item->options = NULL;
+		}
+		item->description = sa_get_share_description(share);
+		if (item->description != NULL &&
+		    strlen(item->description) == 0) {
+			sa_free_share_description(item->description);
+			item->description = NULL;
+		}
+		if (list == NULL) {
+			list = item;
+		} else {
+			for (tmp = list; tmp->next != NULL; tmp = tmp->next)
+				/* do nothing */;
+				tmp->next = item;
+		}
 	}
 	return (list);
 }
@@ -456,32 +454,36 @@ outdfstab(FILE *dfstab, xfs_sharelist_t *list)
 	(void) ftruncate(fileno(dfstab), 0);
 
 	for (item = list; item != NULL; item = item->next) {
-	    if (item->path != NULL) {
-		if (*item->path == '/')
-		    (void) fprintf(dfstab, "share %s%s%s%s%s%s%s %s%s%s%s%s\n",
-			    (item->fstype != NULL) ? "-F " : "",
-			    (item->fstype != NULL) ? item->fstype : "",
-			    (item->options != NULL) ? " -o " : "",
-			    (item->options != NULL) ? item->options : "",
-			    (item->description != NULL) ? " -d \"" : "",
-			    (item->description != NULL) ?
-				item->description : "",
-			    (item->description != NULL) ? "\"" : "",
-			    item->path,
-			    ((item->resource != NULL) ||
-				(item->group != NULL)) ? " " : "",
-			    (item->resource != NULL) ? item->resource : "",
-			    item->group != NULL ? "@" : "",
-			    item->group != NULL ? item->group : "");
-		else
-		    (void) fprintf(dfstab, "%s", item->origline);
-	    } else {
-		if (item->description != NULL) {
-		    (void) fprintf(dfstab, "%s", item->description);
+		if (item->path != NULL) {
+			if (*item->path == '/') {
+				(void) fprintf(dfstab,
+				    "share %s%s%s%s%s%s%s %s%s%s%s%s\n",
+				    (item->fstype != NULL) ? "-F " : "",
+				    (item->fstype != NULL) ? item->fstype : "",
+				    (item->options != NULL) ? " -o " : "",
+				    (item->options != NULL) ?
+				    item->options : "",
+				    (item->description != NULL) ?
+				    " -d \"" : "",
+				    (item->description != NULL) ?
+				    item->description : "",
+				    (item->description != NULL) ? "\"" : "",
+				    item->path,
+				    ((item->resource != NULL) ||
+				    (item->group != NULL)) ? " " : "",
+				    (item->resource != NULL) ?
+				    item->resource : "",
+				    item->group != NULL ? "@" : "",
+				    item->group != NULL ? item->group : "");
+			} else {
+				(void) fprintf(dfstab, "%s", item->origline);
+			}
 		} else {
-		    (void) fprintf(dfstab, "%s", item->origline);
+			if (item->description != NULL)
+				(void) fprintf(dfstab, "%s", item->description);
+			else
+				(void) fprintf(dfstab, "%s", item->origline);
 		}
-	    }
 	}
 }
 
@@ -503,20 +505,21 @@ open_dfstab(char *file)
 
 	dfstab = fopen(file, "r+");
 	if (dfstab == NULL) {
-	    dfstab = fopen(file, "w+");
+		dfstab = fopen(file, "w+");
 	}
 	if (dfstab != NULL) {
 		grsize = sysconf(_SC_GETGR_R_SIZE_MAX);
 		buff = malloc(grsize);
 		if (buff != NULL)
-		    grp = getgrnam_r(SA_DEFAULT_FILE_GRP, &group, buff, grsize);
+			grp = getgrnam_r(SA_DEFAULT_FILE_GRP, &group, buff,
+			    grsize);
 		else
-		    grp = getgrnam(SA_DEFAULT_FILE_GRP); /* take the risk */
+			grp = getgrnam(SA_DEFAULT_FILE_GRP);
 		(void) fchmod(fileno(dfstab), 0644);
 		(void) fchown(fileno(dfstab), 0,
-				grp != NULL ? grp->gr_gid : 3);
+		    grp != NULL ? grp->gr_gid : 3);
 		if (buff != NULL)
-		    free(buff);
+			free(buff);
 		rewind(dfstab);
 	}
 	return (dfstab);
@@ -555,7 +558,7 @@ sa_comment_line(char *line, char *err)
 		(void) fclose(dfstab);
 		saunblocksigs(&old);
 		if (list != NULL)
-		    dfs_free_list(list);
+			dfs_free_list(list);
 	}
 }
 
@@ -584,46 +587,48 @@ sa_delete_legacy(sa_share_t share)
 		path = sa_get_share_attr(share, "path");
 		parent = sa_get_parent_group(share);
 		if (parent != NULL) {
-		    (void) lockf(fileno(dfstab), F_LOCK, 0);
-		    list = getdfstab(dfstab);
-		    rewind(dfstab);
-		    for (optionset = sa_get_optionset(parent, NULL);
-			optionset != NULL;
-			optionset = sa_get_next_optionset(optionset)) {
-			char *proto = sa_get_optionset_attr(optionset, "type");
-			if (list != NULL && proto != NULL)
-			    list = remdfsentry(list, path, proto);
-			if (proto == NULL)
-			    ret = SA_NO_MEMORY;
-			/*
-			 * may want to only do the dfstab if this call
-			 * returns NOT IMPLEMENTED but it shouldn't
-			 * hurt.
-			 */
-			if (ret == SA_OK) {
-			    err = sa_proto_delete_legacy(proto, share);
-			    if (err != SA_NOT_IMPLEMENTED)
-				ret = err;
+			(void) lockf(fileno(dfstab), F_LOCK, 0);
+			list = getdfstab(dfstab);
+			rewind(dfstab);
+			for (optionset = sa_get_optionset(parent, NULL);
+			    optionset != NULL;
+			    optionset = sa_get_next_optionset(optionset)) {
+				char *proto = sa_get_optionset_attr(optionset,
+				    "type");
+				if (list != NULL && proto != NULL)
+					list = remdfsentry(list, path,
+					    proto);
+				if (proto == NULL)
+					ret = SA_NO_MEMORY;
+				/*
+				 * May want to only do the dfstab if
+				 * this call returns NOT IMPLEMENTED
+				 * but it shouldn't hurt.
+				 */
+				if (ret == SA_OK) {
+					err = sa_proto_delete_legacy(proto,
+					    share);
+					if (err != SA_NOT_IMPLEMENTED)
+						ret = err;
+				}
+				if (proto != NULL)
+					sa_free_attr_string(proto);
 			}
-			if (proto != NULL)
-			    sa_free_attr_string(proto);
-		    }
-		    outdfstab(dfstab, list);
-		    if (list != NULL)
-			dfs_free_list(list);
-		    (void) fflush(dfstab);
-		    (void) lockf(fileno(dfstab), F_ULOCK, 0);
+			outdfstab(dfstab, list);
+			if (list != NULL)
+				dfs_free_list(list);
+			(void) fflush(dfstab);
+			(void) lockf(fileno(dfstab), F_ULOCK, 0);
 		}
 		(void) fsync(fileno(dfstab));
 		saunblocksigs(&old);
 		(void) fclose(dfstab);
 		sa_free_attr_string(path);
 	} else {
-		if (errno == EACCES || errno == EPERM) {
-		    ret = SA_NO_PERMISSION;
-		} else {
-		    ret = SA_CONFIG_ERR;
-		}
+		if (errno == EACCES || errno == EPERM)
+			ret = SA_NO_PERMISSION;
+		else
+			ret = SA_CONFIG_ERR;
 	}
 	return (ret);
 }
@@ -653,7 +658,7 @@ sa_update_legacy(sa_share_t share, char *proto)
 
 	ret = sa_proto_update_legacy(proto, share);
 	if (ret != SA_NOT_IMPLEMENTED)
-	    return (ret);
+		return (ret);
 	/* do the dfstab format */
 	persist = sa_get_share_attr(share, "type");
 	/*
@@ -661,36 +666,35 @@ sa_update_legacy(sa_share_t share, char *proto)
 	 * set or the type is not "transient".
 	 */
 	if (persist == NULL || strcmp(persist, "transient") != 0) {
-	    dfstab = open_dfstab(SA_LEGACY_DFSTAB);
-	    if (dfstab != NULL) {
-		(void) setvbuf(dfstab, NULL, _IOLBF, BUFSIZ * 8);
-		sablocksigs(&old);
-		path = sa_get_share_attr(share, "path");
-		(void) lockf(fileno(dfstab), F_LOCK, 0);
-		list = getdfstab(dfstab);
-		rewind(dfstab);
-		if (list != NULL)
-		    list = remdfsentry(list, path, proto);
-		list = adddfsentry(list, share, proto);
-		outdfstab(dfstab, list);
-		(void) fflush(dfstab);
-		(void) lockf(fileno(dfstab), F_ULOCK, 0);
-		(void) fsync(fileno(dfstab));
-		saunblocksigs(&old);
-		(void) fclose(dfstab);
-		sa_free_attr_string(path);
-		if (list != NULL)
-		    dfs_free_list(list);
-	    } else {
-		if (errno == EACCES || errno == EPERM) {
-		    ret = SA_NO_PERMISSION;
+		dfstab = open_dfstab(SA_LEGACY_DFSTAB);
+		if (dfstab != NULL) {
+			(void) setvbuf(dfstab, NULL, _IOLBF, BUFSIZ * 8);
+			sablocksigs(&old);
+			path = sa_get_share_attr(share, "path");
+			(void) lockf(fileno(dfstab), F_LOCK, 0);
+			list = getdfstab(dfstab);
+			rewind(dfstab);
+			if (list != NULL)
+				list = remdfsentry(list, path, proto);
+			list = adddfsentry(list, share, proto);
+			outdfstab(dfstab, list);
+			(void) fflush(dfstab);
+			(void) lockf(fileno(dfstab), F_ULOCK, 0);
+			(void) fsync(fileno(dfstab));
+			saunblocksigs(&old);
+			(void) fclose(dfstab);
+			sa_free_attr_string(path);
+			if (list != NULL)
+				dfs_free_list(list);
 		} else {
-		    ret = SA_CONFIG_ERR;
+			if (errno == EACCES || errno == EPERM)
+				ret = SA_NO_PERMISSION;
+			else
+				ret = SA_CONFIG_ERR;
 		}
-	    }
 	}
 	if (persist != NULL)
-	    sa_free_attr_string(persist);
+		sa_free_attr_string(persist);
 	return (ret);
 }
 
@@ -706,14 +710,14 @@ sa_is_security(char *optname, char *proto)
 {
 	int ret = 0;
 	if (proto != NULL)
-	    ret = sa_proto_security_prop(proto, optname);
+		ret = sa_proto_security_prop(proto, optname);
 	return (ret);
 }
 
 /*
  * add_syntax_comment(root, line, err, todfstab)
  *
- * add a comment to the document indicating a syntax error. If
+ * Add a comment to the document indicating a syntax error. If
  * todfstab is set, write it back to the dfstab file as well.
  */
 
@@ -723,11 +727,10 @@ add_syntax_comment(xmlNodePtr root, char *line, char *err, int todfstab)
 	xmlNodePtr node;
 
 	node = xmlNewChild(root, NULL, (xmlChar *)"error", (xmlChar *)line);
-	if (node != NULL) {
-	    xmlSetProp(node, (xmlChar *)"type", (xmlChar *)err);
-	}
+	if (node != NULL)
+		xmlSetProp(node, (xmlChar *)"type", (xmlChar *)err);
 	if (todfstab)
-	    sa_comment_line(line, err);
+		sa_comment_line(line, err);
 }
 
 /*
@@ -740,7 +743,7 @@ int
 sa_is_share(void *object)
 {
 	if (object != NULL) {
-	    if (strcmp((char *)((xmlNodePtr)object)->name, "share") == 0)
+		if (strcmp((char *)((xmlNodePtr)object)->name, "share") == 0)
 		return (1);
 	}
 	return (0);
@@ -775,22 +778,22 @@ _sa_create_dummy_share()
 
 	parent_node = xmlNewNode(NULL, (xmlChar *)"group");
 	if (parent_node != NULL) {
-	    child_node = xmlNewChild(parent_node, NULL, (xmlChar *)"share",
-					NULL);
-	    if (child_node != NULL) {
-		/*
-		 * Use a "zfs" tag since that will make sure nothing
-		 * really attempts to put values into the
-		 * repository. Also ZFS is currently the only user of
-		 * this interface.
-		 */
-		set_node_attr(parent_node, "type", "transient");
-		set_node_attr(parent_node, "zfs", "true");
-		set_node_attr(child_node, "type", "transient");
-		set_node_attr(child_node, "zfs", "true");
-	    } else {
-		xmlFreeNode(parent_node);
-	    }
+		child_node = xmlNewChild(parent_node, NULL, (xmlChar *)"share",
+		    NULL);
+		if (child_node != NULL) {
+			/*
+			 * Use a "zfs" tag since that will make sure nothing
+			 * really attempts to put values into the
+			 * repository. Also ZFS is currently the only user of
+			 * this interface.
+			 */
+			set_node_attr(parent_node, "type", "transient");
+			set_node_attr(parent_node, "zfs", "true");
+			set_node_attr(child_node, "type", "transient");
+			set_node_attr(child_node, "zfs", "true");
+		} else {
+			xmlFreeNode(parent_node);
+		}
 	}
 	return (child_node);
 }
@@ -811,24 +814,24 @@ _sa_free_dummy_share(sa_share_t share)
 	char *name;
 
 	if (node != NULL) {
-	    parent = node->parent;
-	    name = (char *)xmlGetProp(node, (xmlChar *)"path");
-	    if (name != NULL) {
-		/* Real shares always have a path but a dummy doesn't */
-		ret = SA_NOT_ALLOWED;
-		sa_free_attr_string(name);
-	    } else {
-		/*
-		 * If there is a parent, do the free on that since
-		 * xmlFreeNode is a recursive function and free's an
-		 * child nodes.
-		 */
-		if (parent != NULL) {
-		    node = parent;
+		parent = node->parent;
+		name = (char *)xmlGetProp(node, (xmlChar *)"path");
+		if (name != NULL) {
+			/* Real shares always have a path but a dummy doesn't */
+			ret = SA_NOT_ALLOWED;
+			sa_free_attr_string(name);
+		} else {
+			/*
+			 * If there is a parent, do the free on that since
+			 * xmlFreeNode is a recursive function and free's an
+			 * child nodes.
+			 */
+			if (parent != NULL) {
+				node = parent;
+			}
+			xmlUnlinkNode(node);
+			xmlFreeNode(node);
 		}
-		xmlUnlinkNode(node);
-		xmlFreeNode(node);
-	    }
 	}
 	return (ret);
 }
@@ -855,127 +858,144 @@ sa_parse_legacy_options(sa_group_t group, char *options, char *proto)
 	sa_group_t parent;
 	int using_dummy = B_FALSE;
 	char *pvalue;
+	sa_optionset_t optionset;
+	sa_property_t popt, prop;
+	sa_optionset_t localoptions;
 
 	/*
-	 * if "group" is NULL, this is just a parse without saving
+	 * If "group" is NULL, this is just a parse without saving
 	 * anything in either SMF or ZFS.  Create a dummy group to
 	 * handle this case.
 	 */
 	if (group == NULL) {
-	    group = (sa_group_t)_sa_create_dummy_share();
-	    using_dummy = B_TRUE;
+		group = (sa_group_t)_sa_create_dummy_share();
+		using_dummy = B_TRUE;
 	}
 
 	parent = sa_get_parent_group(group);
 
 	if (proto != NULL)
-	    ret = sa_proto_legacy_opts(proto, group, options);
+		ret = sa_proto_legacy_opts(proto, group, options);
 
 	if (using_dummy) {
-	    /* Since this is a dummy parse, cleanup and quit here */
-	    (void) _sa_free_dummy_share(parent);
-	    return (ret);
+		/* Since this is a dummy parse, cleanup and quit here */
+		(void) _sa_free_dummy_share(parent);
+		return (ret);
 	}
+
+	if (ret != SA_OK)
+		return (ret);
+
 	/*
-	 * if in a group, remove the inherited options and security
+	 * If in a group, remove the inherited options and security
 	 */
-	if (ret == SA_OK) {
-	    if (parent != NULL) {
-		sa_optionset_t optionset;
-		sa_property_t popt, prop;
-		sa_optionset_t localoptions;
-		/* find parent options to remove from child */
-		optionset = sa_get_derived_optionset(parent, proto, 1);
-		localoptions = sa_get_optionset(group, proto);
-		if (optionset != NULL) {
-		    for (popt = sa_get_property(optionset, NULL);
-			popt != NULL;
-			popt = sa_get_next_property(popt)) {
+
+	if (parent == NULL)
+		return (ret);
+
+	/* Find parent options to remove from child */
+	optionset = sa_get_derived_optionset(parent, proto, 1);
+	localoptions = sa_get_optionset(group, proto);
+	if (optionset != NULL) {
+		for (popt = sa_get_property(optionset, NULL);
+		    popt != NULL;
+		    popt = sa_get_next_property(popt)) {
 			char *tag;
 			char *value;
-
 			tag = sa_get_property_attr(popt, "type");
-			if (tag != NULL) {
-			    prop = sa_get_property(localoptions, tag);
-			    if (prop != NULL) {
-				value = sa_get_property_attr(popt, "value");
-				pvalue = sa_get_property_attr(prop, "value");
+			if (tag == NULL)
+				continue;
+			prop = sa_get_property(localoptions, tag);
+			if (prop != NULL) {
+				value = sa_get_property_attr(popt,
+				    "value");
+				pvalue = sa_get_property_attr(prop,
+				    "value");
 				if (value != NULL && pvalue != NULL &&
 				    strcmp(value, pvalue) == 0) {
 					/*
-					 * Remove the property from the
-					 * child. While we removed it, we
-					 * don't need to reset as we do
-					 * below since we always search
-					 * from the beginning.
+					 * Remove the property
+					 * from the
+					 * child. While we
+					 * removed it, we
+					 * don't need to reset
+					 * as we do below
+					 * since we always
+					 * search from the
+					 * beginning.
 					 */
-					(void) _sa_remove_property(prop);
+					(void) _sa_remove_property(
+					    prop);
 				}
 				if (value != NULL)
-				    sa_free_attr_string(value);
+					sa_free_attr_string(value);
 				if (pvalue != NULL)
-				    sa_free_attr_string(pvalue);
-			    }
-			    sa_free_attr_string(tag);
+					sa_free_attr_string(pvalue);
 			}
-		    }
-		    prop = sa_get_property(localoptions, NULL);
-		    if (prop == NULL && sa_is_share(group)) {
+			sa_free_attr_string(tag);
+		}
+		prop = sa_get_property(localoptions, NULL);
+		if (prop == NULL && sa_is_share(group)) {
 			/*
-			 * all properties removed so remove the
+			 * All properties removed so remove the
 			 * optionset if it is on a share
 			 */
 			(void) _sa_remove_optionset(localoptions);
-		    }
-		    sa_free_derived_optionset(optionset);
 		}
-		/*
-		 * need to remove security here. If there are no
-		 * security options on the local group/share, don't
-		 * bother since those are the only ones that would be
-		 * affected.
-		 */
-		localoptions = sa_get_all_security_types(group, proto, 0);
-		if (localoptions != NULL) {
-		    for (prop = sa_get_property(localoptions, NULL);
-			prop != NULL; prop = sa_get_next_property(prop)) {
+		sa_free_derived_optionset(optionset);
+	}
+	/*
+	 * Need to remove security here. If there are no
+	 * security options on the local group/share, don't
+	 * bother since those are the only ones that would be
+	 * affected.
+	 */
+	localoptions = sa_get_all_security_types(group, proto, 0);
+	if (localoptions != NULL) {
+		for (prop = sa_get_property(localoptions, NULL);
+		    prop != NULL;
+		    prop = sa_get_next_property(prop)) {
 			char *tag;
 			sa_security_t security;
 			tag = sa_get_property_attr(prop, "type");
 			if (tag != NULL) {
-			    sa_property_t nextpopt = NULL;
-
-			    security = sa_get_security(group, tag, proto);
-			    sa_free_attr_string(tag);
-			    /* prop's value only changes outside this loop */
-			    pvalue = sa_get_property_attr(prop, "value");
-			    for (popt = sa_get_property(security, NULL);
-				popt != NULL;
-				popt = nextpopt) {
-				char *value;
+				sa_property_t nextpopt = NULL;
+				security = sa_get_security(group, tag, proto);
+				sa_free_attr_string(tag);
 				/*
-				 * Need to get the next prop now since
-				 * we could break the list during removal.
+				 * prop's value only changes outside this loop
 				 */
-				nextpopt = sa_get_next_property(popt);
-
-				/* remove duplicates from this level */
-				value = sa_get_property_attr(popt, "value");
-				if (value != NULL && pvalue != NULL &&
-				    strcmp(value, pvalue) == 0) {
-					/* remove the property from the child */
-					(void) _sa_remove_property(popt);
+				pvalue = sa_get_property_attr(prop, "value");
+				for (popt = sa_get_property(security, NULL);
+				    popt != NULL;
+				    popt = nextpopt) {
+					char *value;
+					/*
+					 * Need to get the next prop
+					 * now since we could break
+					 * the list during removal.
+					 */
+					nextpopt = sa_get_next_property(popt);
+					/* remove Duplicates from this level */
+					value = sa_get_property_attr(popt,
+					    "value");
+					if (value != NULL && pvalue != NULL &&
+					    strcmp(value, pvalue) == 0) {
+						/*
+						 * remove the property
+						 * from the child
+						 */
+						(void) _sa_remove_property
+						    (popt);
+					}
+					if (value != NULL)
+						sa_free_attr_string(value);
 				}
-				if (value != NULL)
-				    sa_free_attr_string(value);
-			    }
-			    if (pvalue != NULL)
-				sa_free_attr_string(pvalue);
+				if (pvalue != NULL)
+					sa_free_attr_string(pvalue);
 			}
-		    }
-		    (void) sa_destroy_optionset(localoptions);
 		}
-	    }
+		(void) sa_destroy_optionset(localoptions);
 	}
 	return (ret);
 }
@@ -993,22 +1013,22 @@ dfs_free_list(xfs_sharelist_t *list)
 {
 	xfs_sharelist_t *entry;
 	for (entry = list; entry != NULL; entry = list) {
-	    if (entry->path != NULL)
-		free(entry->path);
-	    if (entry->resource != NULL)
-		free(entry->resource);
-	    if (entry->fstype != NULL)
-		free(entry->fstype);
-	    if (entry->options != NULL)
-		free(entry->options);
-	    if (entry->description != NULL)
-		free(entry->description);
-	    if (entry->origline != NULL)
-		free(entry->origline);
-	    if (entry->group != NULL)
-		free(entry->group);
-	    list = list->next;
-	    free(entry);
+		if (entry->path != NULL)
+			free(entry->path);
+		if (entry->resource != NULL)
+			free(entry->resource);
+		if (entry->fstype != NULL)
+			free(entry->fstype);
+		if (entry->options != NULL)
+			free(entry->options);
+		if (entry->description != NULL)
+			free(entry->description);
+		if (entry->origline != NULL)
+			free(entry->origline);
+		if (entry->group != NULL)
+			free(entry->group);
+		list = list->next;
+			free(entry);
 	}
 }
 
@@ -1036,131 +1056,128 @@ parse_dfstab(sa_handle_t handle, char *dfstab, xmlNodePtr root)
 	/* read the dfstab format file and fill in the doc tree */
 
 	dfs = fopen(dfstab, "r");
-	if (dfs == NULL) {
-	    return;
-	}
+	if (dfs == NULL)
+		return;
 
 	defgroup = sa_get_group(handle, "default");
 
 	for (head = list = getdfstab(dfs);
-		list != NULL;
-		list = list->next) {
-	    share = NULL;
-	    group = NULL;
-	    defined_group = 0;
-	    err = 0;
-
-	    if (list->origline == NULL) {
-		/*
-		 * Comment line that we will likely skip.
-		 * If the line has the syntax:
-		 *	# error: string: string
-		 * It should be preserved until manually deleted.
-		 */
-		if (list->description != NULL &&
-		    strncmp(list->description, "# Error: ", 9) == 0) {
-		    char *line;
-		    char *error;
-		    char *cmd;
-		    line = strdup(list->description);
-		    if (line != NULL) {
-			error = line + 9;
-			cmd = strchr(error, ':');
-			if (cmd != NULL) {
-			    int len;
-			    *cmd = '\0';
-			    cmd += 2;
-			    len = strlen(cmd);
-			    cmd[len - 1] = '\0';
-			    add_syntax_comment(root, cmd, error, 0);
+	    list != NULL;
+	    list = list->next) {
+		share = NULL;
+		group = NULL;
+		defined_group = 0;
+		err = 0;
+
+		if (list->origline == NULL) {
+			/*
+			 * Comment line that we will likely skip.
+			 * If the line has the syntax:
+			 *	# error: string: string
+			 * It should be preserved until manually deleted.
+			 */
+			if (list->description != NULL &&
+			    strncmp(list->description, "# Error: ", 9) == 0) {
+				char *line;
+				char *error;
+				char *cmd;
+				line = strdup(list->description);
+				if (line != NULL) {
+					error = line + 9;
+					cmd = strchr(error, ':');
+					if (cmd != NULL) {
+						int len;
+						*cmd = '\0';
+						cmd += 2;
+						len = strlen(cmd);
+						cmd[len - 1] = '\0';
+						add_syntax_comment(root, cmd,
+						    error, 0);
+					}
+					free(line);
+				}
 			}
-			free(line);
-		    }
+			continue;
 		}
-		continue;
-	    }
-	    if (list->path != NULL && strlen(list->path) > 0 &&
+		if (list->path != NULL && strlen(list->path) > 0 &&
 		    *list->path == '/') {
-		share = sa_find_share(handle, list->path);
-		if (share != NULL)
-		    sgroup = sa_get_parent_group(share);
-		else
-		    sgroup = NULL;
-	    } else {
-		(void) printf(dgettext(TEXT_DOMAIN,
-					"No share specified in dfstab: "
-					"line %d: %s\n"),
-			list->lineno, list->origline);
-		add_syntax_comment(root, list->origline,
-				    dgettext(TEXT_DOMAIN, "No share specified"),
-				    1);
-		continue;
-	    }
-	    if (list->group != NULL && strlen(list->group) > 0) {
-		group = sa_get_group(handle, list->group);
-		defined_group = 1;
-	    } else {
-		group = defgroup;
-	    }
-	    if (defined_group && group == NULL) {
-		(void) printf(dgettext(TEXT_DOMAIN,
-					"Unknown group used in dfstab: "
-					"line %d: %s\n"),
-			list->lineno, list->origline);
-		add_syntax_comment(root, list->origline,
-				    dgettext(TEXT_DOMAIN,
-						"Unknown group specified"), 1);
-		continue;
-	    }
-	    if (group != NULL) {
+			share = sa_find_share(handle, list->path);
+			if (share != NULL)
+				sgroup = sa_get_parent_group(share);
+			else
+				sgroup = NULL;
+		} else {
+			(void) printf(dgettext(TEXT_DOMAIN,
+			    "No share specified in dfstab: "
+			    "line %d: %s\n"),
+			    list->lineno, list->origline);
+			add_syntax_comment(root, list->origline,
+			    dgettext(TEXT_DOMAIN, "No share specified"), 1);
+			continue;
+		}
+		if (list->group != NULL && strlen(list->group) > 0) {
+			group = sa_get_group(handle, list->group);
+			defined_group = 1;
+		} else {
+			group = defgroup;
+		}
+		if (defined_group && group == NULL) {
+			(void) printf(dgettext(TEXT_DOMAIN,
+			    "Unknown group used in dfstab: line %d: %s\n"),
+			    list->lineno, list->origline);
+			add_syntax_comment(root, list->origline,
+			    dgettext(TEXT_DOMAIN, "Unknown group specified"),
+			    1);
+			continue;
+		}
+		if (group == NULL) {
+			/* Shouldn't happen unless an SMF error */
+			err = SA_CONFIG_ERR;
+			continue;
+		}
 		if (share == NULL) {
-		    if (!defined_group && group == defgroup) {
-			/* this is an OK add for legacy */
+			if (defined_group || group != defgroup)
+				continue;
+			/* This is an OK add for legacy */
 			share = sa_add_share(defgroup, list->path,
-					SA_SHARE_PERMANENT | SA_SHARE_PARSER,
-					&err);
+			    SA_SHARE_PERMANENT | SA_SHARE_PARSER, &err);
 			if (share != NULL) {
-			    if (list->description != NULL &&
-				strlen(list->description) > 0)
-				(void) sa_set_share_description(share,
-							    list->description);
-			    if (list->options != NULL &&
-				strlen(list->options) > 0) {
-				(void) sa_parse_legacy_options(share,
-								list->options,
-								list->fstype);
-			    }
-			    if (list->resource != NULL)
-				(void) sa_set_share_attr(share, "resource",
-						    list->resource);
+				if (list->description != NULL &&
+				    strlen(list->description) > 0)
+					(void) sa_set_share_description(share,
+					    list->description);
+				if (list->options != NULL &&
+				    strlen(list->options) > 0) {
+					(void) sa_parse_legacy_options(share,
+					    list->options, list->fstype);
+				}
+				if (list->resource != NULL)
+					(void) sa_set_share_attr(share,
+					    "resource", list->resource);
 			} else {
-			    (void) printf(dgettext(TEXT_DOMAIN,
-					    "Error in dfstab: "
-					    "line %d: %s\n"),
+				(void) printf(dgettext(TEXT_DOMAIN,
+				    "Error in dfstab: line %d: %s\n"),
 				    list->lineno, list->origline);
-			    if (err != SA_BAD_PATH)
-				add_syntax_comment(root, list->origline,
-						dgettext(TEXT_DOMAIN,
-							    "Syntax"), 1);
-			    else
-				add_syntax_comment(root, list->origline,
-						dgettext(TEXT_DOMAIN,
-							    "Path"), 1);
-			    continue;
+				if (err != SA_BAD_PATH)
+					add_syntax_comment(root, list->origline,
+					    dgettext(TEXT_DOMAIN, "Syntax"), 1);
+				else
+					add_syntax_comment(root, list->origline,
+					    dgettext(TEXT_DOMAIN,
+					    "Path"), 1);
+				continue;
 			}
-		    }
 		} else {
-		    if (group != sgroup) {
-			(void) printf(dgettext(TEXT_DOMAIN, "Attempt to change"
-						"configuration in"
-						"dfstab: line %d: %s\n"),
-				list->lineno, list->origline);
-			add_syntax_comment(root, list->origline,
-				dgettext(TEXT_DOMAIN,
-					    "Attempt to change configuration"),
-				1);
-			continue;
-		    }
+			if (group != sgroup) {
+				(void) printf(dgettext(TEXT_DOMAIN,
+				    "Attempt to change configuration in "
+				    "dfstab: line %d: %s\n"),
+				    list->lineno, list->origline);
+				add_syntax_comment(root, list->origline,
+				    dgettext(TEXT_DOMAIN,
+				    "Attempt to change configuration"), 1);
+				continue;
+			}
 			/*
 			 * It is the same group but could have changed
 			 * options. Make sure we include the group's
@@ -1169,36 +1186,35 @@ parse_dfstab(sa_handle_t handle, char *dfstab, xmlNodePtr root)
 			 * true says to get the inherited properties as well
 			 * as the local properties.
 			 */
-		    oldprops = sa_proto_legacy_format(list->fstype, share,
-			B_TRUE);
-		    if (oldprops != NULL) {
+			oldprops = sa_proto_legacy_format(list->fstype, share,
+			    B_TRUE);
+
+			if (oldprops == NULL)
+				continue;
+
 			if (list->options != NULL &&
-				strcmp(oldprops, list->options) != 0) {
-			    sa_optionset_t opts;
-			    sa_security_t secs;
-			    /* possibly different values */
-			    opts = sa_get_optionset((sa_group_t)share,
-							list->fstype);
-			    (void) sa_destroy_optionset(opts);
-			    for (secs = sa_get_security((sa_group_t)share,
-							NULL, list->fstype);
-				secs != NULL;
-				secs = sa_get_security((sa_group_t)share,
-							NULL, list->fstype)) {
-				(void) sa_destroy_security(secs);
-			    }
-			    (void) sa_parse_legacy_options(share,
-							    list->options,
-							    list->fstype);
+			    strcmp(oldprops, list->options) != 0) {
+				sa_optionset_t opts;
+				sa_security_t secs;
+
+				/* possibly different values */
+				opts = sa_get_optionset((sa_group_t)
+				    share, list->fstype);
+				(void) sa_destroy_optionset(opts);
+
+				for (secs = sa_get_security(
+				    (sa_group_t)share, NULL, list->fstype);
+				    secs != NULL;
+				    secs = sa_get_security((sa_group_t)share,
+				    NULL, list->fstype)) {
+					(void) sa_destroy_security(
+					    secs);
+				}
+				(void) sa_parse_legacy_options(share,
+				    list->options, list->fstype);
 			}
 			sa_format_free(oldprops);
-		    }
 		}
-	    } else {
-		/* shouldn't happen */
-		err = SA_CONFIG_ERR;
-	    }
-
 	}
 	dfs_free_list(head);
 }
@@ -1223,27 +1239,30 @@ legacy_removes(sa_group_t group, char *file)
 
 	dfstab = fopen(file, "r");
 	if (dfstab != NULL) {
-	    list = getdfstab(dfstab);
-	    (void) fclose(dfstab);
+		list = getdfstab(dfstab);
+		(void) fclose(dfstab);
 retry:
-	    for (share = sa_get_share(group, NULL); share != NULL;
-		share = sa_get_next_share(share)) {
-		/* now see if the share is in the dfstab file */
-		path = sa_get_share_attr(share, "path");
-		if (path != NULL) {
-		    item = finddfsentry(list, path);
-		    sa_free_attr_string(path);
-		    if (item == NULL) {
-			/* the share was removed this way */
-			(void) sa_remove_share(share);
-
-			/* start over since the list was broken */
-			goto retry;
-		    }
+		for (share = sa_get_share(group, NULL);
+		    share != NULL;
+		    share = sa_get_next_share(share)) {
+			/* now see if the share is in the dfstab file */
+			path = sa_get_share_attr(share, "path");
+			if (path != NULL) {
+				item = finddfsentry(list, path);
+				sa_free_attr_string(path);
+				if (item == NULL) {
+					/* The share was removed this way */
+					(void) sa_remove_share(share);
+
+					/*
+					 * Start over since the list was broken
+					 */
+					goto retry;
+				}
+			}
 		}
-	    }
-	    if (list != NULL)
-		dfs_free_list(list);
+		if (list != NULL)
+			dfs_free_list(list);
 	}
 }
 
@@ -1260,25 +1279,24 @@ getlegacyconfig(sa_handle_t handle, char *path, xmlNodePtr *root)
 	sa_group_t defgroup;
 
 	if (root != NULL) {
-	    if (*root == NULL)
-		*root = xmlNewNode(NULL, (xmlChar *)"sharecfg");
-	    if (*root != NULL) {
-		if (strcmp(path, SA_LEGACY_DFSTAB) == 0) {
-			/*
-			 * walk the default shares and find anything
-			 * missing.  we do this first to make sure it
-			 * is cleaned up since there may be legacy
-			 * code add/del via dfstab and we need to
-			 * cleanup SMF.
-			 */
-		    defgroup = sa_get_group(handle, "default");
-		    if (defgroup != NULL) {
-			legacy_removes(defgroup, path);
-		    }
-		    /* parse the dfstab and add anything new */
-		    parse_dfstab(handle, path, *root);
+		if (*root == NULL)
+			*root = xmlNewNode(NULL, (xmlChar *)"sharecfg");
+		if (*root != NULL) {
+			if (strcmp(path, SA_LEGACY_DFSTAB) == 0) {
+				/*
+				 * Walk the default shares and find anything
+				 * missing.  we do this first to make sure it
+				 * is cleaned up since there may be legacy
+				 * code add/del via dfstab and we need to
+				 * cleanup SMF.
+				 */
+				defgroup = sa_get_group(handle, "default");
+				if (defgroup != NULL)
+					legacy_removes(defgroup, path);
+				/* Parse the dfstab and add anything new */
+				parse_dfstab(handle, path, *root);
+			}
 		}
-	    }
 	}
 }
 
@@ -1304,43 +1322,42 @@ get_share_list(int *errp)
 		struct share	*sharetab_entry;
 
 		while (getshare(fp, &sharetab_entry) > 0) {
-		    newp = alloc_sharelist();
-		    if (newp == NULL) {
-			goto err;
-		    }
+			newp = alloc_sharelist();
+			if (newp == NULL)
+				goto err;
 
 			/*
-			 * link into the list here so we don't leak
+			 * Link into the list here so we don't leak
 			 * memory on a failure from strdup().
 			 */
-		    if (headp == NULL) {
-			headp = newp;
-			tailp = newp;
-		    } else {
-			tailp->next = newp;
-			tailp = newp;
-		    }
-
-		    newp->path = strdup(sharetab_entry->sh_path);
-		    if (newp->path == NULL)
-			goto err;
-		    newp->resource = strdup(sharetab_entry->sh_res);
-		    if (newp->resource == NULL)
-			goto err;
-		    newp->fstype = strdup(sharetab_entry->sh_fstype);
-		    if (newp->fstype == NULL)
-			goto err;
-		    newp->options = strdup(sharetab_entry->sh_opts);
-		    if (newp->options == NULL)
-			goto err;
-		    newp->description = strdup(sharetab_entry->sh_descr);
-		    if (newp->description == NULL)
-			goto err;
+			if (headp == NULL) {
+				headp = newp;
+				tailp = newp;
+			} else {
+				tailp->next = newp;
+				tailp = newp;
+			}
+
+			newp->path = strdup(sharetab_entry->sh_path);
+			if (newp->path == NULL)
+				goto err;
+			newp->resource = strdup(sharetab_entry->sh_res);
+			if (newp->resource == NULL)
+				goto err;
+			newp->fstype = strdup(sharetab_entry->sh_fstype);
+			if (newp->fstype == NULL)
+				goto err;
+			newp->options = strdup(sharetab_entry->sh_opts);
+			if (newp->options == NULL)
+				goto err;
+			newp->description = strdup(sharetab_entry->sh_descr);
+			if (newp->description == NULL)
+				goto err;
 		}
 		(void) lockf(fileno(fp), F_ULOCK, 0);
 		(void) fclose(fp);
 	} else {
-	    *errp = errno;
+		*errp = errno;
 	}
 
 	/*
@@ -1378,110 +1395,121 @@ parse_sharetab(sa_handle_t handle)
 
 	list = get_share_list(&err);
 	if (list == NULL)
-	    return (legacy);
+		return (legacy);
 
 	lgroup = sa_get_group(handle, "default");
 
 	for (tmplist = list; tmplist != NULL; tmplist = tmplist->next) {
-	    group = NULL;
-	    share = sa_find_share(handle, tmplist->path);
-	    if (share == NULL) {
+		group = NULL;
+		share = sa_find_share(handle, tmplist->path);
+		if (share != NULL) {
+			/*
+			 * If this is a legacy share, mark as shared so we
+			 * only update sharetab appropriately. We also keep
+			 * the sharetab options in order to display for legacy
+			 * share with no arguments.
+			 */
+			set_node_attr(share, "shared", "true");
+			set_node_attr(share, "shareopts", tmplist->options);
+			continue;
+		}
+
 		/*
-		 * this share is transient so needs to be
+		 * This share is transient so needs to be
 		 * added. Initially, this will be under
 		 * default(legacy) unless it is a ZFS
 		 * share. If zfs, we need a zfs group.
 		 */
 		if (tmplist->resource != NULL &&
 		    (groupname = strchr(tmplist->resource, '@')) != NULL) {
-		    /* there is a defined group */
-		    *groupname++ = '\0';
-		    group = sa_get_group(handle, groupname);
-		    if (group != NULL) {
-			share = _sa_add_share(group, tmplist->path,
-						SA_SHARE_TRANSIENT, &err);
-		    } else {
-			/*
-			 * While this case shouldn't occur very often,
-			 * it does occur out of a "zfs set
-			 * sharenfs=off" when the dataset is also set
-			 * to canmount=off. A warning will then cause
-			 * the zfs command to abort. Since we add it
-			 * to the default list, everything works
-			 * properly anyway and the library doesn't
-			 * need to give a warning.
-			 */
-			share = _sa_add_share(lgroup, tmplist->path,
-						SA_SHARE_TRANSIENT, &err);
-		    }
-		} else {
-		    if (sa_zfs_is_shared(handle, tmplist->path)) {
-			group = sa_get_group(handle, "zfs");
-			if (group == NULL) {
-			    group = sa_create_group(handle, "zfs", &err);
-			    if (group == NULL && err == SA_NO_PERMISSION) {
-				group = _sa_create_group(
-						(sa_handle_impl_t)handle,
-						"zfs");
-			    }
-			    if (group != NULL) {
-				(void) sa_create_optionset(group,
-							    tmplist->fstype);
-				(void) sa_set_group_attr(group, "zfs", "true");
-			    }
-			}
+			/* There is a defined group */
+			*groupname++ = '\0';
+			group = sa_get_group(handle, groupname);
 			if (group != NULL) {
-			    share = _sa_add_share(group, tmplist->path,
-						    SA_SHARE_TRANSIENT, &err);
+				share = _sa_add_share(group,
+				    tmplist->path,
+				    SA_SHARE_TRANSIENT, &err);
+			} else {
+				/*
+				 * While this case shouldn't
+				 * occur very often, it does
+				 * occur out of a "zfs set
+				 * sharenfs=off" when the
+				 * dataset is also set to
+				 * canmount=off. A warning
+				 * will then cause the zfs
+				 * command to abort. Since we
+				 * add it to the default list,
+				 * everything works properly
+				 * anyway and the library
+				 * doesn't need to give a
+				 * warning.
+				 */
+				share = _sa_add_share(lgroup,
+				    tmplist->path, SA_SHARE_TRANSIENT,
+				    &err);
+			}
+		} else {
+			if (sa_zfs_is_shared(handle, tmplist->path)) {
+				group = sa_get_group(handle, "zfs");
+				if (group == NULL) {
+					group = sa_create_group(handle,
+					    "zfs", &err);
+					if (group == NULL &&
+					    err == SA_NO_PERMISSION) {
+						group = _sa_create_group(
+						    (sa_handle_impl_t)
+						    handle,
+						    "zfs");
+					}
+					if (group != NULL) {
+						(void) sa_create_optionset(
+						    group, tmplist->fstype);
+						(void) sa_set_group_attr(group,
+						    "zfs", "true");
+					}
+				}
+				if (group != NULL) {
+					share = _sa_add_share(group,
+					    tmplist->path, SA_SHARE_TRANSIENT,
+					    &err);
+				}
+			} else {
+				share = _sa_add_share(lgroup, tmplist->path,
+				    SA_SHARE_TRANSIENT, &err);
 			}
-		    } else {
-			share = _sa_add_share(lgroup, tmplist->path,
-						SA_SHARE_TRANSIENT, &err);
-		    }
 		}
 		if (share == NULL)
-		    (void) printf(dgettext(TEXT_DOMAIN,
-					    "Problem with transient: %s\n"),
-				    sa_errorstr(err));
+			(void) printf(dgettext(TEXT_DOMAIN,
+			    "Problem with transient: %s\n"), sa_errorstr(err));
 		if (share != NULL)
-		    set_node_attr(share, "shared", "true");
-
+			set_node_attr(share, "shared", "true");
 		if (err == SA_OK) {
-		    if (tmplist->options != NULL &&
-			strlen(tmplist->options) > 0) {
-			(void) sa_parse_legacy_options(share,
-							tmplist->options,
-							tmplist->fstype);
-		    }
-		    if (tmplist->resource != NULL &&
-			strcmp(tmplist->resource, "-") != 0)
-			set_node_attr(share, "resource", tmplist->resource);
-		    if (tmplist->description != NULL) {
-			xmlNodePtr node;
-			node = xmlNewChild((xmlNodePtr)share, NULL,
-						(xmlChar *)"description", NULL);
-			xmlNodeSetContent(node,
-					    (xmlChar *)tmplist->description);
-		    }
-		    legacy = 1;
+			if (tmplist->options != NULL &&
+			    strlen(tmplist->options) > 0) {
+				(void) sa_parse_legacy_options(share,
+				    tmplist->options, tmplist->fstype);
+			}
+			if (tmplist->resource != NULL &&
+			    strcmp(tmplist->resource, "-") != 0)
+				set_node_attr(share, "resource",
+				    tmplist->resource);
+			if (tmplist->description != NULL) {
+				xmlNodePtr node;
+				node = xmlNewChild((xmlNodePtr)share, NULL,
+				    (xmlChar *)"description", NULL);
+				xmlNodeSetContent(node,
+				    (xmlChar *)tmplist->description);
+			}
+			legacy = 1;
 		}
-	    } else {
-		/*
-		 * if this is a legacy share, mark as shared so we
-		 * only update sharetab appropriately. We also keep
-		 * the sharetab options in order to display for legacy
-		 * share with no arguments.
-		 */
-		set_node_attr(share, "shared", "true");
-		set_node_attr(share, "shareopts", tmplist->options);
-	    }
 	}
 	dfs_free_list(list);
 	return (legacy);
 }
 
 /*
- * get the transient shares from the sharetab (or other) file.  since
+ * Get the transient shares from the sharetab (or other) file.  since
  * these are transient, they only appear in the working file and not
  * in a repository.
  */
@@ -1491,11 +1519,10 @@ gettransients(sa_handle_impl_t ihandle, xmlNodePtr *root)
 	int legacy = 0;
 
 	if (root != NULL) {
-	    if (*root == NULL)
-		*root = xmlNewNode(NULL, (xmlChar *)"sharecfg");
-	    if (*root != NULL) {
-		legacy = parse_sharetab(ihandle);
-	    }
+		if (*root == NULL)
+			*root = xmlNewNode(NULL, (xmlChar *)"sharecfg");
+		if (*root != NULL)
+			legacy = parse_sharetab(ihandle);
 	}
 	return (legacy);
 }
@@ -1514,13 +1541,13 @@ sa_has_prop(sa_optionset_t optionset, sa_property_t prop)
 	int result = 0;
 
 	if (optionset != NULL) {
-	    name = sa_get_property_attr(prop, "type");
-	    if (name != NULL) {
-		otherprop = sa_get_property(optionset, name);
-		if (otherprop != NULL)
-		    result = 1;
-		sa_free_attr_string(name);
-	    }
+		name = sa_get_property_attr(prop, "type");
+		if (name != NULL) {
+			otherprop = sa_get_property(optionset, name);
+			if (otherprop != NULL)
+				result = 1;
+			sa_free_attr_string(name);
+		}
 	}
 	return (result);
 }
@@ -1557,7 +1584,7 @@ sa_valid_property(void *object, char *proto, sa_property_t property)
 	int ret = SA_OK;
 
 	if (proto != NULL && property != NULL) {
-	    ret = sa_proto_valid_prop(proto, property, object);
+		ret = sa_proto_valid_prop(proto, property, object);
 	}
 
 	return (ret);
@@ -1577,15 +1604,17 @@ sa_fstype(char *path)
 	struct stat st;
 
 	err = stat(path, &st);
-	if (err < 0) {
-	    err = SA_NO_SUCH_PATH;
-	} else {
-	    err = SA_OK;
-	}
-	if (err == SA_OK) {
-		/* have a valid path at this point */
-	    return (strdup(st.st_fstype));
-	}
+	if (err < 0)
+		err = SA_NO_SUCH_PATH;
+	else
+		err = SA_OK;
+
+	/*
+	 * If we have a valid path at this point ret, return the fstype.
+	 */
+	if (err == SA_OK)
+		return (strdup(st.st_fstype));
+
 	return (NULL);
 }
 
@@ -1615,56 +1644,61 @@ sa_get_derived_optionset(void *object, char *proto, int hier)
 
 	if (hier &&
 	    (group = sa_get_parent_group((sa_share_t)object)) != NULL) {
-	    newoptionset = sa_get_derived_optionset((void *)group, proto, hier);
+		newoptionset = sa_get_derived_optionset((void *)group, proto,
+		    hier);
 	} else {
-	    newoptionset = (sa_optionset_t)xmlNewNode(NULL,
-						    (xmlChar *)"optionset");
-	    if (newoptionset != NULL) {
-		sa_set_optionset_attr(newoptionset, "type", proto);
-	    }
+		newoptionset = (sa_optionset_t)xmlNewNode(NULL,
+		    (xmlChar *)"optionset");
+		if (newoptionset != NULL) {
+			sa_set_optionset_attr(newoptionset, "type", proto);
+		}
 	}
-	/* dont' do anything if memory wasn't allocated */
+	/* Dont' do anything if memory wasn't allocated */
 	if (newoptionset == NULL)
-	    return (NULL);
+		return (NULL);
 
-	/* found the top so working back down the stack */
+	/* Found the top so working back down the stack */
 	optionset = sa_get_optionset((sa_optionset_t)object, proto);
 	if (optionset != NULL) {
-	    sa_property_t prop;
-	    /* add optionset to the newoptionset */
-	    for (prop = sa_get_property(optionset, NULL);
-		prop != NULL; prop = sa_get_next_property(prop)) {
-		sa_property_t newprop;
-		char *name;
-		char *value;
-		name = sa_get_property_attr(prop, "type");
-		value = sa_get_property_attr(prop, "value");
-		if (name != NULL) {
-		    newprop = sa_get_property(newoptionset, name);
-		    /* replace the value with the new value */
-		    if (newprop != NULL) {
-			/*
-			 * only set if value is non NULL, old value ok
-			 * if it is NULL.
-			 */
-			if (value != NULL)
-			    set_node_attr(newprop, "value", value);
-		    } else {
-			/* an entirely new property */
-			if (value != NULL) {
-			    newprop = sa_create_property(name, value);
-			    if (newprop != NULL) {
-				newprop = (sa_property_t)
-				    xmlAddChild((xmlNodePtr)newoptionset,
-						(xmlNodePtr)newprop);
-			    }
+		sa_property_t prop;
+		/* add optionset to the newoptionset */
+		for (prop = sa_get_property(optionset, NULL);
+		    prop != NULL;
+		    prop = sa_get_next_property(prop)) {
+			sa_property_t newprop;
+			char *name;
+			char *value;
+			name = sa_get_property_attr(prop, "type");
+			value = sa_get_property_attr(prop, "value");
+			if (name == NULL)
+				continue;
+			newprop = sa_get_property(newoptionset, name);
+			/* Replace the value with the new value */
+			if (newprop != NULL) {
+				/*
+				 * Only set if value is non NULL, old value ok
+				 * if it is NULL.
+				 */
+				if (value != NULL)
+					set_node_attr(newprop, "value", value);
+			} else {
+				/* an entirely new property */
+				if (value != NULL) {
+					newprop = sa_create_property(name,
+					    value);
+					if (newprop != NULL) {
+						newprop = (sa_property_t)
+						    xmlAddChild(
+						    (xmlNodePtr)newoptionset,
+						    (xmlNodePtr)newprop);
+					}
+				}
 			}
-		    }
-		    sa_free_attr_string(name);
+			sa_free_attr_string(name);
+
+			if (value != NULL)
+				sa_free_attr_string(value);
 		}
-		if (value != NULL)
-		    sa_free_attr_string(value);
-	    }
 	}
 	return (newoptionset);
 }
@@ -1672,17 +1706,17 @@ sa_get_derived_optionset(void *object, char *proto, int hier)
 void
 sa_free_derived_optionset(sa_optionset_t optionset)
 {
-	/* while it shouldn't be linked, it doesn't hurt */
+	/* While it shouldn't be linked, it doesn't hurt */
 	if (optionset != NULL) {
-	    xmlUnlinkNode((xmlNodePtr) optionset);
-	    xmlFreeNode((xmlNodePtr) optionset);
+		xmlUnlinkNode((xmlNodePtr) optionset);
+		xmlFreeNode((xmlNodePtr) optionset);
 	}
 }
 
 /*
  *  sa_get_all_security_types(object, proto, hier)
  *
- *	find all the security types set for this object.  This is
+ *	Find all the security types set for this object.  This is
  *	preliminary to getting a derived security set. The return value is an
  *	optionset containg properties which are the sectype values found by
  *	walking up the XML document struture. The returned optionset
@@ -1701,45 +1735,50 @@ sa_get_all_security_types(void *object, char *proto, int hier)
 	options = NULL;
 
 	if (hier &&
-	    (group = sa_get_parent_group((sa_share_t)object)) != NULL) {
-	    options = sa_get_all_security_types((void *)group, proto, hier);
-	} else {
-	    options = (sa_optionset_t)xmlNewNode(NULL,
-						    (xmlChar *)"optionset");
-	}
-	/* hit the top so collect the security types working back */
-	if (options != NULL) {
-	    for (security = sa_get_security((sa_group_t)object, NULL, NULL);
-		security != NULL; security = sa_get_next_security(security)) {
+	    (group = sa_get_parent_group((sa_share_t)object)) != NULL)
+		options = sa_get_all_security_types((void *)group, proto, hier);
+	else
+		options = (sa_optionset_t)xmlNewNode(NULL,
+		    (xmlChar *)"optionset");
+
+	if (options == NULL)
+		return (options);
+
+	/* Hit the top so collect the security types working back. */
+	for (security = sa_get_security((sa_group_t)object, NULL, NULL);
+	    security != NULL;
+	    security = sa_get_next_security(security)) {
 		char *type;
 		char *sectype;
 
 		type = sa_get_security_attr(security, "type");
 		if (type != NULL) {
-		    if (strcmp(type, proto) != 0) {
-			sa_free_attr_string(type);
-			continue;
-		    }
-		    sectype = sa_get_security_attr(security, "sectype");
-		    if (sectype != NULL) {
-			/*
-			 * have a security type, check to see if
-			 * already present in optionset and add if it
-			 * isn't.
-			 */
-			if (sa_get_property(options, sectype) == NULL) {
-			    prop = sa_create_property(sectype, "true");
-			    if (prop != NULL)
-				prop = (sa_property_t)
-				    xmlAddChild((xmlNodePtr)options,
-						(xmlNodePtr)prop);
+			if (strcmp(type, proto) != 0) {
+				sa_free_attr_string(type);
+				continue;
+			}
+			sectype = sa_get_security_attr(security, "sectype");
+			if (sectype != NULL) {
+				/*
+				 * Have a security type, check to see if
+				 * already present in optionset and add if it
+				 * isn't.
+				 */
+				if (sa_get_property(options, sectype) == NULL) {
+					prop = sa_create_property(sectype,
+					    "true");
+					if (prop != NULL)
+						prop = (sa_property_t)
+						    xmlAddChild(
+						    (xmlNodePtr)options,
+						    (xmlNodePtr)prop);
+				}
+				sa_free_attr_string(sectype);
 			}
-			sa_free_attr_string(sectype);
-		    }
-		    sa_free_attr_string(type);
+			sa_free_attr_string(type);
 		}
-	    }
 	}
+
 	return (options);
 }
 
@@ -1758,59 +1797,61 @@ sa_get_derived_security(void *object, char *sectype, char *proto, int hier)
 	sa_security_t newsecurity;
 	sa_security_t security;
 	sa_group_t group;
+	sa_property_t prop;
 
 	if (hier &&
 	    (group = sa_get_parent_group((sa_share_t)object)) != NULL) {
-	    newsecurity = sa_get_derived_security((void *)group,
-						    sectype, proto, hier);
+		newsecurity = sa_get_derived_security((void *)group,
+		    sectype, proto, hier);
 	} else {
-	    newsecurity = (sa_security_t)xmlNewNode(NULL,
-						    (xmlChar *)"security");
-	    if (newsecurity != NULL) {
-		sa_set_security_attr(newsecurity, "type", proto);
-		sa_set_security_attr(newsecurity, "sectype", sectype);
-	    }
+		newsecurity = (sa_security_t)xmlNewNode(NULL,
+		    (xmlChar *)"security");
+		if (newsecurity != NULL) {
+			sa_set_security_attr(newsecurity, "type", proto);
+			sa_set_security_attr(newsecurity, "sectype", sectype);
+		}
 	}
-	/* dont' do anything if memory wasn't allocated */
+	/* Don't do anything if memory wasn't allocated */
 	if (newsecurity == NULL)
-	    return (NULL);
+		return (newsecurity);
 
-	/* found the top so working back down the stack */
+	/* Found the top so working back down the stack. */
 	security = sa_get_security((sa_security_t)object, sectype, proto);
-	if (security != NULL) {
-	    sa_property_t prop;
-	    /* add security to the newsecurity */
-	    for (prop = sa_get_property(security, NULL);
-		prop != NULL; prop = sa_get_next_property(prop)) {
+	if (security == NULL)
+		return (newsecurity);
+
+	/* add security to the newsecurity */
+	for (prop = sa_get_property(security, NULL);
+	    prop != NULL; prop = sa_get_next_property(prop)) {
 		sa_property_t newprop;
 		char *name;
 		char *value;
 		name = sa_get_property_attr(prop, "type");
 		value = sa_get_property_attr(prop, "value");
 		if (name != NULL) {
-		    newprop = sa_get_property(newsecurity, name);
-		    /* replace the value with the new value */
-		    if (newprop != NULL) {
-			/*
-			 * only set if value is non NULL, old value ok
-			 * if it is NULL.
-			 */
-			if (value != NULL)
-			    set_node_attr(newprop, name, value);
-		    } else {
-			/* an entirely new property */
-			if (value != NULL) {
-			    newprop = sa_create_property(name, value);
-			    newprop = (sa_property_t)
-				xmlAddChild((xmlNodePtr)newsecurity,
+			newprop = sa_get_property(newsecurity, name);
+			/* Replace the value with the new value */
+			if (newprop != NULL) {
+				/*
+				 * Only set if value is non NULL, old value ok
+				 * if it is NULL.
+				 */
+				if (value != NULL)
+					set_node_attr(newprop, name, value);
+			} else {
+				/* An entirely new property */
+				if (value != NULL) {
+					newprop = sa_create_property(name,
+					    value);
+					newprop = (sa_property_t)
+					    xmlAddChild((xmlNodePtr)newsecurity,
 					    (xmlNodePtr)newprop);
+				}
 			}
-		    }
-		    sa_free_attr_string(name);
+			sa_free_attr_string(name);
 		}
 		if (value != NULL)
-		    sa_free_attr_string(value);
-	    }
+			sa_free_attr_string(value);
 	}
 	return (newsecurity);
 }
@@ -1820,15 +1861,15 @@ sa_free_derived_security(sa_security_t security)
 {
 	/* while it shouldn't be linked, it doesn't hurt */
 	if (security != NULL) {
-	    xmlUnlinkNode((xmlNodePtr)security);
-	    xmlFreeNode((xmlNodePtr)security);
+		xmlUnlinkNode((xmlNodePtr)security);
+		xmlFreeNode((xmlNodePtr)security);
 	}
 }
 
 /*
  * sharetab utility functions
  *
- * makes use of the original sharetab.c from fs.d/nfs/lib
+ * Makes use of the original sharetab.c from fs.d/nfs/lib
  */
 
 /*
@@ -1847,72 +1888,74 @@ sa_fillshare(sa_share_t share, char *proto, struct share *sh)
 
 	group = sa_get_parent_group(share);
 	if (group != NULL) {
-	    zfs = sa_get_group_attr(group, "zfs");
-	    groupname = sa_get_group_attr(group, "name");
+		zfs = sa_get_group_attr(group, "zfs");
+		groupname = sa_get_group_attr(group, "name");
 
-	    if (groupname != NULL &&
-		(strcmp(groupname, "default") == 0 || zfs != NULL)) {
-		/*
-		 * since the groupname is either "default" or the
-		 * group is a ZFS group, we don't want to keep
-		 * groupname. We do want it if it is any other type of
-		 * group.
-		 */
-		sa_free_attr_string(groupname);
-		groupname = NULL;
-	    }
-	    if (zfs != NULL)
-		sa_free_attr_string(zfs);
+		if (groupname != NULL &&
+		    (strcmp(groupname, "default") == 0 || zfs != NULL)) {
+			/*
+			 * since the groupname is either "default" or the
+			 * group is a ZFS group, we don't want to keep
+			 * groupname. We do want it if it is any other type of
+			 * group.
+			 */
+			sa_free_attr_string(groupname);
+			groupname = NULL;
+		}
+		if (zfs != NULL)
+			sa_free_attr_string(zfs);
 	}
 
 	value = sa_get_share_attr(share, "path");
 	if (value != NULL) {
-	    sh->sh_path = strdup(value);
-	    sa_free_attr_string(value);
+		sh->sh_path = strdup(value);
+		sa_free_attr_string(value);
 	}
 
 	value = sa_get_share_attr(share, "resource");
 	if (value != NULL || groupname != NULL) {
-	    int len = 0;
-
-	    if (value != NULL)
-		len += strlen(value);
-	    if (groupname != NULL)
-		len += strlen(groupname);
-	    len += 3; /* worst case */
-	    buff = malloc(len);
-	    (void) snprintf(buff, len, "%s%s%s",
+		int len = 0;
+
+		if (value != NULL)
+			len += strlen(value);
+		if (groupname != NULL)
+			len += strlen(groupname);
+		len += 3; /* worst case */
+		buff = malloc(len);
+		(void) snprintf(buff, len, "%s%s%s",
 		    (value != NULL && strlen(value) > 0) ? value : "-",
 		    groupname != NULL ? "@" : "",
 		    groupname != NULL ? groupname : "");
-	    sh->sh_res = buff;
-	    if (value != NULL)
-		sa_free_attr_string(value);
-	    if (groupname != NULL) {
-		sa_free_attr_string(groupname);
-		groupname = NULL;
-	    }
+		sh->sh_res = buff;
+		if (value != NULL)
+			sa_free_attr_string(value);
+		if (groupname != NULL) {
+			sa_free_attr_string(groupname);
+			groupname = NULL;
+		}
 	} else {
-	    sh->sh_res = strdup("-");
+		sh->sh_res = strdup("-");
 	}
 
 	sh->sh_fstype = strdup(proto);
 	value = sa_proto_legacy_format(proto, share, 1);
 	if (value != NULL) {
-	    if (strlen(value) > 0)
-		sh->sh_opts = strdup(value);
-	    else
+		if (strlen(value) > 0)
+			sh->sh_opts = strdup(value);
+		else
+			sh->sh_opts = strdup("rw");
+		free(value);
+	} else {
 		sh->sh_opts = strdup("rw");
-	    free(value);
-	} else
-	    sh->sh_opts = strdup("rw");
+	}
 
 	value = sa_get_share_description(share);
 	if (value != NULL) {
-	    sh->sh_descr = strdup(value);
-	    sa_free_share_description(value);
-	} else
-	    sh->sh_descr = strdup("");
+		sh->sh_descr = strdup(value);
+		sa_free_share_description(value);
+	} else {
+		sh->sh_descr = strdup("");
+	}
 }
 
 /*
@@ -1925,19 +1968,19 @@ void
 sa_emptyshare(struct share *sh)
 {
 	if (sh->sh_path != NULL)
-	    free(sh->sh_path);
+		free(sh->sh_path);
 	sh->sh_path = NULL;
 	if (sh->sh_res != NULL)
-	    free(sh->sh_res);
+		free(sh->sh_res);
 	sh->sh_res = NULL;
 	if (sh->sh_fstype != NULL)
-	    free(sh->sh_fstype);
+		free(sh->sh_fstype);
 	sh->sh_fstype = NULL;
 	if (sh->sh_opts != NULL)
-	    free(sh->sh_opts);
+		free(sh->sh_opts);
 	sh->sh_opts = NULL;
 	if (sh->sh_descr != NULL)
-	    free(sh->sh_descr);
+		free(sh->sh_descr);
 	sh->sh_descr = NULL;
 }
 
diff --git a/usr/src/lib/libshare/common/plugin.c b/usr/src/lib/libshare/common/plugin.c
index 83bc4d5d94..4079e24ff7 100644
--- a/usr/src/lib/libshare/common/plugin.c
+++ b/usr/src/lib/libshare/common/plugin.c
@@ -81,83 +81,90 @@ proto_plugin_init()
 	struct stat st;
 
 	/*
-	 * should walk "/usr/lib/fs/" for files of the form:
+	 * Should walk "/usr/lib/fs/" for files of the form:
 	 * libshare_*.so
 	 */
 	dir = opendir(SA_LIB_DIR);
 	if (dir != NULL) {
-	    while (ret == SA_OK && (dent = readdir(dir)) != NULL) {
-		char path[MAXPATHLEN];
-		char isa[MAXISALEN];
+		while (ret == SA_OK && (dent = readdir(dir)) != NULL) {
+			char path[MAXPATHLEN];
+			char isa[MAXISALEN];
 
 #if defined(_LP64)
-		if (sysinfo(SI_ARCHITECTURE_64, isa, MAXISALEN) == -1)
-		    isa[0] = '\0';
+			if (sysinfo(SI_ARCHITECTURE_64, isa, MAXISALEN) == -1)
+				isa[0] = '\0';
 #else
-		isa[0] = '\0';
+			isa[0] = '\0';
 #endif
-		(void) snprintf(path, MAXPATHLEN,
-				"%s/%s/%s/libshare_%s.so.1",
-				SA_LIB_DIR,
-				dent->d_name,
-				isa,
-				dent->d_name);
-			if (stat(path, &st) < 0) {
-		    /* file doesn't exist, so don't try to map it */
-		    continue;
-		}
-		dlhandle = dlopen(path, RTLD_FIRST|RTLD_LAZY);
-		if (dlhandle != NULL) {
-		    plugin_ops = (struct sa_plugin_ops *)
-					dlsym(dlhandle,	"sa_plugin_ops");
-		    proto = (struct sa_proto_plugin *)
-			calloc(1, sizeof (struct sa_proto_plugin));
-		    if (proto != NULL) {
-			proto->plugin_ops = plugin_ops;
-			proto->plugin_handle = dlhandle;
-			num_protos++;
-			proto->plugin_next = sap_proto_list;
-			sap_proto_list = proto;
-		    } else {
-			ret = SA_NO_MEMORY;
-		    }
-		} else {
-		    (void) fprintf(stderr,
-			    dgettext(TEXT_DOMAIN,
+			(void) snprintf(path, MAXPATHLEN,
+			    "%s/%s/%s/libshare_%s.so.1", SA_LIB_DIR,
+			    dent->d_name, isa, dent->d_name);
+			/*
+			 * If file doesn't exist, don't try to map it
+			 */
+			if (stat(path, &st) < 0)
+				continue;
+
+			dlhandle = dlopen(path, RTLD_FIRST|RTLD_LAZY);
+			if (dlhandle != NULL) {
+				plugin_ops = (struct sa_plugin_ops *)
+				    dlsym(dlhandle, "sa_plugin_ops");
+				proto = (struct sa_proto_plugin *)
+				    calloc(1, sizeof (struct sa_proto_plugin));
+				if (proto != NULL) {
+					proto->plugin_ops = plugin_ops;
+					proto->plugin_handle = dlhandle;
+					num_protos++;
+					proto->plugin_next = sap_proto_list;
+					sap_proto_list = proto;
+				} else {
+					ret = SA_NO_MEMORY;
+				}
+			} else {
+				(void) fprintf(stderr,
+				    dgettext(TEXT_DOMAIN,
 				    "Error in plugin for protocol %s: %s\n"),
-			    dent->d_name, dlerror());
+				    dent->d_name, dlerror());
+			}
 		}
-	    }
-	    (void) closedir(dir);
+		(void) closedir(dir);
 	}
 	if (ret == SA_OK) {
-	    sa_proto_handle.sa_proto =
-			(char **)calloc(num_protos, sizeof (char *));
-	    sa_proto_handle.sa_ops =
-			(struct sa_plugin_ops **)calloc(num_protos,
-					    sizeof (struct sa_plugin_ops *));
-	    if (sa_proto_handle.sa_proto != NULL &&
-		sa_proto_handle.sa_ops != NULL) {
-		int i;
-		struct sa_proto_plugin *tmp;
-		for (i = 0, tmp = sap_proto_list; i < num_protos;
-		    tmp = tmp->plugin_next) {
-		    err = 0;
-		    if (tmp->plugin_ops->sa_init != NULL)
-			err = tmp->plugin_ops->sa_init();
-		    if (err == SA_OK) {
-			/* only include if the init succeeded or was NULL */
-			sa_proto_handle.sa_num_proto++;
-			sa_proto_handle.sa_ops[i] = tmp->plugin_ops;
-			sa_proto_handle.sa_proto[i] =
-					tmp->plugin_ops->sa_protocol;
-			i++;
-		    }
+		sa_proto_handle.sa_proto =
+		    (char **)calloc(num_protos, sizeof (char *));
+		sa_proto_handle.sa_ops =
+		    (struct sa_plugin_ops **)calloc(num_protos,
+		    sizeof (struct sa_plugin_ops *));
+		if (sa_proto_handle.sa_proto != NULL &&
+		    sa_proto_handle.sa_ops != NULL) {
+			int i;
+			struct sa_proto_plugin *tmp;
+
+			for (i = 0, tmp = sap_proto_list;
+			    i < num_protos;
+			    tmp = tmp->plugin_next) {
+				err = 0;
+				if (tmp->plugin_ops->sa_init != NULL)
+					err = tmp->plugin_ops->sa_init();
+				if (err == SA_OK) {
+					/*
+					 * Only include if the init
+					 * succeeded or was NULL
+					 */
+					sa_proto_handle.sa_num_proto++;
+					sa_proto_handle.sa_ops[i] =
+					    tmp->plugin_ops;
+					sa_proto_handle.sa_proto[i] =
+					    tmp->plugin_ops->sa_protocol;
+					i++;
+				}
+			}
 		}
-	    }
 	} else {
-	    /* there was an error, so cleanup prior to return of failure. */
-	    proto_plugin_fini();
+		/*
+		 * There was an error, so cleanup prior to return of failure.
+		 */
+		proto_plugin_fini();
 	}
 	return (ret);
 }
@@ -165,32 +172,33 @@ proto_plugin_init()
 /*
  * proto_plugin_fini()
  *
- * uninitialize all the plugin modules.
+ * Uninitialize all the plugin modules.
  */
 
 void
 proto_plugin_fini()
 {
 	/*
-	 * free up all the protocols, calling their fini, if there is
+	 * Free up all the protocols, calling their fini, if there is
 	 * one.
 	 */
 	while (sap_proto_list != NULL) {
-	    struct sa_proto_plugin *next;
-	    next = sap_proto_list->plugin_next;
-	    sap_proto_list->plugin_ops->sa_fini();
-	    if (sap_proto_list->plugin_handle != NULL)
-		(void) dlclose(sap_proto_list->plugin_handle);
-	    free(sap_proto_list);
-	    sap_proto_list = next;
+		struct sa_proto_plugin *next;
+
+		next = sap_proto_list->plugin_next;
+		sap_proto_list->plugin_ops->sa_fini();
+		if (sap_proto_list->plugin_handle != NULL)
+			(void) dlclose(sap_proto_list->plugin_handle);
+		free(sap_proto_list);
+		sap_proto_list = next;
 	}
 	if (sa_proto_handle.sa_ops != NULL) {
-	    free(sa_proto_handle.sa_ops);
-	    sa_proto_handle.sa_ops = NULL;
+		free(sa_proto_handle.sa_ops);
+		sa_proto_handle.sa_ops = NULL;
 	}
 	if (sa_proto_handle.sa_proto != NULL) {
-	    free(sa_proto_handle.sa_proto);
-	    sa_proto_handle.sa_proto = NULL;
+		free(sa_proto_handle.sa_proto);
+		sa_proto_handle.sa_proto = NULL;
 	}
 	sa_proto_handle.sa_num_proto = 0;
 }
@@ -208,10 +216,10 @@ find_protocol(char *proto)
 	int i;
 
 	if (proto != NULL) {
-	    for (i = 0; i < sa_proto_handle.sa_num_proto; i++) {
-		if (strcmp(proto, sa_proto_handle.sa_proto[i]) == 0)
-		    return (sa_proto_handle.sa_ops[i]);
-	    }
+		for (i = 0; i < sa_proto_handle.sa_num_proto; i++) {
+			if (strcmp(proto, sa_proto_handle.sa_proto[i]) == 0)
+				return (sa_proto_handle.sa_ops[i]);
+		}
 	}
 	return (NULL);
 }
@@ -229,7 +237,7 @@ sa_proto_share(char *proto, sa_share_t share)
 	int ret = SA_INVALID_PROTOCOL;
 
 	if (ops != NULL && ops->sa_share != NULL)
-	    ret = ops->sa_share(share);
+		ret = ops->sa_share(share);
 	return (ret);
 }
 
@@ -246,14 +254,14 @@ sa_proto_unshare(sa_share_t share, char *proto, char *path)
 	int ret = SA_INVALID_PROTOCOL;
 
 	if (ops != NULL && ops->sa_unshare != NULL)
-	    ret = ops->sa_unshare(share, path);
+		ret = ops->sa_unshare(share, path);
 	return (ret);
 }
 
 /*
  * sa_proto_valid_prop(proto, prop, opt)
  *
- * check to see if the specified prop is valid for this protocol.
+ * Check to see if the specified prop is valid for this protocol.
  */
 
 int
@@ -263,14 +271,14 @@ sa_proto_valid_prop(char *proto, sa_property_t prop, sa_optionset_t opt)
 	int ret = 0;
 
 	if (ops != NULL && ops->sa_valid_prop != NULL)
-	    ret = ops->sa_valid_prop(prop, opt);
+		ret = ops->sa_valid_prop(prop, opt);
 	return (ret);
 }
 
 /*
  * sa_proto_valid_space(proto, space)
  *
- * check if space is valid optionspace for proto.
+ * Check if space is valid optionspace for proto.
  * Protocols that don't implement this don't support spaces.
  */
 int
@@ -280,14 +288,14 @@ sa_proto_valid_space(char *proto, char *token)
 	int ret = 0;
 
 	if (ops != NULL && ops->sa_valid_space != NULL)
-	    ret = ops->sa_valid_space(token);
+		ret = ops->sa_valid_space(token);
 	return (ret);
 }
 
 /*
  * sa_proto_space_alias(proto, space)
  *
- * if the name for space is an alias, return its proper name.  This is
+ * If the name for space is an alias, return its proper name.  This is
  * used to translate "default" values into proper form.
  */
 char *
@@ -297,7 +305,7 @@ sa_proto_space_alias(char *proto, char *space)
 	char *ret = space;
 
 	if (ops != NULL && ops->sa_space_alias != NULL)
-	    ret = ops->sa_space_alias(space);
+		ret = ops->sa_space_alias(space);
 	return (ret);
 }
 
@@ -315,7 +323,7 @@ sa_proto_security_prop(char *proto, char *token)
 	int ret = 0;
 
 	if (ops != NULL && ops->sa_security_prop != NULL)
-	    ret = ops->sa_security_prop(token);
+		ret = ops->sa_security_prop(token);
 	return (ret);
 }
 
@@ -333,7 +341,7 @@ sa_proto_legacy_opts(char *proto, sa_group_t group, char *options)
 	int ret = SA_INVALID_PROTOCOL;
 
 	if (ops != NULL && ops->sa_legacy_opts != NULL)
-	    ret = ops->sa_legacy_opts(group, options);
+		ret = ops->sa_legacy_opts(group, options);
 	return (ret);
 }
 
@@ -351,7 +359,7 @@ sa_proto_legacy_format(char *proto, sa_group_t group, int hier)
 	char *ret = NULL;
 
 	if (ops != NULL && ops->sa_legacy_format != NULL)
-	    ret = ops->sa_legacy_format(group, hier);
+		ret = ops->sa_legacy_format(group, hier);
 	return (ret);
 }
 
@@ -380,7 +388,7 @@ sa_proto_get_properties(char *proto)
 	sa_protocol_properties_t props = NULL;
 
 	if (ops != NULL && ops->sa_get_proto_set != NULL)
-	    props = ops->sa_get_proto_set();
+		props = ops->sa_get_proto_set();
 	return (props);
 }
 
@@ -395,15 +403,16 @@ sa_proto_set_property(char *proto, sa_property_t prop)
 {
 	struct sa_plugin_ops *ops = find_protocol(proto);
 	int ret = SA_OK;
+
 	if (ops != NULL && ops->sa_set_proto_prop != NULL)
-	    ret = ops->sa_set_proto_prop(prop);
+		ret = ops->sa_set_proto_prop(prop);
 	return (ret);
 }
 
 /*
  * sa_valid_protocol(proto)
  *
- * check to see if the protocol specified is defined by a
+ * Check to see if the protocol specified is defined by a
  * plugin. Returns true (1) or false (0)
  */
 
@@ -424,7 +433,7 @@ sa_get_protocol_status(char *proto)
 	struct sa_plugin_ops *ops = find_protocol(proto);
 	char *ret = NULL;
 	if (ops != NULL && ops->sa_get_proto_status != NULL)
-	    ret = ops->sa_get_proto_status(proto);
+		ret = ops->sa_get_proto_status(proto);
 	return (ret);
 }
 
@@ -442,8 +451,8 @@ sa_proto_update_legacy(char *proto, sa_share_t share)
 	int ret = SA_NOT_IMPLEMENTED;
 
 	if (ops != NULL) {
-	    if (ops->sa_update_legacy != NULL)
-		ret = ops->sa_update_legacy(share);
+		if (ops->sa_update_legacy != NULL)
+			ret = ops->sa_update_legacy(share);
 	}
 	return (ret);
 }
@@ -451,7 +460,7 @@ sa_proto_update_legacy(char *proto, sa_share_t share)
 /*
  * sa_delete_legacy(proto, share)
  *
- * remove the specified share from the protocol specific legacy files.
+ * Remove the specified share from the protocol specific legacy files.
  */
 
 int
@@ -461,13 +470,13 @@ sa_proto_delete_legacy(char *proto, sa_share_t share)
 	int ret = SA_OK;
 
 	if (ops != NULL) {
-	    if (ops->sa_delete_legacy != NULL)
-		ret = ops->sa_delete_legacy(share);
+		if (ops->sa_delete_legacy != NULL)
+			ret = ops->sa_delete_legacy(share);
 	} else {
-	    if (proto != NULL)
-		ret = SA_NOT_IMPLEMENTED;
-	    else
-		ret = SA_INVALID_PROTOCOL;
+		if (proto != NULL)
+			ret = SA_NOT_IMPLEMENTED;
+		else
+			ret = SA_INVALID_PROTOCOL;
 	}
 	return (ret);
 }
diff --git a/usr/src/lib/libshare/common/scfutil.c b/usr/src/lib/libshare/common/scfutil.c
index 9aa79ae5c8..05421d605b 100644
--- a/usr/src/lib/libshare/common/scfutil.c
+++ b/usr/src/lib/libshare/common/scfutil.c
@@ -58,7 +58,7 @@ static char *skip_props[] = {
 /*
  * sa_scf_fini(handle)
  *
- * must be called when done. Called with the handle allocated in
+ * Must be called when done. Called with the handle allocated in
  * sa_scf_init(), it cleans up the state and frees any SCF resources
  * still in use. Called by sa_fini().
  */
@@ -67,31 +67,31 @@ void
 sa_scf_fini(scfutilhandle_t *handle)
 {
 	if (handle != NULL) {
-	    int unbind = 0;
-	    if (handle->scope != NULL) {
-		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);
-	    if (handle->pg != NULL)
-		scf_pg_destroy(handle->pg);
-	    if (handle->handle != NULL) {
-		handle->scf_state = SCH_STATE_UNINIT;
-		if (unbind)
-		    (void) scf_handle_unbind(handle->handle);
-		scf_handle_destroy(handle->handle);
-	    }
-	    free(handle);
+		int unbind = 0;
+		if (handle->scope != NULL) {
+			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);
+		if (handle->pg != NULL)
+			scf_pg_destroy(handle->pg);
+		if (handle->handle != NULL) {
+			handle->scf_state = SCH_STATE_UNINIT;
+			if (unbind)
+				(void) scf_handle_unbind(handle->handle);
+			scf_handle_destroy(handle->handle);
+		}
+		free(handle);
 	}
 }
 
 /*
  * sa_scf_init()
  *
- * must be called before using any of the SCF functions. Called by
+ * Must be called before using any of the SCF functions. Called by
  * sa_init() during the API setup.
  */
 
@@ -102,69 +102,65 @@ sa_scf_init(sa_handle_impl_t ihandle)
 
 	scf_max_name_len = scf_limit(SCF_LIMIT_MAX_NAME_LENGTH);
 	if (scf_max_name_len <= 0)
-	    scf_max_name_len = SA_MAX_NAME_LEN + 1;
+		scf_max_name_len = SA_MAX_NAME_LEN + 1;
 
 	handle = calloc(1, sizeof (scfutilhandle_t));
-	if (handle != NULL) {
-	    ihandle->scfhandle = handle;
-	    handle->scf_state = SCH_STATE_INITIALIZING;
-	    handle->handle = scf_handle_create(SCF_VERSION);
-	    if (handle->handle != NULL) {
-		if (scf_handle_bind(handle->handle) == 0) {
-		    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,
-						    SA_GROUP_SVC_NAME,
-						    handle->service) != 0) {
-			    goto err;
-			}
-			handle->scf_state = SCH_STATE_INIT;
-			if (sa_get_instance(handle, "default") != SA_OK) {
-			    char **protolist;
-			    int numprotos, i;
-			    sa_group_t defgrp;
-			    defgrp = sa_create_group((sa_handle_t)ihandle,
-							"default", NULL);
-			    if (defgrp != NULL) {
-				numprotos = sa_get_protocols(&protolist);
-				for (i = 0; i < numprotos; i++) {
-				    (void) sa_create_optionset(defgrp,
-								protolist[i]);
-				}
-				if (protolist != NULL)
-				    free(protolist);
-			    }
-			}
-		    } else {
-			goto err;
-		    }
-		} else {
-		    goto err;
-		}
-	    } else {
+	if (handle == NULL)
+		return (handle);
+
+	ihandle->scfhandle = handle;
+	handle->scf_state = SCH_STATE_INITIALIZING;
+	handle->handle = scf_handle_create(SCF_VERSION);
+	if (handle->handle == NULL) {
 		free(handle);
 		handle = NULL;
 		(void) printf("libshare could not access SMF repository: %s\n",
-				scf_strerror(scf_error()));
-	    }
+		    scf_strerror(scf_error()));
+		return (handle);
 	}
+	if (scf_handle_bind(handle->handle) != 0)
+		goto err;
+
+	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)
+		goto err;
+	if (scf_scope_get_service(handle->scope,
+	    SA_GROUP_SVC_NAME, handle->service) != 0)
+		goto err;
+
+	handle->scf_state = SCH_STATE_INIT;
+	if (sa_get_instance(handle, "default") != SA_OK) {
+		char **protolist;
+		int numprotos, i;
+		sa_group_t defgrp;
+		defgrp = sa_create_group((sa_handle_t)ihandle, "default", NULL);
+		if (defgrp != NULL) {
+			numprotos = sa_get_protocols(
+			    &protolist);
+			for (i = 0; i < numprotos; i++)
+				(void) sa_create_optionset(defgrp,
+				    protolist[i]);
+			if (protolist != NULL)
+				free(protolist);
+		}
+	}
+
 	return (handle);
 
-	/* error handling/unwinding */
+	/* Error handling/unwinding */
 err:
 	(void) sa_scf_fini(handle);
 	(void) printf("libshare SMF initialization problem: %s\n",
-			scf_strerror(scf_error()));
+	    scf_strerror(scf_error()));
 	return (NULL);
 }
 
@@ -183,14 +179,14 @@ get_scf_limit(uint32_t name)
 
 	vallen = scf_limit(name);
 	if (vallen == (ssize_t)-1)
-	    vallen = MAXPATHLEN;
+		vallen = MAXPATHLEN;
 	return (vallen);
 }
 
 /*
  * skip_property(name)
  *
- * internal function to check to see if a property is an SMF magic
+ * Internal function to check to see if a property is an SMF magic
  * property that needs to be skipped.
  */
 static int
@@ -199,7 +195,7 @@ skip_property(char *name)
 	int i;
 
 	for (i = 0; skip_props[i] != NULL; i++)
-	    if (strcmp(name, skip_props[i]) == 0)
+		if (strcmp(name, skip_props[i]) == 0)
 		return (1);
 	return (0);
 }
@@ -225,7 +221,7 @@ generate_unique_sharename(char *sharename)
 /*
  * valid_protocol(proto)
  *
- * check to see if the specified protocol is a valid one for the
+ * Check to see if the specified protocol is a valid one for the
  * general sharemgr facility. We determine this by checking which
  * plugin protocols were found.
  */
@@ -236,15 +232,15 @@ valid_protocol(char *proto)
 	struct sa_proto_plugin *plugin;
 	for (plugin = sap_proto_list; plugin != NULL;
 	    plugin = plugin->plugin_next)
-	    if (strcmp(proto, plugin->plugin_ops->sa_protocol) == 0)
-		return (1);
+		if (strcmp(proto, plugin->plugin_ops->sa_protocol) == 0)
+			return (1);
 	return (0);
 }
 
 /*
  * sa_extract_pgroup(root, handle, pg, nodetype, proto, sectype)
  *
- * extract the name property group and create the specified type of
+ * Extract the name property group and create the specified type of
  * node on the provided group.  type will be optionset or security.
  */
 
@@ -265,77 +261,79 @@ sa_extract_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 	vallen = get_scf_limit(SCF_LIMIT_MAX_VALUE_LENGTH);
 
 	node = xmlNewChild(root, NULL, (xmlChar *)nodetype, NULL);
-	if (node != NULL) {
-	    if (proto != NULL)
+	if (node == NULL)
+		return (ret);
+
+	if (proto != NULL)
 		xmlSetProp(node, (xmlChar *)"type", (xmlChar *)proto);
-	    if (sectype != NULL)
+	if (sectype != NULL)
 		xmlSetProp(node, (xmlChar *)"sectype", (xmlChar *)sectype);
-		/*
-		 * have node to work with so iterate over the properties
-		 * in the pg and create option sub nodes.
-		 */
-		iter = scf_iter_create(handle->handle);
-		value = scf_value_create(handle->handle);
-		prop = scf_property_create(handle->handle);
-		name = malloc(scf_max_name_len);
-		valuestr = malloc(vallen);
-		/*
-		 * want to iterate through the properties and add them
-		 * to the base optionset.
-		 */
-		if (iter != NULL && value != NULL && prop != NULL &&
-		    valuestr != NULL && name != NULL) {
-		    if (scf_iter_pg_properties(iter, pg) == 0) {
-			/* now iterate the properties in the group */
-			while (scf_iter_next_property(iter, prop) > 0) {
-			    /* have a property */
-			    if (scf_property_get_name(prop, name,
-							scf_max_name_len) > 0) {
-				/* some properties are part of the framework */
+	/*
+	 * Have node to work with so iterate over the properties
+	 * in the pg and create option sub nodes.
+	 */
+	iter = scf_iter_create(handle->handle);
+	value = scf_value_create(handle->handle);
+	prop = scf_property_create(handle->handle);
+	name = malloc(scf_max_name_len);
+	valuestr = malloc(vallen);
+	/*
+	 * Want to iterate through the properties and add them
+	 * to the base optionset.
+	 */
+	if (iter == NULL || value == NULL || prop == NULL ||
+	    valuestr == NULL || name == NULL) {
+		ret = SA_NO_MEMORY;
+		goto out;
+	}
+	if (scf_iter_pg_properties(iter, pg) == 0) {
+		/* Now iterate the properties in the group */
+		while (scf_iter_next_property(iter, prop) > 0) {
+			/* have a property */
+			if (scf_property_get_name(prop, name,
+			    scf_max_name_len) > 0) {
+				sa_property_t saprop;
+				/* Some properties are part of the framework */
 				if (skip_property(name))
-				    continue;
-				if (scf_property_get_value(prop, value) == 0) {
-				    if (scf_value_get_astring(value, valuestr,
-								vallen) >= 0) {
-					sa_property_t saprop;
-					saprop = sa_create_property(name,
-								    valuestr);
-					if (saprop != NULL) {
+					continue;
+				if (scf_property_get_value(prop, value) != 0)
+					continue;
+				if (scf_value_get_astring(value, valuestr,
+				    vallen) < 0)
+					continue;
+				saprop = sa_create_property(name, valuestr);
+				if (saprop != NULL) {
 					/*
-					 * since in SMF, don't
+					 * Since in SMF, don't
 					 * recurse. Use xmlAddChild
 					 * directly, instead.
 					 */
-					    xmlAddChild(node,
-							(xmlNodePtr) saprop);
-					}
-				    }
+					xmlAddChild(node,
+					    (xmlNodePtr) saprop);
 				}
-			    }
 			}
-		    }
-		} else {
-		    ret = SA_NO_MEMORY;
 		}
-		/* cleanup to avoid memory leaks */
-		if (value != NULL)
-		    scf_value_destroy(value);
-		if (iter != NULL)
-		    scf_iter_destroy(iter);
-		if (prop != NULL)
-		    scf_property_destroy(prop);
-		if (name != NULL)
-		    free(name);
-		if (valuestr != NULL)
-		    free(valuestr);
 	}
+out:
+	/* cleanup to avoid memory leaks */
+	if (value != NULL)
+		scf_value_destroy(value);
+	if (iter != NULL)
+		scf_iter_destroy(iter);
+	if (prop != NULL)
+		scf_property_destroy(prop);
+	if (name != NULL)
+		free(name);
+	if (valuestr != NULL)
+		free(valuestr);
+
 	return (ret);
 }
 
 /*
  * sa_extract_attrs(root, handle, instance)
  *
- * local function to extract the actual attributes/properties from the
+ * Local function to extract the actual attributes/properties from the
  * property group of the service instance. These are the well known
  * attributes of "state" and "zfs". If additional attributes are
  * added, they should be added here.
@@ -354,42 +352,45 @@ sa_extract_attrs(xmlNodePtr root, scfutilhandle_t *handle,
 	prop = scf_property_create(handle->handle);
 	value = scf_value_create(handle->handle);
 	valuestr = malloc(vallen);
-	if (prop != NULL && value != NULL && valuestr != NULL &&
-	    scf_instance_get_pg(instance, "operation",
-				handle->pg) == 0) {
-		/*
-		 * have a property group with desired name so now get
-		 * the known attributes.
-		 */
-	    if (scf_pg_get_property(handle->pg, "state", prop) == 0) {
-		/* found the property so get the value */
+	if (prop == NULL || value == NULL || valuestr == NULL ||
+	    scf_instance_get_pg(instance, "operation", handle->pg) != 0) {
+		goto out;
+	}
+	/*
+	 * Have a property group with desired name so now get
+	 * the known attributes.
+	 */
+	if (scf_pg_get_property(handle->pg, "state", prop) == 0) {
+		/* Found the property so get the value */
 		if (scf_property_get_value(prop, value) == 0) {
-		    if (scf_value_get_astring(value, valuestr, vallen) >= 0) {
-			xmlSetProp(root, (xmlChar *)"state",
+			if (scf_value_get_astring(value, valuestr,
+			    vallen) >= 0) {
+				xmlSetProp(root, (xmlChar *)"state",
 				    (xmlChar *)valuestr);
-		    }
+			}
 		}
-	    }
-	    if (scf_pg_get_property(handle->pg, "zfs", prop) == 0) {
-		/* found the property so get the value */
+	}
+	if (scf_pg_get_property(handle->pg, "zfs", prop) == 0) {
+		/* Found the property so get the value */
 		if (scf_property_get_value(prop, value) == 0) {
-		    if (scf_value_get_astring(value, valuestr, vallen) > 0) {
-			xmlSetProp(root, (xmlChar *)"zfs",
+			if (scf_value_get_astring(value, valuestr,
+			    vallen) > 0) {
+				xmlSetProp(root, (xmlChar *)"zfs",
 				    (xmlChar *)valuestr);
-		    }
+			}
 		}
-	    }
 	}
+out:
 	if (valuestr != NULL)
-	    free(valuestr);
+		free(valuestr);
 	if (value != NULL)
-	    scf_value_destroy(value);
+		scf_value_destroy(value);
 	if (prop != NULL)
-	    scf_property_destroy(prop);
+		scf_property_destroy(prop);
 }
 
 /*
- * list of known share attributes.
+ * List of known share attributes.
  */
 
 static char *share_attr[] = {
@@ -404,15 +405,15 @@ is_share_attr(char *name)
 {
 	int i;
 	for (i = 0; share_attr[i] != NULL; i++)
-	    if (strcmp(name, share_attr[i]) == 0)
-		return (1);
+		if (strcmp(name, share_attr[i]) == 0)
+			return (1);
 	return (0);
 }
 
 /*
  * sa_share_from_pgroup
  *
- * extract the share definition from the share property group. We do
+ * Extract the share definition from the share property group. We do
  * some sanity checking to avoid bad data.
  *
  * Since this is only constructing the internal data structures, we
@@ -440,12 +441,13 @@ sa_share_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 	 */
 	vallen = strlen(id);
 	if (*id == SA_SHARE_PG_PREFIX[0] && vallen == SA_SHARE_PG_LEN) {
-	    uuid_t uuid;
-	    if (strncmp(id, SA_SHARE_PG_PREFIX, SA_SHARE_PG_PREFIXLEN) != 0 ||
-		uuid_parse(id + 2, uuid) < 0)
-		return;
+		uuid_t uuid;
+		if (strncmp(id, SA_SHARE_PG_PREFIX,
+		    SA_SHARE_PG_PREFIXLEN) != 0 ||
+		    uuid_parse(id + 2, uuid) < 0)
+			return;
 	} else {
-	    return;
+		return;
 	}
 
 	vallen = get_scf_limit(SCF_LIMIT_MAX_VALUE_LENGTH);
@@ -457,7 +459,7 @@ sa_share_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 	valuestr = malloc(vallen);
 
 	/*
-	 * construct the share XML node. It is similar to sa_add_share
+	 * Construct the share XML node. It is similar to sa_add_share
 	 * but never changes the repository. Also, there won't be any
 	 * ZFS or transient shares.  Root will be the group it is
 	 * associated with.
@@ -465,81 +467,82 @@ sa_share_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 	node = xmlNewChild(root, NULL, (xmlChar *)"share", NULL);
 	if (node != NULL) {
 		/*
-		 * make sure the UUID part of the property group is
+		 * Make sure the UUID part of the property group is
 		 * stored in the share "id" property. We use this
 		 * later.
 		 */
-	    xmlSetProp(node, (xmlChar *)"id", (xmlChar *)id);
-	    xmlSetProp(node, (xmlChar *)"type", (xmlChar *)"persist");
+		xmlSetProp(node, (xmlChar *)"id", (xmlChar *)id);
+		xmlSetProp(node, (xmlChar *)"type", (xmlChar *)"persist");
 	}
 
-	if (iter != NULL && value != NULL && prop != NULL && name != NULL) {
-		/* iterate over the share pg properties */
-	    if (scf_iter_pg_properties(iter, pg) == 0) {
-		while (scf_iter_next_property(iter, prop) > 0) {
-		    ret = SA_SYSTEM_ERR; /* assume the worst */
-		    if (scf_property_get_name(prop, name,
-						scf_max_name_len) > 0) {
+	if (iter == NULL || value == NULL || prop == NULL || name == NULL)
+		goto out;
+
+	/* Iterate over the share pg properties */
+	if (scf_iter_pg_properties(iter, pg) != 0)
+		goto out;
+
+	while (scf_iter_next_property(iter, prop) > 0) {
+		ret = SA_SYSTEM_ERR; /* assume the worst */
+		if (scf_property_get_name(prop, name, scf_max_name_len) > 0) {
 			if (scf_property_get_value(prop, value) == 0) {
-			    if (scf_value_get_astring(value, valuestr,
-							vallen) >= 0) {
-				ret = SA_OK;
-			    }
+				if (scf_value_get_astring(value, valuestr,
+				    vallen) >= 0) {
+					ret = SA_OK;
+				}
 			}
-		    }
-		    if (ret == SA_OK) {
+		}
+		if (ret == SA_OK) {
 			/*
-			 * check that we have the "path" property in
+			 * Check that we have the "path" property in
 			 * name. The string in name will always be nul
 			 * terminated if scf_property_get_name()
 			 * succeeded.
 			 */
 			if (strcmp(name, "path") == 0)
-			    have_path = 1;
+				have_path = 1;
 			if (is_share_attr(name)) {
 				/*
-				 * if a share attr, then simple -
+				 * If a share attr, then simple -
 				 * usually path and resource name
 				 */
-			    xmlSetProp(node, (xmlChar *)name,
-					(xmlChar *)valuestr);
+				xmlSetProp(node, (xmlChar *)name,
+				    (xmlChar *)valuestr);
 			} else {
-			    if (strcmp(name, "description") == 0) {
-				/* we have a description node */
-				xmlNodePtr desc;
-				desc = xmlNewChild(node, NULL,
-						    (xmlChar *)"description",
-						    NULL);
-				if (desc != NULL)
-				    xmlNodeSetContent(desc,
-							(xmlChar *)valuestr);
-			    }
+				if (strcmp(name, "description") == 0) {
+					/* We have a description node */
+					xmlNodePtr desc;
+					desc = xmlNewChild(node, NULL,
+					    (xmlChar *)"description", NULL);
+					if (desc != NULL)
+						xmlNodeSetContent(desc,
+						    (xmlChar *)valuestr);
+				}
 			}
-		    }
 		}
-	    }
 	}
+out:
 	/*
-	 * a share without a path is broken so we want to not include
+	 * A share without a path is broken so we want to not include
 	 * these.  They shouldn't happen but if you kill a sharemgr in
 	 * the process of creating a share, it could happen.  They
 	 * should be harmless.  It is also possible that another
 	 * sharemgr is running and in the process of creating a share.
 	 */
 	if (have_path == 0 && node != NULL) {
-	    xmlUnlinkNode(node);
-	    xmlFreeNode(node);
+		xmlUnlinkNode(node);
+		xmlFreeNode(node);
 	}
 	if (name != NULL)
-	    free(name);
+		free(name);
 	if (valuestr != NULL)
-	    free(valuestr);
+		free(valuestr);
 	if (value != NULL)
-	    scf_value_destroy(value);
+		scf_value_destroy(value);
 	if (iter != NULL)
-	    scf_iter_destroy(iter);
+		scf_iter_destroy(iter);
 	if (prop != NULL)
-	    scf_property_destroy(prop);
+		scf_property_destroy(prop);
 }
 
 /*
@@ -557,10 +560,12 @@ find_share_by_id(sa_handle_t handle, char *shareid)
 	char *id = NULL;
 	int done = 0;
 
-	for (group = sa_get_group(handle, NULL); group != NULL && !done;
-		group = sa_get_next_group(group)) {
-		for (share = sa_get_share(group, NULL); share != NULL;
-			share = sa_get_next_share(share)) {
+	for (group = sa_get_group(handle, NULL);
+	    group != NULL && !done;
+	    group = sa_get_next_group(group)) {
+		for (share = sa_get_share(group, NULL);
+		    share != NULL;
+		    share = sa_get_next_share(share)) {
 			id = sa_get_share_attr(share, "id");
 			if (id != NULL && strcmp(id, shareid) == 0) {
 				sa_free_attr_string(id);
@@ -569,8 +574,8 @@ find_share_by_id(sa_handle_t handle, char *shareid)
 				break;
 			}
 			if (id != NULL) {
-			    sa_free_attr_string(id);
-			    id = NULL;
+				sa_free_attr_string(id);
+				id = NULL;
 			}
 		}
 	}
@@ -580,7 +585,7 @@ find_share_by_id(sa_handle_t handle, char *shareid)
 /*
  * sa_share_props_from_pgroup(root, handle, pg, id)
  *
- * extract share properties from the SMF property group. More sanity
+ * Extract share properties from the SMF property group. More sanity
  * checks are done and the share object is created. We ignore some
  * errors that could exist in the repository and only worry about
  * property groups that validate in naming.
@@ -591,16 +596,17 @@ sa_share_props_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 			scf_propertygroup_t *pg, char *id, sa_handle_t sahandle)
 {
 	xmlNodePtr node;
-	char *name;
-	scf_iter_t *iter;
-	scf_property_t *prop;
-	scf_value_t *value;
+	char *name = NULL;
+	scf_iter_t *iter = NULL;
+	scf_property_t *prop = NULL;
+	scf_value_t *value = NULL;
 	ssize_t vallen;
-	char *valuestr;
+	char *valuestr = NULL;
 	int ret = SA_OK;
 	char *sectype = NULL;
 	char *proto;
 	sa_share_t share;
+	uuid_t uuid;
 
 	/*
 	 * While preliminary check (starts with 'S') passed before
@@ -611,39 +617,37 @@ sa_share_props_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 	 * versions.
 	 */
 	vallen = strlen(id);
-	if (*id == SA_SHARE_PG_PREFIX[0] && vallen > SA_SHARE_PG_LEN) {
-	    uuid_t uuid;
-	    if (strncmp(id, SA_SHARE_PG_PREFIX, SA_SHARE_PG_PREFIXLEN) == 0) {
+	if (*id != SA_SHARE_PG_PREFIX[0] || vallen <= SA_SHARE_PG_LEN) {
+		/*
+		 * It is ok to not have what we thought since someone might
+		 * have added a name via SMF.
+		 */
+		return (ret);
+	}
+	if (strncmp(id, SA_SHARE_PG_PREFIX, SA_SHARE_PG_PREFIXLEN) == 0) {
 		proto = strchr(id, '_');
 		if (proto == NULL)
-		    return (ret);
+			return (ret);
 		*proto++ = '\0';
 		if (uuid_parse(id + SA_SHARE_PG_PREFIXLEN, uuid) < 0)
-		    return (ret);
+			return (ret);
 		/*
 		 * probably a legal optionset so check a few more
 		 * syntax points below.
 		 */
 		if (*proto == '\0') {
-		    /* not a valid proto (null) */
-		    return (ret);
+			/* not a valid proto (null) */
+			return (ret);
 		}
 		sectype = strchr(proto, '_');
 		if (sectype != NULL)
-		    *sectype++ = '\0';
+			*sectype++ = '\0';
 		if (!valid_protocol(proto))
-		    return (ret);
-	    }
-	} else {
-	/*
-	 * it is ok to not have what we thought since someone might
-	 * have added a name via SMF.
-	 */
-	    return (ret);
+			return (ret);
 	}
 
 	/*
-	 * to get here, we have a valid protocol and possibly a
+	 * To get here, we have a valid protocol and possibly a
 	 * security. We now have to find the share that it is really
 	 * associated with. The "id" portion of the pgroup name will
 	 * match.
@@ -651,78 +655,82 @@ sa_share_props_from_pgroup(xmlNodePtr root, scfutilhandle_t *handle,
 
 	share = find_share_by_id(sahandle, id);
 	if (share == NULL)
-	    return (SA_BAD_PATH);
+		return (SA_BAD_PATH);
 
 	root = (xmlNodePtr)share;
 
 	vallen = get_scf_limit(SCF_LIMIT_MAX_VALUE_LENGTH);
 
+	if (sectype == NULL)
+		node = xmlNewChild(root, NULL, (xmlChar *)"optionset", NULL);
+	else {
+		node = xmlNewChild(root, NULL, (xmlChar *)"security", NULL);
+		if (node != NULL)
+			xmlSetProp(node, (xmlChar *)"sectype",
+			    (xmlChar *)sectype);
+	}
+	if (node == NULL) {
+		ret = SA_NO_MEMORY;
+		goto out;
+	}
+
+	xmlSetProp(node, (xmlChar *)"type", (xmlChar *)proto);
+	/* now find the properties */
 	iter = scf_iter_create(handle->handle);
 	value = scf_value_create(handle->handle);
 	prop = scf_property_create(handle->handle);
 	name = malloc(scf_max_name_len);
 	valuestr = malloc(vallen);
 
-	if (sectype == NULL)
-	    node = xmlNewChild(root, NULL, (xmlChar *)"optionset", NULL);
-	else {
-	    node = xmlNewChild(root, NULL, (xmlChar *)"security", NULL);
-	    if (node != NULL)
-		xmlSetProp(node, (xmlChar *)"sectype", (xmlChar *)sectype);
-	}
-	if (node != NULL) {
-	    xmlSetProp(node, (xmlChar *)"type", (xmlChar *)proto);
-	    /* now find the properties */
-	    if (iter != NULL && value != NULL && prop != NULL && name != NULL) {
-		/* iterate over the share pg properties */
-		if (scf_iter_pg_properties(iter, pg) == 0) {
-		    while (scf_iter_next_property(iter, prop) > 0) {
+	if (iter == NULL || value == NULL || prop == NULL || name == NULL)
+		goto out;
+
+	/* Iterate over the share pg properties */
+	if (scf_iter_pg_properties(iter, pg) == 0) {
+		while (scf_iter_next_property(iter, prop) > 0) {
 			ret = SA_SYSTEM_ERR; /* assume the worst */
 			if (scf_property_get_name(prop, name,
-						    scf_max_name_len) > 0) {
-			    if (scf_property_get_value(prop, value) == 0) {
-				if (scf_value_get_astring(value, valuestr,
-							    vallen) >= 0) {
-				    ret = SA_OK;
+			    scf_max_name_len) > 0) {
+				if (scf_property_get_value(prop, value) == 0) {
+					if (scf_value_get_astring(value,
+					    valuestr, vallen) >= 0) {
+						ret = SA_OK;
+					}
 				}
-			    }
 			} else {
-			    ret = SA_SYSTEM_ERR;
+				ret = SA_SYSTEM_ERR;
 			}
 			if (ret == SA_OK) {
-			    sa_property_t prop;
-			    prop = sa_create_property(name, valuestr);
-			    if (prop != NULL)
-				prop = (sa_property_t)xmlAddChild(node,
-							(xmlNodePtr)prop);
-			    else
-				ret = SA_NO_MEMORY;
+				sa_property_t prop;
+				prop = sa_create_property(name, valuestr);
+				if (prop != NULL)
+					prop = (sa_property_t)xmlAddChild(node,
+					    (xmlNodePtr)prop);
+				else
+					ret = SA_NO_MEMORY;
 			}
-		    }
-		} else {
-		    ret = SA_SYSTEM_ERR;
 		}
-	    }
 	} else {
-	    ret = SA_NO_MEMORY;
+		ret = SA_SYSTEM_ERR;
 	}
+out:
 	if (iter != NULL)
-	    scf_iter_destroy(iter);
+		scf_iter_destroy(iter);
 	if (value != NULL)
-	    scf_value_destroy(value);
+		scf_value_destroy(value);
 	if (prop != NULL)
-	    scf_property_destroy(prop);
+		scf_property_destroy(prop);
 	if (name != NULL)
-	    free(name);
+		free(name);
 	if (valuestr != NULL)
-	    free(valuestr);
+		free(valuestr);
 	return (ret);
 }
 
 /*
  * sa_extract_group(root, handle, instance)
  *
- * get the config info for this instance of a group and create the XML
+ * Get the config info for this instance of a group and create the XML
  * subtree from it.
  */
 
@@ -742,152 +750,153 @@ sa_extract_group(xmlNodePtr root, scfutilhandle_t *handle,
 	int err;
 
 	buff = malloc(scf_max_name_len);
+	if (buff == NULL)
+		return (SA_NO_MEMORY);
+
 	iter = scf_iter_create(handle->handle);
-	if (buff != NULL) {
-	    if (scf_instance_get_name(instance, buff,
-						scf_max_name_len) > 0) {
+	if (iter == NULL) {
+		ret = SA_NO_MEMORY;
+		goto out;
+	}
+
+	if (scf_instance_get_name(instance, buff, scf_max_name_len) > 0) {
 		node = xmlNewChild(root, NULL, (xmlChar *)"group", NULL);
-		if (node != NULL) {
-		    xmlSetProp(node, (xmlChar *)"name", (xmlChar *)buff);
-		    if (strcmp(buff, "default") == 0)
+		if (node == NULL) {
+			ret = SA_NO_MEMORY;
+			goto out;
+		}
+		xmlSetProp(node, (xmlChar *)"name", (xmlChar *)buff);
+		if (strcmp(buff, "default") == 0)
 			is_default++;
-		    sa_extract_attrs(node, handle, instance);
-			/*
-			 * Iterate through all the property groups
-			 * looking for those with security or
-			 * optionset prefixes. The names of the
-			 * matching pgroups are parsed to get the
-			 * protocol, and for security, the sectype.
-			 * Syntax is as follows:
-			 *    optionset | optionset_<proto>
-			 *    security_default | security_<proto>_<sectype>
-			 * "operation" is handled by
-			 * sa_extract_attrs().
-			 */
-		    if (iter != NULL) {
-			if (scf_iter_instance_pgs(iter, instance) == 0) {
-			    while (scf_iter_next_pg(iter, handle->pg) > 0) {
-				/* have a pgroup so sort it out */
-				ret = scf_pg_get_name(handle->pg, buff,
-							scf_max_name_len);
-				if (ret  > 0) {
-				    if (buff[0] == SA_SHARE_PG_PREFIX[0]) {
+
+		sa_extract_attrs(node, handle, instance);
+		/*
+		 * Iterate through all the property groups
+		 * looking for those with security or
+		 * optionset prefixes. The names of the
+		 * matching pgroups are parsed to get the
+		 * protocol, and for security, the sectype.
+		 * Syntax is as follows:
+		 *    optionset | optionset_<proto>
+		 *    security_default | security_<proto>_<sectype>
+		 * "operation" is handled by
+		 * sa_extract_attrs().
+		 */
+		if (scf_iter_instance_pgs(iter, instance) != 0) {
+			ret = SA_NO_MEMORY;
+			goto out;
+		}
+		while (scf_iter_next_pg(iter, handle->pg) > 0) {
+			/* Have a pgroup so sort it out */
+			ret = scf_pg_get_name(handle->pg, buff,
+			    scf_max_name_len);
+			if (ret  > 0) {
+				if (buff[0] == SA_SHARE_PG_PREFIX[0]) {
 					sa_share_from_pgroup(node, handle,
-								handle->pg,
-								buff);
+					    handle->pg, buff);
 					have_shares++;
-				    } else if (strncmp(buff, "optionset", 9) ==
-						0) {
+				} else if (strncmp(buff, "optionset", 9) ==
+				    0) {
 					char *nodetype = "optionset";
-					/* have an optionset */
+					/* Have an optionset */
 					sectype = NULL;
 					proto = strchr(buff, '_');
 					if (proto != NULL) {
-					    *proto++ = '\0';
-					    sectype = strchr(proto, '_');
-					    if (sectype != NULL) {
-						*sectype++ = '\0';
-						nodetype = "security";
-					    }
+						*proto++ = '\0';
+						sectype = strchr(proto, '_');
+						if (sectype != NULL) {
+							*sectype++ = '\0';
+							nodetype = "security";
+						}
 					}
 					ret = sa_extract_pgroup(node, handle,
-							    handle->pg,
-							    nodetype,
-							    proto, sectype);
+					    handle->pg, nodetype, proto,
+					    sectype);
 					has_proto++;
-				    } else if (strncmp(buff,
-							"security", 8) == 0) {
+				} else if (strncmp(buff, "security", 8) == 0) {
 					/*
-					 * have a security (note that
+					 * Have a security (note that
 					 * this should change in the
 					 * future)
 					 */
 					proto = strchr(buff, '_');
 					sectype = NULL;
 					if (proto != NULL) {
-					    *proto++ = '\0';
-					    sectype = strchr(proto, '_');
-					    if (sectype != NULL)
-						*sectype++ = '\0';
-					    if (strcmp(proto, "default") == 0)
-						proto = NULL;
+						*proto++ = '\0';
+						sectype = strchr(proto, '_');
+						if (sectype != NULL)
+							*sectype++ = '\0';
+						if (strcmp(proto, "default") ==
+						    0)
+							proto = NULL;
 					}
 					ret = sa_extract_pgroup(node, handle,
-							    handle->pg,
-							    "security", proto,
-							    sectype);
+					    handle->pg, "security", proto,
+					    sectype);
 					has_proto++;
-				    }
-				    /* ignore everything else */
 				}
-			    }
-			} else {
-			    ret = SA_NO_MEMORY;
+				/* Ignore everything else */
 			}
-			/*
-			 * Make sure we have a valid default group.
-			 * On first boot, default won't have any
-			 * protocols defined and won't be enabled (but
-			 * should be).
-			 */
-			if (is_default) {
-			    char *state = sa_get_group_attr((sa_group_t)node,
-							    "state");
-			    char **protos;
-			    int numprotos;
-			    int i;
-
-			    if (state == NULL) {
+		}
+		/*
+		 * Make sure we have a valid default group.
+		 * On first boot, default won't have any
+		 * protocols defined and won't be enabled (but
+		 * should be).
+		 */
+		if (is_default) {
+			char *state = sa_get_group_attr((sa_group_t)node,
+			    "state");
+			char **protos;
+			int numprotos;
+			int i;
+
+			if (state == NULL) {
 				/* set attribute to enabled */
 				(void) sa_set_group_attr((sa_group_t)node,
-							    "state",
-							    "enabled");
-				/* we can assume no protocols */
+				    "state", "enabled");
+				/* We can assume no protocols */
 				numprotos = sa_get_protocols(&protos);
 				for (i = 0; i < numprotos; i++)
-				    (void) sa_create_optionset((sa_group_t)node,
-								protos[i]);
+					(void) sa_create_optionset(
+					    (sa_group_t)node, protos[i]);
 				if (numprotos > 0)
-				    free(protos);
-			    } else {
+					free(protos);
+			} else {
 				sa_free_attr_string(state);
-			    }
 			}
-			/* do a second pass if shares were found */
-			if (have_shares &&
-				scf_iter_instance_pgs(iter, instance) == 0) {
-			    while (scf_iter_next_pg(iter, handle->pg) > 0) {
+		}
+		/* Do a second pass if shares were found */
+		if (have_shares && scf_iter_instance_pgs(iter, instance) == 0) {
+			while (scf_iter_next_pg(iter, handle->pg) > 0) {
 				/*
-				 * have a pgroup so see if it is a
+				 * Have a pgroup so see if it is a
 				 * share optionset
 				 */
 				err = scf_pg_get_name(handle->pg, buff,
-							scf_max_name_len);
-				if (err  > 0) {
-				    if (buff[0] == SA_SHARE_PG_PREFIX[0]) {
+				    scf_max_name_len);
+				if (err  <= 0)
+					continue;
+				if (buff[0] == SA_SHARE_PG_PREFIX[0]) {
 					ret = sa_share_props_from_pgroup(node,
-								handle,
-								handle->pg,
-								buff, sahandle);
-				    }
+					    handle, handle->pg, buff,
+					    sahandle);
 				}
-			    }
 			}
-		    }
 		}
-	    }
 	}
+out:
 	if (iter != NULL)
-	    scf_iter_destroy(iter);
+		scf_iter_destroy(iter);
 	if (buff != NULL)
-	    free(buff);
+		free(buff);
 	return (ret);
 }
 
 /*
  * sa_extract_defaults(root, handle, instance)
  *
- * local function to find the default properties that live in the
+ * Local function to find the default properties that live in the
  * default instance's "operation" proprerty group.
  */
 
@@ -905,39 +914,41 @@ sa_extract_defaults(xmlNodePtr root, scfutilhandle_t *handle,
 	prop = scf_property_create(handle->handle);
 	value = scf_value_create(handle->handle);
 	valuestr = malloc(vallen);
-	if (prop != NULL && value != NULL && vallen != NULL &&
-	    scf_instance_get_pg(instance, "operation",
-				handle->pg) == 0) {
-	    if (scf_pg_get_property(handle->pg,
-				    "legacy-timestamp", prop) == 0) {
-		/* found the property so get the value */
-		if (scf_property_get_value(prop, value) == 0) {
-		    if (scf_value_get_astring(value, valuestr, vallen) > 0) {
+
+	if (prop == NULL || value == NULL || vallen == 0 ||
+	    scf_instance_get_pg(instance, "operation", handle->pg) != 0)
+		goto out;
+
+	if (scf_pg_get_property(handle->pg, "legacy-timestamp", prop) != 0)
+		goto out;
+
+	/* Found the property so get the value */
+	if (scf_property_get_value(prop, value) == 0) {
+		if (scf_value_get_astring(value, valuestr, vallen) > 0) {
 			node = xmlNewChild(root, NULL, (xmlChar *)"legacy",
-					    NULL);
+			    NULL);
 			if (node != NULL) {
-			    xmlSetProp(node, (xmlChar *)"timestamp",
-					(xmlChar *)valuestr);
-			    xmlSetProp(node, (xmlChar *)"path",
-					(xmlChar *)SA_LEGACY_DFSTAB);
+				xmlSetProp(node, (xmlChar *)"timestamp",
+				    (xmlChar *)valuestr);
+				xmlSetProp(node, (xmlChar *)"path",
+				    (xmlChar *)SA_LEGACY_DFSTAB);
 			}
-		    }
 		}
-	    }
 	}
+out:
 	if (valuestr != NULL)
-	    free(valuestr);
+		free(valuestr);
 	if (value != NULL)
-	    scf_value_destroy(value);
+		scf_value_destroy(value);
 	if (prop != NULL)
-	    scf_property_destroy(prop);
+		scf_property_destroy(prop);
 }
 
 
 /*
  * sa_get_config(handle, root, doc, sahandlec)
  *
- * walk the SMF repository for /network/shares/group and find all the
+ * Walk the SMF repository for /network/shares/group and find all the
  * instances. These become group names.  Then add the XML structure
  * below the groups based on property groups and properties.
  */
@@ -952,33 +963,34 @@ sa_get_config(scfutilhandle_t *handle, xmlNodePtr root, sa_handle_t sahandle)
 	instance = scf_instance_create(handle->handle);
 	iter = scf_iter_create(handle->handle);
 	if (instance != NULL && iter != NULL) {
-	    if ((ret = scf_iter_service_instances(iter,
-						    handle->service)) == 0) {
-		while ((ret = scf_iter_next_instance(iter,
-							instance)) > 0) {
-		    if (scf_instance_get_name(instance, buff,
-						sizeof (buff)) > 0) {
-			if (strcmp(buff, "default") == 0)
-			    sa_extract_defaults(root, handle, instance);
-			ret = sa_extract_group(root, handle, instance,
-						sahandle);
-		    }
+		if ((ret = scf_iter_service_instances(iter,
+		    handle->service)) == 0) {
+			while ((ret = scf_iter_next_instance(iter,
+			    instance)) > 0) {
+				if (scf_instance_get_name(instance, buff,
+				    sizeof (buff)) > 0) {
+					if (strcmp(buff, "default") == 0)
+						sa_extract_defaults(root,
+						    handle, instance);
+					ret = sa_extract_group(root, handle,
+					    instance, sahandle);
+				}
+			}
 		}
-	    }
 	}
 
-	/* always cleanup these */
+	/* Always cleanup these */
 	if (instance != NULL)
-	    scf_instance_destroy(instance);
+		scf_instance_destroy(instance);
 	if (iter != NULL)
-	    scf_iter_destroy(iter);
+		scf_iter_destroy(iter);
 	return (ret);
 }
 
 /*
  * sa_get_instance(handle, instance)
  *
- * get the instance of the group service. This is actually the
+ * Get the instance of the group service. This is actually the
  * specific group name. The instance is needed for all property and
  * control operations.
  */
@@ -987,8 +999,8 @@ int
 sa_get_instance(scfutilhandle_t *handle, char *instname)
 {
 	if (scf_service_get_instance(handle->service, instname,
-					handle->instance) != 0) {
-	    return (SA_NO_SUCH_GROUP);
+	    handle->instance) != 0) {
+		return (SA_NO_SUCH_GROUP);
 	}
 	return (SA_OK);
 }
@@ -1006,17 +1018,17 @@ sa_create_instance(scfutilhandle_t *handle, char *instname)
 	int ret = SA_OK;
 	char instance[SA_GROUP_INST_LEN];
 	if (scf_service_add_instance(handle->service, instname,
-					handle->instance) != 0) {
+	    handle->instance) != 0) {
 	/* better error returns need to be added based on real error */
-	    if (scf_error() == SCF_ERROR_PERMISSION_DENIED)
-		ret = SA_NO_PERMISSION;
-	    else
-		ret = SA_DUPLICATE_NAME;
+		if (scf_error() == SCF_ERROR_PERMISSION_DENIED)
+			ret = SA_NO_PERMISSION;
+		else
+			ret = SA_DUPLICATE_NAME;
 	} else {
-	    /* have the service created, so enable it */
-	    (void) snprintf(instance, sizeof (instance), "%s:%s",
-				SA_SVC_FMRI_BASE, instname);
-	    (void) smf_enable_instance(instance, 0);
+		/* have the service created, so enable it */
+		(void) snprintf(instance, sizeof (instance), "%s:%s",
+		    SA_SVC_FMRI_BASE, instname);
+		(void) smf_enable_instance(instance, 0);
 	}
 	return (ret);
 }
@@ -1033,13 +1045,13 @@ sa_delete_instance(scfutilhandle_t *handle, char *instname)
 	int ret;
 
 	if (strcmp(instname, "default") == 0) {
-	    ret = SA_NO_PERMISSION;
+		ret = SA_NO_PERMISSION;
 	} else {
-	    if ((ret = sa_get_instance(handle, instname)) == SA_OK) {
-		if (scf_instance_delete(handle->instance) != 0)
-			/* need better analysis */
-		    ret = SA_NO_PERMISSION;
-	    }
+		if ((ret = sa_get_instance(handle, instname)) == SA_OK) {
+			if (scf_instance_delete(handle->instance) != 0)
+				/* need better analysis */
+				ret = SA_NO_PERMISSION;
+		}
 	}
 	return (ret);
 }
@@ -1055,31 +1067,30 @@ sa_create_pgroup(scfutilhandle_t *handle, char *pgroup)
 {
 	int ret = SA_OK;
 	/*
-	 * only create a handle if it doesn't exist. It is ok to exist
+	 * Only create a handle if it doesn't exist. It is ok to exist
 	 * since the pg handle will be set as a side effect.
 	 */
-	if (handle->pg == NULL) {
-	    handle->pg = scf_pg_create(handle->handle);
-	}
+	if (handle->pg == NULL)
+		handle->pg = scf_pg_create(handle->handle);
+
 	/*
-	 * if the pgroup exists, we are done. If it doesn't, then we
+	 * If the pgroup exists, we are done. If it doesn't, then we
 	 * need to actually add one to the service instance.
 	 */
 	if (scf_instance_get_pg(handle->instance,
-				pgroup, handle->pg) != 0) {
-	    /* doesn't exist so create one */
-	    if (scf_instance_add_pg(handle->instance, pgroup,
-				    SCF_GROUP_APPLICATION, 0,
-				    handle->pg) != 0) {
-		switch (scf_error()) {
-		case SCF_ERROR_PERMISSION_DENIED:
-		    ret = SA_NO_PERMISSION;
-		    break;
-		default:
-		    ret = SA_SYSTEM_ERR;
-		    break;
+	    pgroup, handle->pg) != 0) {
+		/* Doesn't exist so create one */
+		if (scf_instance_add_pg(handle->instance, pgroup,
+		    SCF_GROUP_APPLICATION, 0, handle->pg) != 0) {
+			switch (scf_error()) {
+			case SCF_ERROR_PERMISSION_DENIED:
+				ret = SA_NO_PERMISSION;
+				break;
+			default:
+				ret = SA_SYSTEM_ERR;
+				break;
+			}
 		}
-	    }
 	}
 	return (ret);
 }
@@ -1087,7 +1098,7 @@ sa_create_pgroup(scfutilhandle_t *handle, char *pgroup)
 /*
  * sa_delete_pgroup(handle, pgroup)
  *
- * remove the property group from the current instance of the service,
+ * Remove the property group from the current instance of the service,
  * but only if it actually exists.
  */
 
@@ -1096,16 +1107,14 @@ sa_delete_pgroup(scfutilhandle_t *handle, char *pgroup)
 {
 	int ret = SA_OK;
 	/*
-	 * only delete if it does exist.
+	 * Only delete if it does exist.
 	 */
-	if (scf_instance_get_pg(handle->instance,
-				pgroup, handle->pg) == 0) {
-	    /* does exist so delete it */
-	    if (scf_pg_delete(handle->pg) != 0) {
-		ret = SA_SYSTEM_ERR;
-	    }
+	if (scf_instance_get_pg(handle->instance, pgroup, handle->pg) == 0) {
+		/* does exist so delete it */
+		if (scf_pg_delete(handle->pg) != 0)
+			ret = SA_SYSTEM_ERR;
 	} else {
-	    ret = SA_SYSTEM_ERR;
+		ret = SA_SYSTEM_ERR;
 	}
 	if (ret == SA_SYSTEM_ERR &&
 	    scf_error() == SCF_ERROR_PERMISSION_DENIED) {
@@ -1136,25 +1145,26 @@ sa_start_transaction(scfutilhandle_t *handle, char *propgroup)
 {
 	int ret = SA_OK;
 	/*
-	 * lookup the property group and create it if it doesn't already
+	 * Lookup the property group and create it if it doesn't already
 	 * exist.
 	 */
 	if (handle->scf_state == SCH_STATE_INIT) {
-	    ret = sa_create_pgroup(handle, propgroup);
-	    if (ret == SA_OK) {
-		handle->trans = scf_transaction_create(handle->handle);
-		if (handle->trans != NULL) {
-		    if (scf_transaction_start(handle->trans, handle->pg) != 0) {
-			ret = SA_SYSTEM_ERR;
-		    }
-		    if (ret != SA_OK) {
-			scf_transaction_destroy(handle->trans);
-			handle->trans = NULL;
-		    }
-		} else {
-		    ret = SA_SYSTEM_ERR;
+		ret = sa_create_pgroup(handle, propgroup);
+		if (ret == SA_OK) {
+			handle->trans = scf_transaction_create(handle->handle);
+			if (handle->trans != NULL) {
+				if (scf_transaction_start(handle->trans,
+				    handle->pg) != 0) {
+					ret = SA_SYSTEM_ERR;
+				}
+				if (ret != SA_OK) {
+					scf_transaction_destroy(handle->trans);
+					handle->trans = NULL;
+				}
+			} else {
+				ret = SA_SYSTEM_ERR;
+			}
 		}
-	    }
 	}
 	if (ret == SA_SYSTEM_ERR &&
 	    scf_error() == SCF_ERROR_PERMISSION_DENIED) {
@@ -1176,13 +1186,13 @@ sa_end_transaction(scfutilhandle_t *handle)
 	int ret = SA_OK;
 
 	if (handle->trans == NULL) {
-	    ret = SA_SYSTEM_ERR;
-	} else {
-	    if (scf_transaction_commit(handle->trans) < 0)
 		ret = SA_SYSTEM_ERR;
-	    scf_transaction_destroy_children(handle->trans);
-	    scf_transaction_destroy(handle->trans);
-	    handle->trans = NULL;
+	} else {
+		if (scf_transaction_commit(handle->trans) < 0)
+			ret = SA_SYSTEM_ERR;
+		scf_transaction_destroy_children(handle->trans);
+		scf_transaction_destroy(handle->trans);
+		handle->trans = NULL;
 	}
 	return (ret);
 }
@@ -1198,17 +1208,17 @@ void
 sa_abort_transaction(scfutilhandle_t *handle)
 {
 	if (handle->trans != NULL) {
-	    scf_transaction_reset_all(handle->trans);
-	    scf_transaction_destroy_children(handle->trans);
-	    scf_transaction_destroy(handle->trans);
-	    handle->trans = NULL;
+		scf_transaction_reset_all(handle->trans);
+		scf_transaction_destroy_children(handle->trans);
+		scf_transaction_destroy(handle->trans);
+		handle->trans = NULL;
 	}
 }
 
 /*
  * sa_set_property(handle, prop, value)
  *
- * set a property transaction entry into the pending SMF transaction.
+ * Set a property transaction entry into the pending SMF transaction.
  */
 
 int
@@ -1218,59 +1228,57 @@ sa_set_property(scfutilhandle_t *handle, char *propname, char *valstr)
 	scf_value_t *value;
 	scf_transaction_entry_t *entry;
 	/*
-	 * properties must be set in transactions and don't take
+	 * Properties must be set in transactions and don't take
 	 * effect until the transaction has been ended/committed.
 	 */
 	value = scf_value_create(handle->handle);
 	entry = scf_entry_create(handle->handle);
 	if (value != NULL && entry != NULL) {
-	    if (scf_transaction_property_change(handle->trans, entry,
-						propname,
-						SCF_TYPE_ASTRING) == 0 ||
-		scf_transaction_property_new(handle->trans, entry,
-						propname,
-						SCF_TYPE_ASTRING) == 0) {
-		if (scf_value_set_astring(value, valstr) == 0) {
-		    if (scf_entry_add_value(entry, value) != 0) {
-			ret = SA_SYSTEM_ERR;
-			scf_value_destroy(value);
-		    }
-		    /* the value is in the transaction */
-		    value = NULL;
+		if (scf_transaction_property_change(handle->trans, entry,
+		    propname, SCF_TYPE_ASTRING) == 0 ||
+		    scf_transaction_property_new(handle->trans, entry,
+		    propname, SCF_TYPE_ASTRING) == 0) {
+			if (scf_value_set_astring(value, valstr) == 0) {
+				if (scf_entry_add_value(entry, value) != 0) {
+					ret = SA_SYSTEM_ERR;
+					scf_value_destroy(value);
+				}
+				/* The value is in the transaction */
+				value = NULL;
+			} else {
+				/* Value couldn't be constructed */
+				ret = SA_SYSTEM_ERR;
+			}
+			/* The entry is in the transaction */
+			entry = NULL;
 		} else {
-		    /* value couldn't be constructed */
-		    ret = SA_SYSTEM_ERR;
+			ret = SA_SYSTEM_ERR;
 		}
-		/* the entry is in the transaction */
-		entry = NULL;
-	    } else {
-		ret = SA_SYSTEM_ERR;
-	    }
 	} else {
-	    ret = SA_SYSTEM_ERR;
+		ret = SA_SYSTEM_ERR;
 	}
 	if (ret == SA_SYSTEM_ERR) {
-	    switch (scf_error()) {
-	    case SCF_ERROR_PERMISSION_DENIED:
-		ret = SA_NO_PERMISSION;
-		break;
-	    }
+		switch (scf_error()) {
+		case SCF_ERROR_PERMISSION_DENIED:
+			ret = SA_NO_PERMISSION;
+			break;
+		}
 	}
 	/*
-	 * cleanup if there were any errors that didn't leave these
+	 * Cleanup if there were any errors that didn't leave these
 	 * values where they would be cleaned up later.
 	 */
 	if (value != NULL)
-	    scf_value_destroy(value);
+		scf_value_destroy(value);
 	if (entry != NULL)
-	    scf_entry_destroy(entry);
+		scf_entry_destroy(entry);
 	return (ret);
 }
 
 /*
  * sa_commit_share(handle, group, share)
  *
- *	commit this share to the repository.
+ *	Commit this share to the repository.
  *	properties are added if they exist but can be added later.
  *	Need to add to dfstab and sharetab, if appropriate.
  */
@@ -1287,113 +1295,120 @@ sa_commit_share(scfutilhandle_t *handle, sa_group_t group, sa_share_t share)
 	char *propstring;
 
 	/*
-	 * don't commit in the zfs group. We do commit legacy
+	 * Don't commit in the zfs group. We do commit legacy
 	 * (default) and all other groups/shares. ZFS is handled
 	 * through the ZFS configuration rather than SMF.
 	 */
 
 	groupname = sa_get_group_attr(group, "name");
 	if (groupname != NULL) {
-	    if (strcmp(groupname, "zfs") == 0) {
-		/*
-		 * adding to the ZFS group will result in the sharenfs
-		 * property being set but we don't want to do anything
-		 * SMF related at this point.
-		 */
-		sa_free_attr_string(groupname);
-		return (ret);
-	    }
+		if (strcmp(groupname, "zfs") == 0) {
+			/*
+			 * Adding to the ZFS group will result in the sharenfs
+			 * property being set but we don't want to do anything
+			 * SMF related at this point.
+			 */
+			sa_free_attr_string(groupname);
+			return (ret);
+		}
 	}
 
 	proplen = get_scf_limit(SCF_LIMIT_MAX_VALUE_LENGTH);
 	propstring = malloc(proplen);
 	if (propstring == NULL)
-	    ret = SA_NO_MEMORY;
+		ret = SA_NO_MEMORY;
 
 	if (groupname != NULL && ret == SA_OK) {
-	    ret = sa_get_instance(handle, groupname);
-	    sa_free_attr_string(groupname);
-	    groupname = NULL;
-	    sharename = sa_get_share_attr(share, "id");
-	    if (sharename == NULL) {
-		/* slipped by */
-		char shname[SA_SHARE_UUID_BUFLEN];
-		generate_unique_sharename(shname);
-		xmlSetProp((xmlNodePtr)share, (xmlChar *)"id",
+		ret = sa_get_instance(handle, groupname);
+		sa_free_attr_string(groupname);
+		groupname = NULL;
+		sharename = sa_get_share_attr(share, "id");
+		if (sharename == NULL) {
+			/* slipped by */
+			char shname[SA_SHARE_UUID_BUFLEN];
+			generate_unique_sharename(shname);
+			xmlSetProp((xmlNodePtr)share, (xmlChar *)"id",
 			    (xmlChar *)shname);
-		sharename = strdup(shname);
-	    }
-	    if (sharename != NULL) {
-		sigset_t old, new;
-		/*
-		 * have a share name allocated so create a pgroup for
-		 * it. It may already exist, but that is OK.  In order
-		 * to avoid creating a share pgroup that doesn't have
-		 * a path property, block signals around the critical
-		 * region of creating the share pgroup and props.
-		 */
-		(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);
-
-		ret = sa_create_pgroup(handle, sharename);
-		if (ret == SA_OK) {
+			sharename = strdup(shname);
+		}
+		if (sharename != NULL) {
+			sigset_t old, new;
 			/*
-			 * now start the transaction for the
-			 * properties that define this share. They may
-			 * exist so attempt to update before create.
+			 * Have a share name allocated so create a pgroup for
+			 * it. It may already exist, but that is OK.  In order
+			 * to avoid creating a share pgroup that doesn't have
+			 * a path property, block signals around the critical
+			 * region of creating the share pgroup and props.
 			 */
-		    ret = sa_start_transaction(handle, sharename);
-		}
-		if (ret == SA_OK) {
-		    name = sa_get_share_attr(share, "path");
-		    if (name != NULL) {
-			/* there needs to be a path for a share to exist */
-			ret = sa_set_property(handle, "path", name);
-			sa_free_attr_string(name);
-		    } else {
-			ret = SA_NO_MEMORY;
-		    }
-		}
-		if (ret == SA_OK) {
-		    resource = sa_get_share_attr(share, "resource");
-		    if (resource != NULL) {
-			ret = sa_set_property(handle, "resource", resource);
-			sa_free_attr_string(resource);
-		    }
-		}
-		if (ret == SA_OK) {
-		    description = sa_get_share_description(share);
-		    if (description != NULL) {
-			ret = sa_set_property(handle, "description",
-						description);
-			sa_free_share_description(description);
-		    }
-		}
-		/* make sure we cleanup the transaction */
-		if (ret == SA_OK) {
-		    ret = sa_end_transaction(handle);
-		} else {
-		    sa_abort_transaction(handle);
-		}
+			(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);
+
+			ret = sa_create_pgroup(handle, sharename);
+			if (ret == SA_OK) {
+				/*
+				 * Now start the transaction for the
+				 * properties that define this share. They may
+				 * exist so attempt to update before create.
+				 */
+				ret = sa_start_transaction(handle, sharename);
+			}
+			if (ret == SA_OK) {
+				name = sa_get_share_attr(share, "path");
+				if (name != NULL) {
+					/*
+					 * There needs to be a path
+					 * for a share to exist.
+					 */
+					ret = sa_set_property(handle, "path",
+					    name);
+					sa_free_attr_string(name);
+				} else {
+					ret = SA_NO_MEMORY;
+				}
+			}
+			if (ret == SA_OK) {
+				resource = sa_get_share_attr(share,
+				    "resource");
+				if (resource != NULL) {
+					ret = sa_set_property(handle,
+					    "resource", resource);
+					sa_free_attr_string(resource);
+				}
+			}
+			if (ret == SA_OK) {
+				description = sa_get_share_description(share);
+				if (description != NULL) {
+					ret = sa_set_property(handle,
+					    "description",
+					    description);
+					sa_free_share_description(description);
+				}
+			}
+			/* Make sure we cleanup the transaction */
+			if (ret == SA_OK) {
+				ret = sa_end_transaction(handle);
+			} else {
+				sa_abort_transaction(handle);
+			}
 
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+			(void) sigprocmask(SIG_SETMASK, &old, NULL);
 
-		free(sharename);
-	    }
+			free(sharename);
+		}
 	}
 	if (ret == SA_SYSTEM_ERR) {
-	    int err = scf_error();
-	    if (err == SCF_ERROR_PERMISSION_DENIED)
-		ret = SA_NO_PERMISSION;
+		int err = scf_error();
+		if (err == SCF_ERROR_PERMISSION_DENIED)
+			ret = SA_NO_PERMISSION;
 	}
 	if (propstring != NULL)
-	    free(propstring);
+		free(propstring);
 	if (groupname != NULL)
-	    sa_free_attr_string(groupname);
+		sa_free_attr_string(groupname);
 
 	return (ret);
 }
@@ -1401,7 +1416,7 @@ sa_commit_share(scfutilhandle_t *handle, sa_group_t group, sa_share_t share)
 /*
  * sa_delete_share(handle, group, share)
  *
- * remove the specified share from the group (and service instance).
+ * Remove the specified share from the group (and service instance).
  */
 
 int
@@ -1418,63 +1433,69 @@ sa_delete_share(scfutilhandle_t *handle, sa_group_t group, sa_share_t share)
 	proplen = get_scf_limit(SCF_LIMIT_MAX_VALUE_LENGTH);
 	propstring = malloc(proplen);
 	if (propstring == NULL)
-	    ret = SA_NO_MEMORY;
+		ret = SA_NO_MEMORY;
 
 	if (ret == SA_OK) {
-	    groupname = sa_get_group_attr(group, "name");
-	    shareid = sa_get_share_attr(share, "id");
-	    if (groupname != NULL && shareid != NULL) {
+		groupname = sa_get_group_attr(group, "name");
+		shareid = sa_get_share_attr(share, "id");
+		if (groupname == NULL || shareid == NULL) {
+			ret = SA_CONFIG_ERR;
+			goto out;
+		}
 		ret = sa_get_instance(handle, groupname);
 		if (ret == SA_OK) {
-		    /* if a share has properties, remove them */
-		    ret = sa_delete_pgroup(handle, shareid);
-		    for (opt = sa_get_optionset(share, NULL); opt != NULL;
-			opt = sa_get_next_optionset(opt)) {
-			char *proto;
-			proto = sa_get_optionset_attr(opt, "type");
-			if (proto != NULL) {
-			    (void) snprintf(propstring, proplen, "%s_%s",
-						shareid, proto);
-			    ret = sa_delete_pgroup(handle, propstring);
-			    sa_free_attr_string(proto);
-			} else {
-			    ret = SA_NO_MEMORY;
+			/* If a share has properties, remove them */
+			ret = sa_delete_pgroup(handle, shareid);
+			for (opt = sa_get_optionset(share, NULL);
+			    opt != NULL;
+			    opt = sa_get_next_optionset(opt)) {
+				char *proto;
+				proto = sa_get_optionset_attr(opt, "type");
+				if (proto != NULL) {
+					(void) snprintf(propstring,
+					    proplen, "%s_%s", shareid,
+					    proto);
+					ret = sa_delete_pgroup(handle,
+					    propstring);
+					sa_free_attr_string(proto);
+				} else {
+					ret = SA_NO_MEMORY;
+				}
 			}
-		    }
 			/*
-			 * if a share has security/negotiable
+			 * If a share has security/negotiable
 			 * properties, remove them.
 			 */
-		    for (sec = sa_get_security(share, NULL, NULL); sec != NULL;
-			sec = sa_get_next_security(sec)) {
-			char *proto;
-			char *sectype;
-			proto = sa_get_security_attr(sec, "type");
-			sectype = sa_get_security_attr(sec, "sectype");
-			if (proto != NULL && sectype != NULL) {
-			    (void) snprintf(propstring, proplen, "%s_%s_%s",
-					shareid,
-					proto, sectype);
-			    ret = sa_delete_pgroup(handle, propstring);
-			} else {
-			    ret = SA_NO_MEMORY;
+			for (sec = sa_get_security(share, NULL, NULL);
+			    sec != NULL;
+			    sec = sa_get_next_security(sec)) {
+				char *proto;
+				char *sectype;
+				proto = sa_get_security_attr(sec, "type");
+				sectype = sa_get_security_attr(sec, "sectype");
+				if (proto != NULL && sectype != NULL) {
+					(void) snprintf(propstring, proplen,
+					    "%s_%s_%s", shareid,  proto,
+					    sectype);
+					ret = sa_delete_pgroup(handle,
+					    propstring);
+				} else {
+					ret = SA_NO_MEMORY;
+				}
+				if (proto != NULL)
+					sa_free_attr_string(proto);
+				if (sectype != NULL)
+					sa_free_attr_string(sectype);
 			}
-			if (proto != NULL)
-			    sa_free_attr_string(proto);
-			if (sectype != NULL)
-			    sa_free_attr_string(sectype);
-		    }
 		}
-	    } else {
-		ret = SA_CONFIG_ERR;
-	    }
 	}
+out:
 	if (groupname != NULL)
-	    sa_free_attr_string(groupname);
+		sa_free_attr_string(groupname);
 	if (shareid != NULL)
-	    sa_free_attr_string(shareid);
+		sa_free_attr_string(shareid);
 	if (propstring != NULL)
-	    free(propstring);
+		free(propstring);
 
 	return (ret);
 }