Fixed style issues with variable ordering and naming, spacing and

parentheses.

Fixed alignment issue in gr_dup() in its assignment of gr_mem using a
struct to force alignment without performing alignment mathematics.  This
was noticed recently with libutil was built with WARNS=6 on platform such
as sparc64.

Added checks to gr_dup(), gr_equal() and gr_make() to prevent segfaults
when examining struct group's with the struct members pointing to NULL's.

With fix of alignment issue, restore WARNS?=6.

Reviewed by:	des
MFC after:	1 week

1 files changed, 97 insertions, 75 deletions

diff --git a/libutil/gr_util.c b/libutil/gr_util.c
index fc5e442..77e0653 100644
--- a/libutil/gr_util.c
+++ b/libutil/gr_util.c
@@ -37,7 +37,12 @@ __FBSDID("$FreeBSD$");
 #include <stdlib.h>
 #include <string.h>
 
-static const char GroupLineFormat[] = "%s:%s:%ju:";
+struct group_storage {
+	struct group	 gr;
+	char		*members[];
+};
+
+static const char group_line_format[] = "%s:%s:%ju:";
 
 /*
  * Compares two struct group's.
@@ -45,38 +50,48 @@ static const char GroupLineFormat[] = "%s:%s:%ju:";
 int
 gr_equal(const struct group *gr1, const struct group *gr2)
 {
-	int gr1Ndx;
-	int gr2Ndx;
-	bool equal;
+	int gr1_ndx;
+	int gr2_ndx;
 	bool found;
 
 	/* Check that the non-member information is the same. */
-	equal = strcmp(gr1->gr_name, gr2->gr_name) == 0 &&
-	    strcmp(gr1->gr_passwd, gr2->gr_passwd) == 0 &&
-	    gr1->gr_gid == gr2->gr_gid;
+	if (gr1->gr_name == NULL || gr2->gr_name == NULL) {
+		if (gr1->gr_name != gr2->gr_name)
+			return (false);
+	} else if (strcmp(gr1->gr_name, gr2->gr_name) != 0)
+		return (false);
+	if (gr1->gr_passwd == NULL || gr2->gr_passwd == NULL) {
+		if (gr1->gr_passwd != gr2->gr_passwd)
+			return (false);
+	} else if (strcmp(gr1->gr_passwd, gr2->gr_passwd) != 0)
+		return (false);
+	if (gr1->gr_gid != gr2->gr_gid)
+		return (false);
 
 	/* Check all members in both groups. */
-	if (equal) {
-		for (found = false, gr1Ndx = 0; gr1->gr_mem[gr1Ndx] != NULL;
-		    gr1Ndx++) {
-			for (gr2Ndx = 0; gr2->gr_mem[gr2Ndx] != NULL; gr2Ndx++)
-				if (strcmp(gr1->gr_mem[gr1Ndx],
-				    gr2->gr_mem[gr2Ndx]) == 0) {
+	if (gr1->gr_mem == NULL || gr2->gr_mem == NULL) {
+		if (gr1->gr_mem != gr2->gr_mem)
+			return (false);
+	} else {
+		for (found = false, gr1_ndx = 0; gr1->gr_mem[gr1_ndx] != NULL;
+		    gr1_ndx++) {
+			for (gr2_ndx = 0; gr2->gr_mem[gr2_ndx] != NULL;
+			    gr2_ndx++)
+				if (strcmp(gr1->gr_mem[gr1_ndx],
+				    gr2->gr_mem[gr2_ndx]) == 0) {
 					found = true;
 					break;
 				}
-			if (! found) {
-				equal = false;
-				break;
-			}
+			if (!found)
+				return (false);
 		}
 
 		/* Check that group2 does not have more members than group1. */
-		if (gr2->gr_mem[gr1Ndx] != NULL)
-			equal = false;
+		if (gr2->gr_mem[gr1_ndx] != NULL)
+			return (false);
 	}
 
-	return (equal);
+	return (true);
 }
 
 /*
@@ -86,27 +101,30 @@ char *
 gr_make(const struct group *gr)
 {
 	char *line;
-	size_t lineSize;
+	size_t line_size;
 	int ndx;
 
 	/* Calculate the length of the group line. */
-	lineSize = snprintf(NULL, 0, GroupLineFormat, gr->gr_name,
+	line_size = snprintf(NULL, 0, group_line_format, gr->gr_name,
 	    gr->gr_passwd, (uintmax_t)gr->gr_gid) + 1;
-	for (ndx = 0; gr->gr_mem[ndx] != NULL; ndx++)
-		lineSize += strlen(gr->gr_mem[ndx]) + 1;
-	if (ndx > 0)
-		lineSize--;
+	if (gr->gr_mem != NULL) {
+		for (ndx = 0; gr->gr_mem[ndx] != NULL; ndx++)
+			line_size += strlen(gr->gr_mem[ndx]) + 1;
+		if (ndx > 0)
+			line_size--;
+	}
 
 	/* Create the group line and fill it. */
-	if ((line = malloc(lineSize)) == NULL)
+	if ((line = malloc(line_size)) == NULL)
 		return (NULL);
-	lineSize = snprintf(line, lineSize, GroupLineFormat, gr->gr_name,
+	line_size = snprintf(line, line_size, group_line_format, gr->gr_name,
 	    gr->gr_passwd, (uintmax_t)gr->gr_gid);
-	for (ndx = 0; gr->gr_mem[ndx] != NULL; ndx++) {
-		strcat(line, gr->gr_mem[ndx]);
-		if (gr->gr_mem[ndx + 1] != NULL)
-			strcat(line, ",");
-	}
+	if (gr->gr_mem != NULL)
+		for (ndx = 0; gr->gr_mem[ndx] != NULL; ndx++) {
+			strcat(line, gr->gr_mem[ndx]);
+			if (gr->gr_mem[ndx + 1] != NULL)
+				strcat(line, ",");
+		}
 
 	return (line);
 }
@@ -117,47 +135,48 @@ gr_make(const struct group *gr)
 struct group *
 gr_dup(const struct group *gr)
 {
+	char *dst;
 	size_t len;
-	struct group *ngr;
+	struct group_storage *gs;
 	int ndx;
-	int numMem;
-
-	/* Calculate size of group. */
-	len = sizeof(*gr) +
-	    (gr->gr_name != NULL ? strlen(gr->gr_name) + 1 : 0) +
-	    (gr->gr_passwd != NULL ? strlen(gr->gr_passwd) + 1 : 0);
-	numMem = 0;
+	int num_mem;
+
+	/* Calculate size of the group. */
+	len = sizeof(*gs);
+	if (gr->gr_name != NULL)
+		len += strlen(gr->gr_name) + 1;
+	if (gr->gr_passwd != NULL)
+		len += strlen(gr->gr_passwd) + 1;
 	if (gr->gr_mem != NULL) {
-		for (; gr->gr_mem[numMem] != NULL; numMem++)
-			len += strlen(gr->gr_mem[numMem]) + 1;
-		len += (numMem + 1) * sizeof(*gr->gr_mem);
-	}
+		for (num_mem = 0; gr->gr_mem[num_mem] != NULL; num_mem++)
+			len += strlen(gr->gr_mem[num_mem]) + 1;
+		len += (num_mem + 1) * sizeof(*gr->gr_mem);
+	} else
+		num_mem = -1;
 
 	/* Create new group and copy old group into it. */
-	if ((ngr = calloc(1, len)) == NULL)
+	if ((gs = calloc(1, len)) == NULL)
 		return (NULL);
-	len = sizeof(*ngr);
-	ngr->gr_gid = gr->gr_gid;
+	dst = (char *)&gs->members[num_mem + 1];
 	if (gr->gr_name != NULL) {
-		ngr->gr_name = (char *)ngr + len;
-		len += sprintf(ngr->gr_name, "%s", gr->gr_name) + 1;
+		gs->gr.gr_name = dst;
+		dst = stpcpy(gs->gr.gr_name, gr->gr_name) + 1;
 	}
 	if (gr->gr_passwd != NULL) {
-		ngr->gr_passwd = (char *)ngr + len;
-		len += sprintf(ngr->gr_passwd, "%s", gr->gr_passwd) + 1;
+		gs->gr.gr_passwd = dst;
+		dst = stpcpy(gs->gr.gr_passwd, gr->gr_passwd) + 1;
 	}
+	gs->gr.gr_gid = gr->gr_gid;
 	if (gr->gr_mem != NULL) {
-		ngr->gr_mem = (char **)((char *)ngr + len);
-		len += (numMem + 1) * sizeof(*ngr->gr_mem);
-		for (ndx = 0; gr->gr_mem[ndx] != NULL; ndx++) {
-			ngr->gr_mem[ndx] = (char *)ngr + len;
-			len += sprintf(ngr->gr_mem[ndx], "%s",
-			    gr->gr_mem[ndx]) + 1;
+		gs->gr.gr_mem = gs->members;
+		for (ndx = 0; ndx < num_mem; ndx++) {
+			gs->gr.gr_mem[ndx] = dst;
+			dst = stpcpy(gs->gr.gr_mem[ndx], gr->gr_mem[ndx]) + 1;
 		}
-		ngr->gr_mem[ndx] = NULL;
+		gs->gr.gr_mem[ndx] = NULL;
 	}
 
-	return (ngr);
+	return (&gs->gr);
 }
 
 /*
@@ -190,15 +209,18 @@ __gr_scan(char *line, struct group *gr)
 		return (false);
 	line = loc + 1;
 	gr->gr_mem = NULL;
-	if (*line != '\0') {
-		ndx = 0;
+	ndx = 0;
+	do {
+		gr->gr_mem = reallocf(gr->gr_mem, sizeof(*gr->gr_mem) *
+		    (ndx + 1));
+		if (gr->gr_mem == NULL)
+			return (false);
+
+		/* Skip locations without members (i.e., empty string). */
 		do {
-			if ((gr->gr_mem = reallocf(gr->gr_mem,
-			    sizeof(*gr->gr_mem) * (ndx + 1))) == NULL)
-				return (false);
 			gr->gr_mem[ndx] = strsep(&line, ",");
-		} while (gr->gr_mem[ndx++] != NULL);
-	}
+		} while (gr->gr_mem[ndx] != NULL && *gr->gr_mem[ndx] == '\0');
+	} while (gr->gr_mem[ndx++] != NULL);
 
 	return (true);
 }
@@ -210,19 +232,19 @@ struct group *
 gr_scan(const char *line)
 {
 	struct group gr;
-	char *lineCopy;
-	struct group *newGr;
+	char *line_copy;
+	struct group *new_gr;
 
-	if ((lineCopy = strdup(line)) == NULL)
+	if ((line_copy = strdup(line)) == NULL)
 		return (NULL);
-	if (!__gr_scan(lineCopy, &gr)) {
-		free(lineCopy);
+	if (!__gr_scan(line_copy, &gr)) {
+		free(line_copy);
 		return (NULL);
 	}
-	newGr = gr_dup(&gr);
-	free(lineCopy);
+	new_gr = gr_dup(&gr);
+	free(line_copy);
 	if (gr.gr_mem != NULL)
 		free(gr.gr_mem);
 
-	return (newGr);
+	return (new_gr);
 }