ccanlint: score_file_error() takes printf-format
authorRusty Russell <rusty@rustcorp.com.au>
Mon, 17 Jan 2011 05:47:49 +0000 (16:17 +1030)
committerRusty Russell <rusty@rustcorp.com.au>
Mon, 17 Jan 2011 05:47:49 +0000 (16:17 +1030)
We simply build up the error string in score_file_error; a bit different
but simpler than current behaviour.  We keep around struct file_error
because some tests need it.

17 files changed:
tools/ccanlint/ccanlint.c
tools/ccanlint/ccanlint.h
tools/ccanlint/compulsory_tests/info_exists.c
tools/ccanlint/compulsory_tests/objects_build.c
tools/ccanlint/file_analysis.c
tools/ccanlint/tests/depends_accurate.c
tools/ccanlint/tests/examples_compile.c
tools/ccanlint/tests/examples_exist.c
tools/ccanlint/tests/headers_idempotent.c
tools/ccanlint/tests/info_documentation_exists.c
tools/ccanlint/tests/license_exists.c
tools/ccanlint/tests/no_trailing_whitespace.c
tools/ccanlint/tests/tests_compile.c
tools/ccanlint/tests/tests_compile_coverage.c
tools/ccanlint/tests/tests_coverage.c
tools/ccanlint/tests/tests_exist.c
tools/ccanlint/tests/tests_helpers_compile.c

index 6bfc6d15bd1debf7ad7355da0ce3ba2181e2dde8..1b11803c220e7e38b5537cb3c4a065aecdfafbc0 100644 (file)
@@ -149,26 +149,8 @@ static bool run_test(struct ccanlint *i,
        }
 
        if ((!quiet && !score->pass) || verbose) {
-               struct file_error *f;
-               unsigned int lines = 1;
-
                if (score->error)
-                       printf("%s%s\n", score->error,
-                              list_empty(&score->per_file_errors) ? "" : ":");
-
-               list_for_each(&score->per_file_errors, f, list) {
-                       if (f->line)
-                               printf("%s:%u:%s\n",
-                                      f->file->fullname, f->line, f->error);
-                       else if (f->file)
-                               printf("%s:%s\n", f->file->fullname, f->error);
-                       else
-                               printf("%s\n", f->error);
-                       if (verbose < 2 && ++lines > 5) {
-                               printf("... more (use -vv to see them all)\n");
-                               break;
-                       }
-               }
+                       printf("%s", score->error);
                if (!quiet && !score->pass && i->handle)
                        i->handle(m, score);
        }
index 19d53dbf12e4f5fb7bcd340e65087d10a452ebfb..40cb4195ffc65263ee2ef2bbb936fa3fd9e31707 100644 (file)
@@ -49,14 +49,13 @@ struct manifest *get_manifest(const void *ctx, const char *dir);
 struct file_error {
        struct list_node list;
        struct ccan_file *file;
-       unsigned int line; /* 0 not to print */
-       const char *error;
+       unsigned int line;
 };
 
 struct score {
        bool pass;
        unsigned int score, total;
-       const char *error;
+       char *error;
        struct list_head per_file_errors;
 };
 
@@ -203,9 +202,9 @@ char *get_symbol_token(void *ctx, const char **line);
 /* Similarly for ->doc_sections */
 struct list_head *get_ccan_file_docs(struct ccan_file *f);
 
-/* Add an error about this file (and line, if non-zero) to the score struct */
+/* Append message about this file (and line, if non-zero) to the score->error */
 void score_file_error(struct score *, struct ccan_file *f, unsigned line,
-                     const char *error);
+                     const char *errorfmt, ...);
 
 /* Normal tests. */
 extern struct ccanlint trailing_whitespace;
index a9a6c1c993a77c532515fe5c92eef282cccafba9..ac84011978e86edbd0209270b8b07f68ae3d5b21 100644 (file)
@@ -21,10 +21,11 @@ static void check_has_info(struct manifest *m,
                score->pass = true;
                score->score = score->total;
        } else {
-               score->error = "You have no _info file.\n\n"
+               score->error = talloc_strdup(score,
+       "You have no _info file.\n\n"
        "The file _info contains the metadata for a ccan package: things\n"
        "like the dependencies, the documentation for the package as a whole\n"
-       "and license information.\n";
+       "and license information.\n");
        }
 }
 
index 13d34a14e041fb90b2fcbba7df020c06805aec31..32546e591732ba080ed439d89b34d53e7dfd86da 100644 (file)
@@ -41,12 +41,15 @@ static void check_objs_build(struct manifest *m,
                if (!compile_object(score, fullfile, ccan_dir, "", i->compiled,
                                    &output)) {
                        talloc_free(i->compiled);
-                       score->error = "Compiling object files";
-                       score_file_error(score, i, 0, output);
+                       score_file_error(score, i, 0,
+                                        "Compiling object files:\n%s",
+                                        output);
                        errors = true;
                } else if (!streq(output, "")) {
-                       score->error = "Compiling object files gave warnings";
-                       score_file_error(score, i, 0, output);
+                       score_file_error(score, i, 0,
+                                        "Compiling object files gave"
+                                        " warnings:\n%s",
+                                        output);
                        warnings = true;
                }
        }
index 14b2631e1ef630671748a78441e35995663659ce..b941f94e1c31e84c765b522f36da38338b7bca0b 100644 (file)
@@ -617,11 +617,35 @@ enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
 }
 
 void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
-                     const char *error)
+                     const char *errorfmt, ...)
 {
+       va_list ap;
+
        struct file_error *fe = talloc(score, struct file_error);
        fe->file = f;
        fe->line = line;
-       fe->error = error;
        list_add_tail(&score->per_file_errors, &fe->list);
+
+       if (!score->error)
+               score->error = talloc_strdup(score, "");
+       
+       if (verbose < 2 && strcount(score->error, "\n") > 5)
+               return;
+
+       if (line)
+               score->error = talloc_asprintf_append(score->error,
+                                                     "%s:%u:",
+                                                     f->fullname, line);
+       else
+               score->error = talloc_asprintf_append(score->error,
+                                                     "%s:", f->fullname);
+
+       va_start(ap, errorfmt);
+       score->error = talloc_vasprintf_append(score->error, errorfmt, ap);
+       va_end(ap);
+       score->error = talloc_append_string(score->error, "\n");
+
+       if (verbose < 2 && strcount(score->error, "\n") > 5)
+               score->error = talloc_append_string(score->error,
+                                   "... more (use -vv to see them all)\n");
 }
index 34db4f84d0dd75b9d402f7ead9bc798cdf88268b..3ab6933ee7e4c57825d447ac771d9161a27b9500 100644 (file)
@@ -55,9 +55,9 @@ static void check_depends_accurate(struct manifest *m,
                                        continue;
                                if (has_dep(m, mod))
                                        continue;
-                               score->error = "Includes a ccan module"
-                                       " not listed in _info";
-                               score_file_error(score, f, i+1, lines[i]);
+                               score_file_error(score, f, i+1,
+                                                "%s not listed in _info",
+                                                mod);
                        }
                }
        }
index a3ab73a967e0557f567a00f02d518aa26032bcff..1ccc620248cadd3b0723de8ee01fcbef8732e687 100644 (file)
@@ -542,25 +542,23 @@ static void build_examples(struct manifest *m, bool keep,
                                        prev = lines[j];
                                score->score++;
                                warnings = true;
-                               score->error = "Compiling extracted example"
-                                       " gave warnings";
-                               error = talloc_asprintf(score,
-                                       "Example:\n"
-                                       "%s\n"
-                                       "Compiler:\n"
-                                       "%s",
-                                       get_ccan_file_contents(file[j]),
-                                       err[j]);
-                               score_file_error(score, file[j], 0, error);
+                               score_file_error(score, file[j], 0,
+                                                "Compiling extracted example"
+                                                " gave warnings:\n"
+                                                "Example:\n"
+                                                "%s\n"
+                                                "Compiler:\n"
+                                                "%s",
+                                                get_ccan_file_contents(file[j]),
+                                                err[j]);
                                goto next;
                        }
                }
 
                score->pass = false;
-               score->error = "Compiling extracted examples failed";
                if (!verbose) {
                        if (num == 3)
-                               error = "Standalone, adding headers, "
+                               error = "Compiling standalone, adding headers, "
                                        "and including previous "
                                        "example all failed";
                        else
index 947d76a4a768751053e6830b4bb788cb2c56df77..2091f67f0f97c82218b57d77a380ee3528eedc4f 100644 (file)
@@ -98,7 +98,6 @@ static void extract_examples(struct manifest *m,
                return;
        }
 
-       score->error = "Expect examples in header and _info";
        if (!have_info_example)
                score_file_error(score, m->info_file, 0, "No Example: section");
        if (!have_header_example)
index 340a0698ac6aecbcb8baf85d231ce4e4d8d978fc..a1d30961f2035bc0dadbd9d557d9444f78f2edd5 100644 (file)
@@ -80,7 +80,7 @@ static void handle_idem(struct manifest *m, struct score *score)
        }
 }
 
-static bool check_idem(struct ccan_file *f, struct score *score)
+static void check_idem(struct ccan_file *f, struct score *score)
 {
        struct line_info *line_info;
        unsigned int i, first_preproc_line;
@@ -89,7 +89,7 @@ static bool check_idem(struct ccan_file *f, struct score *score)
        line_info = get_ccan_line_info(f);
        if (f->num_lines < 3)
                /* FIXME: We assume small headers probably uninteresting. */
-               return true;
+               return;
 
        for (i = 0; i < f->num_lines; i++) {
                if (line_info[i].type == DOC_LINE
@@ -99,14 +99,14 @@ static bool check_idem(struct ccan_file *f, struct score *score)
                        score_file_error(score, f, i+1,
                                         "Expect first non-comment line to be"
                                         " #ifndef.");
-                       return false;
+                       return;
                } else if (line_info[i].type == PREPROC_LINE)
                        break;
        }
 
        /* No code at all?  Don't complain. */
        if (i == f->num_lines)
-               return true;
+               return;
 
        first_preproc_line = i;
        for (i = first_preproc_line+1; i < f->num_lines; i++) {
@@ -117,19 +117,19 @@ static bool check_idem(struct ccan_file *f, struct score *score)
                        score_file_error(score, f, i+1,
                                         "Expect second non-comment line to be"
                                         " #define.");
-                       return false;
+                       return;
                } else if (line_info[i].type == PREPROC_LINE)
                        break;
        }
 
        /* No code at all?  Weird. */
        if (i == f->num_lines)
-               return true;
+               return;
 
        /* We expect a condition on this line. */
        if (!line_info[i].cond) {
                score_file_error(score, f, i+1, "Expected #ifndef");
-               return false;
+               return;
        }
 
        line = f->lines[i];
@@ -138,7 +138,7 @@ static bool check_idem(struct ccan_file *f, struct score *score)
        if (line_info[i].cond->type != PP_COND_IFDEF
            || !line_info[i].cond->inverse) {
                score_file_error(score, f, i+1, "Expected #ifndef");
-               return false;
+               return;
        }
 
        /* And this to be #define <symbol> */
@@ -149,7 +149,7 @@ static bool check_idem(struct ccan_file *f, struct score *score)
                                            "expected '#define %s'",
                                            line_info[i].cond->symbol);
                score_file_error(score, f, i+1, str);
-               return false;
+               return;
        }
        sym = get_symbol_token(f, &line);
        if (!sym || !streq(sym, line_info[i].cond->symbol)) {
@@ -157,7 +157,7 @@ static bool check_idem(struct ccan_file *f, struct score *score)
                                            "expected '#define %s'",
                                            line_info[i].cond->symbol);
                score_file_error(score, f, i+1, str);
-               return false;
+               return;
        }
 
        /* Rest of code should all be covered by that conditional. */
@@ -170,11 +170,9 @@ static bool check_idem(struct ccan_file *f, struct score *score)
                    != NOT_COMPILED) {
                        score_file_error(score, f, i+1, "code outside"
                                         " idempotent region");
-                       return false;
+                       return;
                }
        }
-
-       return true;
 }
 
 static void check_idempotent(struct manifest *m,
@@ -184,8 +182,7 @@ static void check_idempotent(struct manifest *m,
        struct ccan_file *f;
 
        list_for_each(&m->h_files, f, list) {
-               if (!check_idem(f, score))
-                       score->error = "Headers are not idempotent";
+               check_idem(f, score);
        }
        if (!score->error) {
                score->pass = true;
index a5316fe00d0ae3dba4a2a2287a51ce1dfeb17faa..108f57f1e935255ff9bf00e48c611e0b68f523b7 100644 (file)
@@ -80,15 +80,17 @@ static void check_info_documentation_exists(struct manifest *m,
                score->score = score->total;
                score->pass = true;
        } else if (!summary) {
-               score->error = "_info file has no module documentation.\n\n"
+               score->error = talloc_strdup(score,
+               "_info file has no module documentation.\n\n"
                "CCAN modules use /**-style comments for documentation: the\n"
-               "overall documentation belongs in the _info metafile.\n";
+               "overall documentation belongs in the _info metafile.\n");
                info_documentation_exists.handle = create_info_template_doc;
        } else if (!description)  {
-               score->error = "_info file has no module description.\n\n"
+               score->error = talloc_strdup(score,
+               "_info file has no module description.\n\n"
                "The lines after the first summary line in the _info file\n"
                "documentation should describe the purpose and use of the\n"
-               "overall package\n";
+               "overall package\n");
        }
 }
 
index 965a159ff8fff53f15bea96d4675c052eedbd9f0..72461bec6d40ab4b3534385dc1c1c3bc932c3c9f 100644 (file)
@@ -90,7 +90,7 @@ static void check_has_license(struct manifest *m,
 
        d = find_license(m);
        if (!d) {
-               score->error = "No License: tag in _info";
+               score->error = talloc_strdup(score, "No License: tag in _info");
                return;
        }
        expected = expected_link(m, d);
@@ -111,7 +111,8 @@ static void check_has_license(struct manifest *m,
                        return;
                }
                if (errno == ENOENT) {
-                       score->error = "LICENSE does not exist";
+                       score->error = talloc_strdup(score,
+                                                    "LICENSE does not exist");
                        if (expected)
                                has_license.handle = handle_license_link;
                        return;
index a66fd74c3c57a8fdd7f42f352975bbe681366665..1018be3cd64938788a47019f71b11c832755fa8b 100644 (file)
@@ -35,11 +35,8 @@ static void check_trailing_whitespace(struct manifest *m,
                        char **lines = get_ccan_file_lines(f);
                        for (i = 0; i < f->num_lines; i++) {
                                char *err = get_trailing_whitespace(lines[i]);
-                               if (err) {
-                                       score->error = "Trailing whitespace"
-                                               " found";
+                               if (err)
                                        score_file_error(score, f, i+1, err);
-                               }
                        }
                }
        }
index 4d8686798c0aa12040defb489cd28ebc4d7acd1b..473d55f821b668368a2aa673b16ded2096ca49e9 100644 (file)
@@ -90,12 +90,14 @@ static void do_compile_tests(struct manifest *m,
                list_for_each(list, i, list) {
                        if (!compile(score, m, i, false, list == &m->api_tests,
                                     keep, &cmdout)) {
-                               score->error = "Failed to compile tests";
-                               score_file_error(score, i, 0, cmdout);
+                               score_file_error(score, i, 0,
+                                                "Compile failed:\n%s",
+                                                cmdout);
                                errors = true;
                        } else if (!streq(cmdout, "")) {
-                               score->error = "Test compiled with warnings";
-                               score_file_error(score, i, 0, cmdout);
+                               score_file_error(score, i, 0,
+                                                "Compile gave warnings:\n%s",
+                                                cmdout);
                                warnings = true;
                        }
                }
@@ -108,19 +110,22 @@ static void do_compile_tests(struct manifest *m,
        /* For historical reasons, "fail" often means "gives warnings" */
        list_for_each(&m->compile_fail_tests, i, list) {
                if (!compile(score, m, i, false, false, false, &cmdout)) {
-                       score->error = "Failed to compile without -DFAIL";
-                       score_file_error(score, i, 0, cmdout);
+                       score_file_error(score, i, 0,
+                                        "Compile without -DFAIL failed:\n%s",
+                                        cmdout);
                        return;
                }
                if (!streq(cmdout, "")) {
-                       score->error = "Compile with warnigns without -DFAIL";
-                       score_file_error(score, i, 0, cmdout);
+                       score_file_error(score, i, 0,
+                                        "Compile gave warnings"
+                                        " without -DFAIL:\n%s",
+                                        cmdout);
                        return;
                }
                if (compile(score, m, i, true, false, false, &cmdout)
                    && streq(cmdout, "")) {
-                       score->error = "Compiled successfully with -DFAIL?";
-                       score_file_error(score, i, 0, NULL);
+                       score_file_error(score, i, 0,
+                                        "Compiled successfully with -DFAIL?");
                        return;
                }
        }
index 115ae94a15e50a7f4050f2b307e5bdb18295b944..f41edc0bc6f36588a334a07c923707bbc10421d5 100644 (file)
@@ -2,6 +2,7 @@
 #include <tools/tools.h>
 #include <ccan/talloc/talloc.h>
 #include <ccan/str/str.h>
+#include <ccan/foreach/foreach.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -114,26 +115,23 @@ static void do_compile_coverage_tests(struct manifest *m,
 {
        char *cmdout, *modobjs = NULL;
        struct ccan_file *i;
+       struct list_head *h;
 
        if (!list_empty(&m->api_tests)
            && !build_module_objs_with_coverage(m, keep, score, &modobjs)) {
-               score->error = "Failed to compile module objects with coverage";
+               score->error = talloc_strdup(score,
+                            "Failed to compile module objects with coverage");
                return;
        }
 
-       list_for_each(&m->run_tests, i, list) {
-               cmdout = cov_compile(m, m, i, NULL, keep);
-               if (cmdout) {
-                       score->error = "Failed to compile test with coverage";
-                       score_file_error(score, i, 0, cmdout);
-               }
-       }
-
-       list_for_each(&m->api_tests, i, list) {
-               cmdout = cov_compile(m, m, i, modobjs, keep);
-               if (cmdout) {
-                       score->error = "Failed to compile test with coverage";
-                       score_file_error(score, i, 0, cmdout);
+       foreach_ptr(h, &m->run_tests, &m->api_tests) {
+               list_for_each(h, i, list) {
+                       cmdout = cov_compile(m, m, i, NULL, keep);
+                       if (cmdout) {
+                               score_file_error(score, i, 0,
+                                 "Failed to compile test with coverage: %s",
+                                 cmdout);
+                       }
                }
        }
        if (!score->error) {
index af8d6f21c845948fbbae3cd1413cbfc6ac8e2938..7943071b76634bfc50ce88e962ba8dfffc4480e6 100644 (file)
@@ -145,8 +145,9 @@ static void do_run_coverage_tests(struct manifest *m,
                                covcmd = talloc_asprintf_append(covcmd, " %s",
                                                                i->fullname);
                        } else {
-                               score->error = "Running test with coverage";
-                               score_file_error(score, i, 0, cmdout);
+                               score_file_error(score, i, 0,
+                                                "Running test with coverage"
+                                                " failed: %s", cmdout);
                                return;
                        }
                }
index bdfe49d56b8725389b279671052fd8356f499a06..64b2691e0eaceba570e0e7595cda0a543b3e134d 100644 (file)
@@ -100,7 +100,7 @@ static void check_tests_exist(struct manifest *m,
        char *test_dir = talloc_asprintf(m, "%s/test", m->dir);
 
        if (lstat(test_dir, &st) != 0) {
-               score->error = "No test directory";
+               score->error = talloc_strdup(score, "No test directory");
                if (errno != ENOENT)
                        err(1, "statting %s", test_dir);
                tests_exist.handle = handle_no_tests;
@@ -108,7 +108,7 @@ static void check_tests_exist(struct manifest *m,
        }
 
        if (!S_ISDIR(st.st_mode)) {
-               score->error = "test is not a directory";
+               score->error = talloc_strdup(score, "test is not a directory");
                return;
        }
 
@@ -116,10 +116,12 @@ static void check_tests_exist(struct manifest *m,
            && list_empty(&m->run_tests)
            && list_empty(&m->compile_ok_tests)) {
                if (list_empty(&m->compile_fail_tests)) {
-                       score->error = "No tests in test directory";
+                       score->error = talloc_strdup(score,
+                                            "No tests in test directory");
                        tests_exist.handle = handle_no_tests;
                } else
-                       score->error = "No positive tests in test directory";
+                       score->error = talloc_strdup(score,
+                                    "No positive tests in test directory");
                return;
        }
        score->pass = true;
index 0ad5a7e67cc7847fb9bbc9c568ef403b7752eb7d..67ca8d8ae332f192f568272909befb7ac2281aaa 100644 (file)
@@ -49,12 +49,12 @@ static void do_compile_test_helpers(struct manifest *m,
 
                if (!compile(m, keep, i, &cmdout)) {
                        errors = true;
-                       score->error = "Failed to compile helper C files";
-                       score_file_error(score, i, 0, cmdout);
+                       score_file_error(score, i, 0, "Compile failed:\n%s",
+                                        cmdout);
                } else if (!streq(cmdout, "")) {
                        warnings = true;
-                       score->error = "Helper C files gave warnings";
-                       score_file_error(score, i, 0, cmdout);
+                       score_file_error(score, i, 0,
+                                        "Compile gave warnings:\n%s", cmdout);
                }
        }