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

diff --git a/libutil/gr_util.c b/libutil/gr_util.c
index fc5e44238c91b78f1438c25c3b8ef2da895761ad..77e06531f7058f139c0fc50bb3f6843b562ced49 100644 (file)
--- 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);
 }