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.

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 <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;
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 <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) {
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);
 		}
 	}