From 7bb7cd58c2d9df126dd6072e5f3bec1eb4dc916b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 17 Jan 2011 16:17:49 +1030 Subject: [PATCH] ccanlint: score_file_error() takes printf-format 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. --- tools/ccanlint/ccanlint.c | 20 +------------ tools/ccanlint/ccanlint.h | 9 +++--- tools/ccanlint/compulsory_tests/info_exists.c | 5 ++-- .../ccanlint/compulsory_tests/objects_build.c | 11 +++++--- tools/ccanlint/file_analysis.c | 28 +++++++++++++++++-- tools/ccanlint/tests/depends_accurate.c | 6 ++-- tools/ccanlint/tests/examples_compile.c | 22 +++++++-------- tools/ccanlint/tests/examples_exist.c | 1 - tools/ccanlint/tests/headers_idempotent.c | 27 ++++++++---------- .../tests/info_documentation_exists.c | 10 ++++--- tools/ccanlint/tests/license_exists.c | 5 ++-- tools/ccanlint/tests/no_trailing_whitespace.c | 5 +--- tools/ccanlint/tests/tests_compile.c | 25 ++++++++++------- tools/ccanlint/tests/tests_compile_coverage.c | 26 ++++++++--------- tools/ccanlint/tests/tests_coverage.c | 5 ++-- tools/ccanlint/tests/tests_exist.c | 10 ++++--- tools/ccanlint/tests/tests_helpers_compile.c | 8 +++--- 17 files changed, 116 insertions(+), 107 deletions(-) diff --git a/tools/ccanlint/ccanlint.c b/tools/ccanlint/ccanlint.c index 6bfc6d15..1b11803c 100644 --- a/tools/ccanlint/ccanlint.c +++ b/tools/ccanlint/ccanlint.c @@ -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); } diff --git a/tools/ccanlint/ccanlint.h b/tools/ccanlint/ccanlint.h index 19d53dbf..40cb4195 100644 --- a/tools/ccanlint/ccanlint.h +++ b/tools/ccanlint/ccanlint.h @@ -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; diff --git a/tools/ccanlint/compulsory_tests/info_exists.c b/tools/ccanlint/compulsory_tests/info_exists.c index a9a6c1c9..ac840119 100644 --- a/tools/ccanlint/compulsory_tests/info_exists.c +++ b/tools/ccanlint/compulsory_tests/info_exists.c @@ -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"); } } diff --git a/tools/ccanlint/compulsory_tests/objects_build.c b/tools/ccanlint/compulsory_tests/objects_build.c index 13d34a14..32546e59 100644 --- a/tools/ccanlint/compulsory_tests/objects_build.c +++ b/tools/ccanlint/compulsory_tests/objects_build.c @@ -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; } } diff --git a/tools/ccanlint/file_analysis.c b/tools/ccanlint/file_analysis.c index 14b2631e..b941f94e 100644 --- a/tools/ccanlint/file_analysis.c +++ b/tools/ccanlint/file_analysis.c @@ -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"); } diff --git a/tools/ccanlint/tests/depends_accurate.c b/tools/ccanlint/tests/depends_accurate.c index 34db4f84..3ab6933e 100644 --- a/tools/ccanlint/tests/depends_accurate.c +++ b/tools/ccanlint/tests/depends_accurate.c @@ -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); } } } diff --git a/tools/ccanlint/tests/examples_compile.c b/tools/ccanlint/tests/examples_compile.c index a3ab73a9..1ccc6202 100644 --- a/tools/ccanlint/tests/examples_compile.c +++ b/tools/ccanlint/tests/examples_compile.c @@ -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 diff --git a/tools/ccanlint/tests/examples_exist.c b/tools/ccanlint/tests/examples_exist.c index 947d76a4..2091f67f 100644 --- a/tools/ccanlint/tests/examples_exist.c +++ b/tools/ccanlint/tests/examples_exist.c @@ -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) diff --git a/tools/ccanlint/tests/headers_idempotent.c b/tools/ccanlint/tests/headers_idempotent.c index 340a0698..a1d30961 100644 --- a/tools/ccanlint/tests/headers_idempotent.c +++ b/tools/ccanlint/tests/headers_idempotent.c @@ -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 */ @@ -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; diff --git a/tools/ccanlint/tests/info_documentation_exists.c b/tools/ccanlint/tests/info_documentation_exists.c index a5316fe0..108f57f1 100644 --- a/tools/ccanlint/tests/info_documentation_exists.c +++ b/tools/ccanlint/tests/info_documentation_exists.c @@ -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"); } } diff --git a/tools/ccanlint/tests/license_exists.c b/tools/ccanlint/tests/license_exists.c index 965a159f..72461bec 100644 --- a/tools/ccanlint/tests/license_exists.c +++ b/tools/ccanlint/tests/license_exists.c @@ -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; diff --git a/tools/ccanlint/tests/no_trailing_whitespace.c b/tools/ccanlint/tests/no_trailing_whitespace.c index a66fd74c..1018be3c 100644 --- a/tools/ccanlint/tests/no_trailing_whitespace.c +++ b/tools/ccanlint/tests/no_trailing_whitespace.c @@ -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); - } } } } diff --git a/tools/ccanlint/tests/tests_compile.c b/tools/ccanlint/tests/tests_compile.c index 4d868679..473d55f8 100644 --- a/tools/ccanlint/tests/tests_compile.c +++ b/tools/ccanlint/tests/tests_compile.c @@ -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; } } diff --git a/tools/ccanlint/tests/tests_compile_coverage.c b/tools/ccanlint/tests/tests_compile_coverage.c index 115ae94a..f41edc0b 100644 --- a/tools/ccanlint/tests/tests_compile_coverage.c +++ b/tools/ccanlint/tests/tests_compile_coverage.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -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) { diff --git a/tools/ccanlint/tests/tests_coverage.c b/tools/ccanlint/tests/tests_coverage.c index af8d6f21..7943071b 100644 --- a/tools/ccanlint/tests/tests_coverage.c +++ b/tools/ccanlint/tests/tests_coverage.c @@ -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; } } diff --git a/tools/ccanlint/tests/tests_exist.c b/tools/ccanlint/tests/tests_exist.c index bdfe49d5..64b2691e 100644 --- a/tools/ccanlint/tests/tests_exist.c +++ b/tools/ccanlint/tests/tests_exist.c @@ -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; diff --git a/tools/ccanlint/tests/tests_helpers_compile.c b/tools/ccanlint/tests/tests_helpers_compile.c index 0ad5a7e6..67ca8d8a 100644 --- a/tools/ccanlint/tests/tests_helpers_compile.c +++ b/tools/ccanlint/tests/tests_helpers_compile.c @@ -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); } } -- 2.39.2