Print this page
2nd round review feedback from rmustacc.

@@ -72,36 +72,36 @@
 #define WS      " \t"
 
 static int next_env = 0;
 
 struct compile_env {
-        char            *name;
-        char            *lang;
-        char            *defs;
-        int             index;
+        char            *ce_name;
+        char            *ce_lang;
+        char            *ce_defs;
+        int             ce_index;
 };
 
 static struct compile_env compile_env[MAXENV];
 
 struct env_group {
-        char                    *name;
-        uint64_t                mask;
-        struct env_group        *next;
+        char                    *eg_name;
+        uint64_t                eg_mask;
+        struct env_group        *eg_next;
 };
 
 typedef enum { SYM_TYPE, SYM_VALUE, SYM_FUNC } sym_type_t;
 
 struct sym_test {
-        char                    *name;
-        sym_type_t              type;
-        char                    *hdrs[MAXHDR];
-        char                    *rtype;
-        char                    *atypes[MAXARG];
-        uint64_t                test_mask;
-        uint64_t                need_mask;
-        char                    *prog;
-        struct sym_test         *next;
+        char                    *st_name;
+        sym_type_t              st_type;
+        char                    *st_hdrs[MAXHDR];
+        char                    *st_rtype;
+        char                    *st_atypes[MAXARG];
+        uint64_t                st_test_mask;
+        uint64_t                st_need_mask;
+        char                    *st_prog;
+        struct sym_test         *st_next;
 };
 
 struct env_group *env_groups = NULL;
 
 struct sym_test *sym_tests = NULL;

@@ -145,27 +145,27 @@
 
 static void
 append_sym_test(struct sym_test *st)
 {
         *sym_insert = st;
-        sym_insert = &st->next;
+        sym_insert = &st->st_next;
 }
 
 static int
 find_env_mask(const char *name, uint64_t *mask)
 {
-        for (int i = 0; i < 64; i++) {
-                if (compile_env[i].name != NULL &&
-                    strcmp(compile_env[i].name, name) == 0) {
+        for (int i = 0; i < MAXENV; i++) {
+                if (compile_env[i].ce_name != NULL &&
+                    strcmp(compile_env[i].ce_name, name) == 0) {
                         *mask |= (1ULL << i);
                         return (0);
                 }
         }
 
-        for (struct env_group *eg = env_groups; eg != NULL; eg = eg->next) {
-                if (strcmp(name, eg->name) == 0) {
-                        *mask |= eg->mask;
+        for (struct env_group *eg = env_groups; eg != NULL; eg = eg->eg_next) {
+                if (strcmp(name, eg->eg_name) == 0) {
+                        *mask |= eg->eg_mask;
                         return (0);
                 }
         }
         return (-1);
 }

@@ -243,14 +243,14 @@
 
         name = fields[0];
         lang = fields[1];
         defs = fields[2];
 
-        compile_env[next_env].name = mystrdup(name);
-        compile_env[next_env].lang = mystrdup(lang);
-        compile_env[next_env].defs = mystrdup(defs);
-        compile_env[next_env].index = next_env;
+        compile_env[next_env].ce_name = mystrdup(name);
+        compile_env[next_env].ce_lang = mystrdup(lang);
+        compile_env[next_env].ce_defs = mystrdup(defs);
+        compile_env[next_env].ce_index = next_env;
         next_env++;
         return (0);
 }
 
 static int

@@ -275,13 +275,13 @@
                 myasprintf(err, "reference to undefined env %s", item);
                 return (-1);
         }
 
         eg = myzalloc(sizeof (*eg));
-        eg->name = mystrdup(name);
-        eg->mask = mask;
-        eg->next = env_groups;
+        eg->eg_name = mystrdup(name);
+        eg->eg_mask = mask;
+        eg->eg_next = env_groups;
         env_groups = eg;
         return (0);
 }
 
 static char *progbuf = NULL;

@@ -332,15 +332,15 @@
 {
         char *s;
 
         proglen = 0;
 
-        for (int i = 0; i < MAXHDR && st->hdrs[i] != NULL; i++) {
-                addprogfmt("#include <%s>\n", st->hdrs[i]);
+        for (int i = 0; i < MAXHDR && st->st_hdrs[i] != NULL; i++) {
+                addprogfmt("#include <%s>\n", st->st_hdrs[i]);
         }
 
-        for (s = st->rtype; *s; s++) {
+        for (s = st->st_rtype; *s; s++) {
                 addprogch(*s);
                 if (*s == '(') {
                         s++;
                         addprogch(*s);
                         s++;

@@ -349,38 +349,38 @@
         }
         addprogch(' ');
 
         /* for function pointers, s is closing suffix, otherwise empty */
 
-        switch (st->type) {
+        switch (st->st_type) {
         case SYM_TYPE:
                 addprogstr("test_type;");
                 break;
 
         case SYM_VALUE:
                 addprogfmt("test_value%s;\n", s);       /* s usually empty */
                 addprogstr("void\ntest_func(void)\n{\n");
-                addprogfmt("\ttest_value = %s;\n}", st->name);
+                addprogfmt("\ttest_value = %s;\n}", st->st_name);
                 break;
 
         case SYM_FUNC:
                 addprogstr("\ntest_func(");
-                for (int i = 0; st->atypes[i] != NULL && i < MAXARG; i++) {
+                for (int i = 0; st->st_atypes[i] != NULL && i < MAXARG; i++) {
                         int didname = 0;
                         if (i > 0) {
                                 addprogstr(", ");
                         }
-                        if (strcmp(st->atypes[i], "void") == 0) {
+                        if (strcmp(st->st_atypes[i], "void") == 0) {
                                 didname = 1;
                         }
-                        if (strcmp(st->atypes[i], "") == 0) {
+                        if (strcmp(st->st_atypes[i], "") == 0) {
                                 didname = 1;
                                 addprogstr("void");
                         }
 
                         /* print the argument list */
-                        for (char *a = st->atypes[i]; *a; a++) {
+                        for (char *a = st->st_atypes[i]; *a; a++) {
                                 if (*a == '(' && a[1] == '*' && !didname) {
                                         addprogfmt("(*a%d", i);
                                         didname = 1;
                                         a++;
                                 } else if (*a == '[' && !didname) {

@@ -393,27 +393,32 @@
                         if (!didname) {
                                 addprogfmt(" a%d", i);
                         }
                 }
 
-                if (st->atypes[0] == NULL) {
+                if (st->st_atypes[0] == NULL) {
                         addprogstr("void");
                 }
 
-                /* close argument list, and closing ")" for func ptrs */
-                addprogfmt(")%s\n{\n\t", s);    /* NB: s is normally empty */
+                /*
+                 * Close argument list, and closing ")" for func ptrs.
+                 * Note that for non-function pointers, s will be empty
+                 * below, otherwise it points to the trailing argument
+                 * list.
+                 */
+                addprogfmt(")%s\n{\n\t", s);
 
-                if (strcmp(st->rtype, "") != 0 &&
-                    strcmp(st->rtype, "void") != 0) {
+                if (strcmp(st->st_rtype, "") != 0 &&
+                    strcmp(st->st_rtype, "void") != 0) {
                         addprogstr("return ");
                 }
 
                 /* add the function call */
-                addprogfmt("%s(", st->name);
-                for (int i = 0; st->atypes[i] != NULL && i < MAXARG; i++) {
-                        if (strcmp(st->atypes[i], "") != 0 &&
-                            strcmp(st->atypes[i], "void") != 0) {
+                addprogfmt("%s(", st->st_name);
+                for (int i = 0; st->st_atypes[i] != NULL && i < MAXARG; i++) {
+                        if (strcmp(st->st_atypes[i], "") != 0 &&
+                            strcmp(st->st_atypes[i], "void") != 0) {
                                 addprogfmt("%sa%d", i > 0 ? ", " : "", i);
                         }
                 }
 
                 addprogstr(");\n}");

@@ -420,18 +425,19 @@
                 break;
         }
 
         addprogch('\n');
 
-        st->prog = progbuf;
+        st->st_prog = progbuf;
 }
 
 static int
 add_envs(struct sym_test *st, char *envs, char **err)
 {
         char *item;
-        if (expand_env_list(envs, &st->test_mask, &st->need_mask, &item) < 0) {
+        if (expand_env_list(envs, &st->st_test_mask, &st->st_need_mask,
+            &item) < 0) {
                 myasprintf(err, "bad env action %s", item);
                 return (-1);
         }
         return (0);
 }

@@ -445,11 +451,11 @@
                 if (i >= MAXHDR) {
                         myasprintf(err, "too many headers");
                         return (-1);
                 }
                 test_trim(&h);
-                st->hdrs[i++] = mystrdup(h);
+                st->st_hdrs[i++] = mystrdup(h);
         }
 
         return (0);
 }
 

@@ -462,11 +468,11 @@
                 if (i >= MAXARG) {
                         myasprintf(err, "too many arguments");
                         return (-1);
                 }
                 test_trim(&a);
-                st->atypes[i++] = mystrdup(a);
+                st->st_atypes[i++] = mystrdup(a);
         }
 
         return (0);
 }
 

@@ -485,13 +491,13 @@
         decl = fields[0];
         hdrs = fields[1];
         envs = fields[2];
 
         st = myzalloc(sizeof (*st));
-        st->type = SYM_TYPE;
-        st->name = mystrdup(decl);
-        st->rtype = mystrdup(decl);
+        st->st_type = SYM_TYPE;
+        st->st_name = mystrdup(decl);
+        st->st_rtype = mystrdup(decl);
 
         if ((add_envs(st, envs, err) < 0) ||
             (add_headers(st, hdrs, err) < 0)) {
                 return (-1);
         }

@@ -517,13 +523,13 @@
         type = fields[1];
         hdrs = fields[2];
         envs = fields[3];
 
         st = myzalloc(sizeof (*st));
-        st->type = SYM_VALUE;
-        st->name = mystrdup(name);
-        st->rtype = mystrdup(type);
+        st->st_type = SYM_VALUE;
+        st->st_name = mystrdup(name);
+        st->st_rtype = mystrdup(type);
 
         if ((add_envs(st, envs, err) < 0) ||
             (add_headers(st, hdrs, err) < 0)) {
                 return (-1);
         }

@@ -551,13 +557,13 @@
         atype = fields[2];
         hdrs = fields[3];
         envs = fields[4];
 
         st = myzalloc(sizeof (*st));
-        st->type = SYM_FUNC;
-        st->name = mystrdup(name);
-        st->rtype = mystrdup(rtype);
+        st->st_type = SYM_FUNC;
+        st->st_name = mystrdup(name);
+        st->st_rtype = mystrdup(rtype);
 
         if ((add_envs(st, envs, err) < 0) ||
             (add_headers(st, hdrs, err) < 0) ||
             (add_arg_types(st, atype, err) < 0)) {
                 return (-1);

@@ -568,26 +574,26 @@
 }
 
 struct sym_test *
 next_sym_test(struct sym_test *st)
 {
-        return (st == NULL ? sym_tests : st->next);
+        return (st == NULL ? sym_tests : st->st_next);
 }
 
 const char *
 sym_test_prog(struct sym_test *st)
 {
-        if (st->prog == NULL) {
+        if (st->st_prog == NULL) {
                 mkprog(st);
         }
-        return (st->prog);
+        return (st->st_prog);
 }
 
 const char *
 sym_test_name(struct sym_test *st)
 {
-        return (st->name);
+        return (st->st_name);
 }
 
 /*
  * Iterate through tests.  Pass in NULL for cenv to begin the iteration. For
  * subsequent iterations, use the return value from the previous iteration.

@@ -594,17 +600,17 @@
  * Returns NULL when there are no more environments.
  */
 struct compile_env *
 sym_test_env(struct sym_test *st, struct compile_env *cenv, int *need)
 {
-        int i = cenv ? cenv->index + 1: 0;
+        int i = cenv ? cenv->ce_index + 1: 0;
         uint64_t b = 1ULL << i;
 
         while ((i < MAXENV) && (b != 0)) {
                 cenv = &compile_env[i];
-                if (b & st->test_mask) {
-                        *need = (st->need_mask & b) ? 1 : 0;
+                if (b & st->st_test_mask) {
+                        *need = (st->st_need_mask & b) ? 1 : 0;
                         return (cenv);
                 }
                 b <<= 1;
                 i++;
         }

@@ -612,23 +618,23 @@
 }
 
 const char *
 env_name(struct compile_env *cenv)
 {
-        return (cenv->name);
+        return (cenv->ce_name);
 }
 
 const char *
 env_lang(struct compile_env *cenv)
 {
-        return (cenv->lang);
+        return (cenv->ce_lang);
 }
 
 const char *
 env_defs(struct compile_env *cenv)
 {
-        return (cenv->defs);
+        return (cenv->ce_defs);
 }
 
 static void
 show_file(test_t t, const char *path)
 {

@@ -770,10 +776,13 @@
                                 test_debugf(t, "c89flags: %s", c89flags);
                                 test_debugf(t, "c99flags: %s", c99flags);
                         }
                         test_passed(t);
                         break;
+                case 99:
+                        test_debugf(t, "Found unknown (unsupported) compiler");
+                        continue;
                 default:
                         continue;
                 }
                 myasprintf(&compiler, "%s", compilers[i]);
                 test_debugf(t, "compiler: %s", compiler);

@@ -867,11 +876,11 @@
                 }
                 /* XXX: we really want a sym_test_desc() */
                 for (cenv = sym_test_env(st, NULL, &need);
                     cenv != NULL;
                     cenv = sym_test_env(st, cenv, &need)) {
-                        t = test_start("%s : %c%s", st->name,
+                        t = test_start("%s : %c%s", sym_test_name(st),
                             need ? '+' : '-', env_name(cenv));
                         if (do_compile(t, st, cenv, need) == 0) {
                                 test_passed(t);
                         }
                 }