ccanlint: rework so checks have more structure.

Previously each check returned a void *, but in fact most of them fell into
similar patterns.  So define 'struct score' and a helper to add files to it,
and use that.

Under these rules, you get 0/1 if you skip a test because a dependency failed
which in theory means your score (as a percentage) could drop if you fix
a test.

tools/ccanlint/ccanlint.c

@@ -88,30 +88,37 @@ static const char *should_skip(struct manifest *m, struct ccanlint *i)
 
 static bool run_test(struct ccanlint *i,
 		     bool quiet,
-		     unsigned int *score,
-		     unsigned int *total_score,
+		     unsigned int *running_score,
+		     unsigned int *running_total,
 		     struct manifest *m)
 {
-	void *result;
-	unsigned int this_score, max_score, timeleft;
+	unsigned int timeleft;
 	const struct dependent *d;
 	const char *skip;
-	bool bad, good;
+	struct score *score;
 
 	//one less test to run through
 	list_for_each(&i->dependencies, d, node)
 		d->dependent->num_depends--;
 
+	score = talloc(m, struct score);
+	list_head_init(&score->per_file_errors);
+	score->error = NULL;
+	score->pass = false;
+	score->score = 0;
+	score->total = 1;
+
 	skip = should_skip(m, i);
 
 	if (skip) {
 	skip:
 		if (verbose && !streq(skip, "not relevant to target"))
-			printf("  %s: skipped (%s)\n", i->name, skip);
+			printf("%s: skipped (%s)\n", i->name, skip);
 
-		/* If we're skipping this because a prereq failed, we fail. */
+		/* If we're skipping this because a prereq failed, we fail:
+		 * count it as a score of 1. */
 		if (i->skip_fail)
-			*total_score += i->total_score;
+			(*running_total)++;
 			
 		list_del(&i->list);
 		list_add_tail(&finished_tests, &i->list);
@@ -121,55 +128,52 @@ static bool run_test(struct ccanlint *i,
 			d->dependent->skip = "dependency was skipped";
 			d->dependent->skip_fail = i->skip_fail;
 		}
-		return true;
+		return i->skip_fail ? false : true;
 	}
 
 	timeleft = timeout ? timeout : default_timeout_ms;
-	result = i->check(m, i->keep_results, &timeleft);
+	i->check(m, i->keep_results, &timeleft, score);
 	if (timeout && timeleft == 0) {
 		skip = "timeout";
 		goto skip;
 	}
 
-	max_score = i->total_score;
-	if (!max_score)
-		max_score = 1;
-
-	if (!result)
-		this_score = max_score;
-	else if (i->score)
-		this_score = i->score(m, result);
-	else
-		this_score = 0;
-
-	bad = (this_score == 0);
-	good = (this_score >= max_score);
-
-	if (verbose || (!good && !quiet)) {
-		printf("  %s: %s", i->name,
-		       bad ? "FAIL" : good ? "PASS" : "PARTIAL");
-		if (max_score > 1)
-			printf(" (+%u/%u)", this_score, max_score);
+	assert(score->score <= score->total);
+	if ((!score->pass && !quiet)
+	    || (score->score < score->total && verbose)
+	    || verbose > 1) {
+		printf("%s: %s", i->name, score->pass ? "PASS" : "FAIL");
+		if (score->total > 1)
+			printf(" (+%u/%u)", score->score, score->total);
 		printf("\n");
 	}
 
-	if (!quiet && result) {
-		const char *desc;
-		if (i->describe && (desc = i->describe(m, result)) != NULL) 
-			printf("    %s\n", desc);
+	if (!quiet && !score->pass) {
+		struct file_error *f;
+
+		if (score->error)
+			printf("%s:\n", score->error);
+
+		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 (i->handle)
-			i->handle(m, result);
+			i->handle(m, score);
 	}
 
-	if (i->total_score) {
-		*score += this_score;
-		*total_score += i->total_score;
-	}
+	*running_score += score->score;
+	*running_total += score->total;
 
 	list_del(&i->list);
 	list_add_tail(&finished_tests, &i->list);
 
-	if (bad) {
+	if (!score->pass) {
 		/* Skip any tests which depend on this one. */
 		list_for_each(&i->dependencies, d, node) {
 			if (d->dependent->skip)
@@ -178,7 +182,7 @@ static bool run_test(struct ccanlint *i,
 			d->dependent->skip_fail = true;
 		}
 	}
-	return good;
+	return score->pass;
 }
 
 static void register_test(struct list_head *h, struct ccanlint *test, ...)
@@ -451,9 +455,6 @@ int main(int argc, char *argv[])
 	}
 
 	/* If you don't pass the compulsory tests, you get a score of 0. */
-	if (verbose)
-		printf("Compulsory tests:\n");
-
 	while ((i = get_next_test(&compulsory_tests)) != NULL) {
 		if (!run_test(i, summary, &score, &total_score, m)) {
 			printf("%sTotal score: 0/%u\n", prefix, total_score);
@@ -462,9 +463,6 @@ int main(int argc, char *argv[])
 	}
 
 	add_info_fails(m->info_file);
-
-	if (verbose)
-		printf("\nNormal tests:\n");
 	while ((i = get_next_test(&normal_tests)) != NULL)
 		run_test(i, summary, &score, &total_score, m);
 

tools/ccanlint/ccanlint.h

@@ -42,6 +42,20 @@ struct manifest {
 
 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;
+};
+
+struct score {
+	bool pass;
+	unsigned int score, total;
+	const char *error;
+	struct list_head per_file_errors;
+};
+
 struct ccanlint {
 	struct list_node list;
 
@@ -51,27 +65,18 @@ struct ccanlint {
 	/* Unique name of test */
 	const char *name;
 
-	/* Total score that this test is worth. */
-	unsigned int total_score;
-
 	/* Can we run this test?  Return string explaining why, if not. */
 	const char *(*can_run)(struct manifest *m);
 
-	/* If this returns non-NULL, it means the check failed.
-	 * keep is set if you should keep the results.
-	 * If timeleft is set to 0, means it timed out. */
-	void *(*check)(struct manifest *m, bool keep, unsigned int *timeleft);
-
-	/* The non-NULL return from check is passed to one of these: */
-
-	/* So, what did this get out of the total_score?  (NULL means 0). */
-	unsigned int (*score)(struct manifest *m, void *check_result);
-
-	/* Verbose description of what was wrong. */
-	const char *(*describe)(struct manifest *m, void *check_result);
+	/* keep is set if you should keep the results.
+	 * If timeleft is set to 0, means it timed out.
+	 * score is the result, and a talloc context freed after all our
+	 * depends are done. */
+	void (*check)(struct manifest *m,
+		      bool keep, unsigned int *timeleft, struct score *score);
 
 	/* Can we do something about it? (NULL if not) */
-	void (*handle)(struct manifest *m, void *check_result);
+	void (*handle)(struct manifest *m, struct score *score);
 
 	/* Internal use fields: */
 	/* Who depends on us? */
@@ -184,11 +189,9 @@ char *get_symbol_token(void *ctx, const char **line);
 struct list_head *get_ccan_file_docs(struct ccan_file *f);
 
 
-/* Call the reporting on every line in the file.  sofar contains
- * previous results. */
-char *report_on_lines(struct list_head *files,
-		      char *(*report)(const char *),
-		      char *sofar);
+/* Add an error about this file (and line, if non-zero) to the score struct */
+void score_file_error(struct score *, struct ccan_file *f, unsigned line,
+		      const char *error);
 
 /* Normal tests. */
 extern struct ccanlint trailing_whitespace;

tools/ccanlint/compulsory_tests/build.c

@@ -33,42 +33,41 @@ static char *obj_list(const struct manifest *m)
 	return list;
 }
 
-static void *do_build(struct manifest *m,
-		      bool keep,
-		      unsigned int *timeleft)
+static void do_build(struct manifest *m,
+		     bool keep,
+		     unsigned int *timeleft,
+		     struct score *score)
 {
-	char *filename, *err;
+	char *filename, *errstr;
 
 	if (list_empty(&m->c_files)) {
 		/* No files?  No score, but we "pass". */
-		build.total_score = 0;
-		return NULL;
+		score->total = 0;
+		score->pass = true;
+		return;
 	}
-	filename = link_objects(m, m->basename, false, obj_list(m), &err);
-	if (filename && keep) {
+
+	filename = link_objects(m, m->basename, false, obj_list(m), &errstr);
+	if (!filename) {
+		score->error = "The object file didn't build";
+		score_file_error(score, NULL, 0, errstr);
+		return;
+	}
+
+	if (keep) {
 		char *realname = talloc_asprintf(m, "%s.o", m->dir);
 		/* We leave this object file around, all built. */
 		if (!move_file(filename, realname))
-			return talloc_asprintf(m, "Failed to rename %s to %s",
-					       filename, realname);
-		return NULL;
+			err(1, "Renaming %s to %s", filename, realname);
 	}
-	return err;
-}
-
-static const char *describe_build(struct manifest *m, void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "The object file for the module didn't build:\n"
-			       "%s", (char *)check_result);
+	score->pass = true;
+	score->score = score->total;
 }
 
 struct ccanlint build = {
 	.key = "build",
 	.name = "Module can be built from object files",
-	.total_score = 1,
 	.check = do_build,
-	.describe = describe_build,
 	.can_run = can_build,
 };
 

tools/ccanlint/compulsory_tests/build_objs.c

@@ -21,42 +21,37 @@ static const char *can_build(struct manifest *m)
 	return NULL;
 }
 
-static void *check_objs_build(struct manifest *m,
-			      bool keep, unsigned int *timeleft)
+static void check_objs_build(struct manifest *m,
+			     bool keep,
+			     unsigned int *timeleft, struct score *score)
 {
-	char *report = NULL;
 	struct ccan_file *i;
 
+	if (list_empty(&m->c_files))
+		score->total = 0;
+
 	list_for_each(&m->c_files, i, list) {
 		char *err;
 		char *fullfile = talloc_asprintf(m, "%s/%s", m->dir, i->name);
 
-		/* One point for each obj file. */
-		build_objs.total_score++;
-
 		i->compiled = maybe_temp_file(m, "", keep, fullfile);
 		err = compile_object(m, fullfile, ccan_dir, "", i->compiled);
 		if (err) {
 			talloc_free(i->compiled);
-			if (report)
-				report = talloc_append_string(report, err);
-			else
-				report = err;
+			score->error = "Compiling object files";
+			score_file_error(score, i, 0, err);
 		}
 	}
-	return report;
-}
-
-static const char *describe_objs_build(struct manifest *m, void *check_result)
-{
-	return check_result;
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint build_objs = {
 	.key = "build-objects",
 	.name = "Module object files can be built",
 	.check = check_objs_build,
-	.describe = describe_objs_build,
 	.can_run = can_build,
 };
 

tools/ccanlint/compulsory_tests/check_build.c

@@ -45,9 +45,9 @@ static char *lib_list(const struct manifest *m)
 	return ret;
 }
 
-static void *check_use_build(struct manifest *m,
-			     bool keep,
-			     unsigned int *timeleft)
+static void check_use_build(struct manifest *m,
+			    bool keep,
+			    unsigned int *timeleft, struct score *score)
 {
 	char *contents;
 	char *tmpfile;
@@ -58,8 +58,7 @@ static void *check_use_build(struct manifest *m,
 
 	fd = open(tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0600);
 	if (fd < 0)
-		return talloc_asprintf(m, "Creating temporary file: %s",
-				       strerror(errno));
+		err(1, "Creating temporary file %s", tmpfile);
 
 	contents = talloc_asprintf(tmpfile,
 				   "#include <ccan/%s/%s.h>\n"
@@ -68,30 +67,23 @@ static void *check_use_build(struct manifest *m,
 				   "	return 0;\n"
 				   "}\n",
 				   m->basename, m->basename);
-	if (write(fd, contents, strlen(contents)) != strlen(contents)) {
-		close(fd);
-		return "Failure writing to temporary file";
-	}
+	if (write(fd, contents, strlen(contents)) != strlen(contents))
+		err(1, "Failure writing to temporary file %s", tmpfile);
 	close(fd);
 
-	return compile_and_link(m, tmpfile, ccan_dir, obj_list(m), "",
-				lib_list(m),
-				maybe_temp_file(m, "", keep, tmpfile));
-}
-
-static const char *describe_use_build(struct manifest *m, void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "Linking against module:\n"
-			       "%s", (char *)check_result);
+	score->error = compile_and_link(m, tmpfile, ccan_dir, obj_list(m), "",
+					lib_list(m),
+					maybe_temp_file(m, "", keep, tmpfile));
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint check_build = {
 	.key = "check-link",
 	.name = "Module can be linked against trivial program",
-	.total_score = 1,
 	.check = check_use_build,
-	.describe = describe_use_build,
 	.can_run = can_build,
 };
 

tools/ccanlint/compulsory_tests/check_depends_built.c

@@ -35,13 +35,12 @@ static bool expect_obj_file(const char *dir)
 	return has_c_files;
 }
 
-static void *check_depends_built(struct manifest *m,
-				 bool keep,
-				 unsigned int *timeleft)
+static void check_depends_built(struct manifest *m,
+				bool keep,
+				unsigned int *timeleft, struct score *score)
 {
 	struct ccan_file *i;
 	struct stat st;
-	char *report = NULL;
 
 	list_for_each(&m->dep_dirs, i, list) {
 		if (!expect_obj_file(i->fullname))
@@ -49,9 +48,11 @@ static void *check_depends_built(struct manifest *m,
 
 		i->compiled = talloc_asprintf(i, "%s.o", i->fullname);
 		if (stat(i->compiled, &st) != 0) {
-			report = talloc_asprintf_append(report,
-							"object file %s\n",
-							i->compiled);
+			score->error = "Dependencies are not built";
+			score_file_error(score, i, 0,
+					 talloc_asprintf(score,
+							"object file %s",
+							 i->compiled));
 			i->compiled = NULL;
 		}			
 	}
@@ -61,30 +62,21 @@ static void *check_depends_built(struct manifest *m,
 	    && (!list_empty(&m->run_tests) || !list_empty(&m->api_tests))) {
 		char *tapobj = talloc_asprintf(m, "%s/ccan/tap.o", ccan_dir);
 		if (stat(tapobj, &st) != 0) {
-			report = talloc_asprintf_append(report,
-							"object file %s"
-							" (for tests)\n",
-							tapobj);
+			score->error = talloc_asprintf(score,
+					       "tap object file not built");
 		}
 	}
 
-	return talloc_steal(m, report);
-}
-
-static const char *describe_depends_built(struct manifest *m,
-					  void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "The following dependencies are not built:\n"
-			       "%s", (char *)check_result);
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint depends_built = {
 	.key = "depends-built",
 	.name = "Module's CCAN dependencies are already built",
-	.total_score = 1,
 	.check = check_depends_built,
-	.describe = describe_depends_built,
 	.can_run = can_build,
 };
 

tools/ccanlint/compulsory_tests/check_depends_exist.c

@@ -14,29 +14,24 @@
 #include <string.h>
 #include <ctype.h>
 
-static char *add_dep(char *sofar, struct manifest *m, const char *dep)
+static void add_dep(struct manifest *m, const char *dep, struct score *score)
 {
 	struct stat st;
 	struct ccan_file *f;
 
 	f = new_ccan_file(m, ccan_dir, talloc_strdup(m, dep));
 	if (stat(f->fullname, &st) != 0) {
-		return talloc_asprintf_append(sofar,
-					      "ccan/%s: expected it in"
-					      " directory %s\n",
-					      dep, f->fullname);
-	}
-
-	list_add_tail(&m->dep_dirs, &f->list);
-	return sofar;
+		score->error = "Depends don't exist";
+		score_file_error(score, f, 0, "could not stat");
+	} else
+		list_add_tail(&m->dep_dirs, &f->list);
 }
 
-static void *check_depends_exist(struct manifest *m,
-				 bool keep,
-				 unsigned int *timeleft)
+static void check_depends_exist(struct manifest *m,
+				bool keep,
+				unsigned int *timeleft, struct score *score)
 {
 	unsigned int i;
-	char *report = NULL;
 	char **deps;
 	char *updir = talloc_strdup(m, m->dir);
 
@@ -52,25 +47,18 @@ static void *check_depends_exist(struct manifest *m,
 		if (!strstarts(deps[i], "ccan/"))
 			continue;
 
-		report = add_dep(report, m, deps[i]);
+		add_dep(m, deps[i], score);
+	}
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
 	}
-	return report;
-}
-
-static const char *describe_depends_exist(struct manifest *m,
-					  void *check_result)
-{
-	return talloc_asprintf(check_result,
-			       "The following dependencies are are expected:\n"
-			       "%s", (char *)check_result);
 }
 
 struct ccanlint depends_exist = {
 	.key = "depends-exist",
 	.name = "Module's CCAN dependencies are present",
-	.total_score = 1,
 	.check = check_depends_exist,
-	.describe = describe_depends_exist,
 };
 
 REGISTER_TEST(depends_exist, NULL);

tools/ccanlint/compulsory_tests/check_includes_build.c

@@ -32,12 +32,12 @@ static struct ccan_file *main_header(struct manifest *m)
 			return f;
 	}
 	/* Should not happen: we depend on has_main_header */
-	return NULL;
+	abort();
 }
 
-static void *check_includes_build(struct manifest *m,
-				  bool keep,
-				  unsigned int *timeleft)
+static void check_includes_build(struct manifest *m,
+				 bool keep,
+				 unsigned int *timeleft, struct score *score)
 {
 	char *contents;
 	char *tmpsrc, *tmpobj;
@@ -49,34 +49,29 @@ static void *check_includes_build(struct manifest *m,
 
 	fd = open(tmpsrc, O_WRONLY | O_CREAT | O_EXCL, 0600);
 	if (fd < 0)
-		return talloc_asprintf(m, "Creating temporary file %s: %s",
-				       tmpsrc, strerror(errno));
+		err(1, "Creating temporary file %s", tmpsrc);
 
 	contents = talloc_asprintf(tmpsrc, "#include <ccan/%s/%s.h>\n",
 				   m->basename, m->basename);
-	if (write(fd, contents, strlen(contents)) != strlen(contents)) {
-		close(fd);
-		return "Failure writing to temporary file";
-	}
+	if (write(fd, contents, strlen(contents)) != strlen(contents))
+		err(1, "writing to temporary file %s", tmpsrc);
 	close(fd);
 
-	return compile_object(m, tmpsrc, ccan_dir, "", tmpobj);
-}
-
-static const char *describe_includes_build(struct manifest *m,
-					   void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "#include of the main header file:\n"
-			       "%s", (char *)check_result);
+	score->error = compile_object(m, tmpsrc, ccan_dir, "", tmpobj);
+	if (score->error) {
+		score->error = talloc_asprintf(score,
+				       "#include of the main header file:\n%s",
+				       score->error);
+	} else {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint includes_build = {
 	.key = "include-main",
 	.name = "Modules main header compiles",
-	.total_score = 1,
 	.check = check_includes_build,
-	.describe = describe_includes_build,
 	.can_run = can_build,
 };
 

tools/ccanlint/compulsory_tests/has_info.c

@@ -12,21 +12,20 @@
 #include <ccan/noerr/noerr.h>
 #include <ccan/talloc/talloc.h>
 
-static void *check_has_info(struct manifest *m,
-			    bool keep,
-			    unsigned int *timeleft)
+static void check_has_info(struct manifest *m,
+			   bool keep,
+			   unsigned int *timeleft,
+			   struct score *score)
 {
-	if (m->info_file)
-		return NULL;
-	return m;
-}
-
-static const char *describe_has_info(struct manifest *m, void *check_result)
-{
-	return "You have no _info file.\n\n"
+	if (m->info_file) {
+		score->pass = true;
+		score->score = score->total;
+	} else {
+		score->error = "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";
+	}
 }
 
 static const char template[] = 
@@ -55,7 +54,7 @@ static const char template[] =
 	"	return 1;\n"
 	"}\n";
 
-static void create_info_template(struct manifest *m, void *check_result)
+static void create_info_template(struct manifest *m, struct score *score)
 {
 	FILE *info;
 	const char *filename;
@@ -79,7 +78,6 @@ struct ccanlint has_info = {
 	.key = "info",
 	.name = "Module has _info file",
 	.check = check_has_info,
-	.describe = describe_has_info,
 	.handle = create_info_template,
 };
 

tools/ccanlint/compulsory_tests/has_main_header.c

@@ -12,35 +12,31 @@
 #include <ccan/talloc/talloc.h>
 #include <ccan/noerr/noerr.h>
 
-static void *check_has_main_header(struct manifest *m,
-				   bool keep,
-				   unsigned int *timeleft)
+static void check_has_main_header(struct manifest *m,
+				  bool keep,
+				  unsigned int *timeleft, struct score *score)
 {
 	struct ccan_file *f;
 
 	list_for_each(&m->h_files, f, list) {
 		if (strstarts(f->name, m->basename)
-		    && strlen(f->name) == strlen(m->basename) + 2)
-			return NULL;
+		    && strlen(f->name) == strlen(m->basename) + 2) {
+			score->pass = true;
+			score->score = score->total;
+			return;
+		} 
 	}
-	return m;
-}
-
-static const char *describe_has_main_header(struct manifest *m,
-					    void *check_result)
-{
-	return talloc_asprintf(m,
+	score->error = talloc_asprintf(score,
 	"You have no %s/%s.h header file.\n\n"
 	"CCAN modules have a name, the same as the directory name.  They're\n"
 	"expected to have an interface in the header of the same name.\n",
-			       m->basename, m->basename);
+				       m->basename, m->basename);
 }
 
 struct ccanlint has_main_header = {
 	.key = "has-main-header",
 	.name = "Module has main header file",
 	.check = check_has_main_header,
-	.describe = describe_has_main_header,
 };
 
 REGISTER_TEST(has_main_header, NULL);

tools/ccanlint/file_analysis.c

@@ -139,30 +139,6 @@ static void add_files(struct manifest *m, const char *dir)
 	closedir(d);
 }
 
-char *report_on_lines(struct list_head *files,
-		      char *(*report)(const char *),
-		      char *sofar)
-{
-	struct ccan_file *f;
-
-	list_for_each(files, f, list) {
-		unsigned int i;
-		char **lines = get_ccan_file_lines(f);
-
-		for (i = 0; i < f->num_lines; i++) {
-			char *r = report(lines[i]);
-			if (!r)
-				continue;
-
-			sofar = talloc_asprintf_append(sofar,
-						       "%s:%u:%s\n",
-						       f->name, i+1, r);
-			talloc_free(r);
-		}
-	}
-	return sofar;
-}
-
 struct manifest *get_manifest(const void *ctx, const char *dir)
 {
 	struct manifest *m = talloc(ctx, struct manifest);
@@ -574,3 +550,12 @@ enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
 	return ret;
 }
 
+void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
+		      const char *error)
+{
+	struct file_error *fe = talloc(score, struct file_error);
+	fe->file = f;
+	fe->line = line;
+	fe->error = error;
+	list_add(&score->per_file_errors, &fe->list);
+}

tools/ccanlint/tests/build-coverage.c

@@ -25,8 +25,9 @@ static const char *can_run_coverage(struct manifest *m)
 	return NULL;
 }
 
-static char *build_module_objs_with_coverage(struct manifest *m, bool keep,
-					     char **modobjs)
+static bool build_module_objs_with_coverage(struct manifest *m, bool keep,
+					    struct score *score,
+					    char **modobjs)
 {
 	struct ccan_file *i;
 
@@ -39,13 +40,15 @@ static char *build_module_objs_with_coverage(struct manifest *m, bool keep,
 		err = compile_object(m, fullfile, ccan_dir, "",
 				     i->cov_compiled);
 		if (err) {
+			score_file_error(score, i, 0, err);
 			talloc_free(i->cov_compiled);
-			return err;
+			i->cov_compiled = NULL;
+			return false;
 		}
 		*modobjs = talloc_asprintf_append(*modobjs,
 						  " %s", i->cov_compiled);
 	}
-	return NULL;
+	return true;
 }
 
 static char *obj_list(const struct manifest *m, const char *modobjs)
@@ -101,97 +104,53 @@ static char *cov_compile(const void *ctx,
 				  lib_list(m), file->cov_compiled);
 	if (errmsg) {
 		talloc_free(file->cov_compiled);
+		file->cov_compiled = NULL;
 		return errmsg;
 	}
 
 	return NULL;
 }
 
-struct compile_tests_result {
-	struct list_node list;
-	const char *filename;
-	const char *description;
-	const char *output;
-};
-
-static void *do_compile_coverage_tests(struct manifest *m,
-				       bool keep,
-				       unsigned int *timeleft)
+/* FIXME: Coverage from testable examples as well. */
+static void do_compile_coverage_tests(struct manifest *m,
+				      bool keep,
+				      unsigned int *timeleft,
+				      struct score *score)
 {
-	struct list_head *list = talloc(m, struct list_head);
 	char *cmdout, *modobjs = NULL;
 	struct ccan_file *i;
-	struct compile_tests_result *res;
 
-	list_head_init(list);
-
-	if (!list_empty(&m->api_tests)) {
-		cmdout = build_module_objs_with_coverage(m, keep, &modobjs);
-		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = "Module objects with coverage";
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
-			return list;
-		}
+	if (!list_empty(&m->api_tests)
+	    && !build_module_objs_with_coverage(m, keep, score, &modobjs)) {
+		score->error = "Failed to compile module objects with coverage";
+		return;
 	}
 
 	list_for_each(&m->run_tests, i, list) {
-		compile_tests.total_score++;
 		cmdout = cov_compile(m, m, i, NULL, keep);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			score->error = "Failed to compile test with coverage";
+			score_file_error(score, i, 0, cmdout);
 		}
 	}
 
 	list_for_each(&m->api_tests, i, list) {
-		compile_tests.total_score++;
 		cmdout = cov_compile(m, m, i, modobjs, keep);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			score->error = "Failed to compile test with coverage";
+			score_file_error(score, i, 0, cmdout);
 		}
 	}
-
-	if (list_empty(list)) {
-		talloc_free(list);
-		list = NULL;
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
 	}
-
-	return list;
-}
-
-static const char *describe_compile_coverage_tests(struct manifest *m,
-						   void *check_result)
-{
-	struct list_head *list = check_result;
-	struct compile_tests_result *i;
-	char *descrip;
-
-	descrip = talloc_strdup(list,
-				"Compilation of tests for coverage failed:\n");
-
-	list_for_each(list, i, list)
-		descrip = talloc_asprintf_append(descrip, "%s %s\n%s",
-						 i->filename, i->description,
-						 i->output);
-	return descrip;
 }
 
 struct ccanlint compile_coverage_tests = {
 	.key = "compile-coverage-tests",
 	.name = "Module tests compile with profiling",
 	.check = do_compile_coverage_tests,
-	.total_score = 1,
-	.describe = describe_compile_coverage_tests,
 	.can_run = can_run_coverage,
 };
 

tools/ccanlint/tests/compile_test_helpers.c

@@ -14,7 +14,7 @@
 #include <string.h>
 #include <ctype.h>
 
-static const char *can_build(struct manifest *m)
+static const char *can_run(struct manifest *m)
 {
 	if (safe_mode)
 		return "Safe mode enabled";
@@ -30,39 +30,35 @@ static char *compile(struct manifest *m,
 			      cfile->compiled);
 }
 
-static void *do_compile_test_helpers(struct manifest *m,
-				     bool keep,
-				     unsigned int *timeleft)
+static void do_compile_test_helpers(struct manifest *m,
+				    bool keep,
+				    unsigned int *timeleft,
+				    struct score *score)
 {
-	char *cmdout = NULL;
 	struct ccan_file *i;
 
-	compile_tests.total_score = 0;
+	if (list_empty(&m->other_test_c_files))
+		score->total = 0;
+
 	list_for_each(&m->other_test_c_files, i, list) {
-		compile_tests.total_score++;
-		cmdout = compile(m, keep, i);
-		if (cmdout)
-			return talloc_asprintf(m,
-					       "Failed to compile helper C"
-					       " code file %s:\n%s",
-					       i->name, cmdout);
+		char *cmdout = compile(m, keep, i);
+		if (cmdout) {
+			score->error = "Failed to compile helper C files";
+			score_file_error(score, i, 0, cmdout);
+		}
 	}
-	return NULL;
-}
 
-static const char *describe_compile_test_helpers(struct manifest *m,
-						 void *check_result)
-{
-	return check_result;
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint compile_test_helpers = {
 	.key = "compile-helpers",
 	.name = "Module test helper objects compile",
-	.total_score = 1,
 	.check = do_compile_test_helpers,
-	.describe = describe_compile_test_helpers,
-	.can_run = can_build,
+	.can_run = can_run,
 };
 
 REGISTER_TEST(compile_test_helpers, &depends_built, &has_tests, NULL);

tools/ccanlint/tests/compile_tests.c

@@ -82,124 +82,64 @@ static char *compile(const void *ctx,
 	return NULL;
 }
 
-struct compile_tests_result {
-	struct list_node list;
-	const char *filename;
-	const char *description;
-	const char *output;
-};
-
-static void *do_compile_tests(struct manifest *m,
-			      bool keep,
-			      unsigned int *timeleft)
+static void do_compile_tests(struct manifest *m,
+			     bool keep,
+			     unsigned int *timeleft, struct score *score)
 {
-	struct list_head *list = talloc(m, struct list_head);
 	char *cmdout;
 	struct ccan_file *i;
-	struct compile_tests_result *res;
 
-	list_head_init(list);
-
-	compile_tests.total_score = 0;
 	list_for_each(&m->compile_ok_tests, i, list) {
-		compile_tests.total_score++;
-		cmdout = compile(list, m, i, false, false, keep);
+		cmdout = compile(score, m, i, false, false, keep);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			score->error = "Failed to compile tests";
+			score_file_error(score, i, 0, cmdout);
 		}
 	}
 
 	list_for_each(&m->run_tests, i, list) {
-		compile_tests.total_score++;
-		cmdout = compile(m, m, i, false, false, keep);
+		cmdout = compile(score, m, i, false, false, keep);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			score->error = "Failed to compile tests";
+			score_file_error(score, i, 0, cmdout);
 		}
 	}
 
 	list_for_each(&m->api_tests, i, list) {
-		compile_tests.total_score++;
-		cmdout = compile(m, m, i, false, true, keep);
+		cmdout = compile(score, m, i, false, true, keep);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			score->error = "Failed to compile tests";
+			score_file_error(score, i, 0, cmdout);
 		}
 	}
 
+	/* The compile fail tests are a bit weird, handle them separately */
+	if (score->error)
+		return;
+
 	list_for_each(&m->compile_fail_tests, i, list) {
-		compile_tests.total_score++;
-		cmdout = compile(list, m, i, false, false, false);
+		cmdout = compile(score, m, i, false, false, false);
 		if (cmdout) {
-			res = talloc(list, struct compile_tests_result);
-			res->filename = i->name;
-			res->description = "failed to compile without -DFAIL";
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
-		} else {
-			cmdout = compile(list, m, i, true, false, false);
-			if (!cmdout) {
-				res = talloc(list, struct compile_tests_result);
-				res->filename = i->name;
-				res->description = "compiled successfully"
-					" with -DFAIL";
-				res->output = "";
-				list_add_tail(list, &res->list);
-			}
+			score->error = "Failed to compile without -DFAIL";
+			score_file_error(score, i, 0, cmdout);
+			return;
+		}
+		cmdout = compile(score, m, i, true, false, false);
+		if (!cmdout) {
+			score->error = "Compiled successfully with -DFAIL?";
+			score_file_error(score, i, 0, NULL);
+			return;
 		}
 	}
 
-	if (list_empty(list)) {
-		talloc_free(list);
-		list = NULL;
-	}
-
-	return list;
-}
-
-static unsigned int score_compile_tests(struct manifest *m,
-					void *check_result)
-{
-	struct list_head *list = check_result;
-	struct compile_tests_result *i;
-	unsigned int score = compile_tests.total_score;
-
-	list_for_each(list, i, list)
-		score--;
-	return score;
-}
-
-static const char *describe_compile_tests(struct manifest *m,
-					  void *check_result)
-{
-	struct list_head *list = check_result;
-	struct compile_tests_result *i;
-	char *descrip = talloc_strdup(list, "Compilation tests failed:\n");
-
-	list_for_each(list, i, list)
-		descrip = talloc_asprintf_append(descrip, "%s %s\n%s",
-						 i->filename, i->description,
-						 i->output);
-	return descrip;
+	score->pass = true;
+	score->score = score->total;
 }
 
 struct ccanlint compile_tests = {
 	.key = "compile-tests",
 	.name = "Module tests compile",
-	.score = score_compile_tests,
-	.total_score = 1,
 	.check = do_compile_tests,
-	.describe = describe_compile_tests,
 	.can_run = can_build,
 };
 

tools/ccanlint/tests/depends_accurate.c

@@ -46,12 +46,11 @@ static bool has_dep(struct manifest *m, const char *depname, bool tap_ok)
 	return false;
 }
 
-static void *check_depends_accurate(struct manifest *m,
-				    bool keep,
-				    unsigned int *timeleft)
+static void check_depends_accurate(struct manifest *m,
+				   bool keep,
+				   unsigned int *timeleft, struct score *score)
 {
 	struct list_head *list;
-	char *report = talloc_strdup(m, "");
 
 	foreach_ptr(list, &m->c_files, &m->h_files,
 		    &m->run_tests, &m->api_tests,
@@ -61,7 +60,7 @@ static void *check_depends_accurate(struct manifest *m,
 		bool tap_ok;
 
 		/* Including ccan/tap is fine for tests. */
-		tap_ok =  (list != &m->c_files && list != &m->h_files);
+		tap_ok = (list != &m->c_files && list != &m->h_files);
 
 		list_for_each(list, f, list) {
 			unsigned int i;
@@ -79,35 +78,25 @@ static void *check_depends_accurate(struct manifest *m,
 				if (!strchr(strchr(p, '/') + 1, '/'))
 					continue;
 				*strchr(strchr(p, '/') + 1, '/') = '\0';
-				if (!has_dep(m, p, tap_ok))
-					report = talloc_asprintf_append(report,
-					       "%s:%u:%s\n",
-					       f->name, i+1, lines[i]);
+				if (has_dep(m, p, tap_ok))
+					continue;
+				score->error = "Includes a ccan module"
+					" not listed in _info";
+				score_file_error(score, f, i+1, lines[i]);
 			}
 		}
 	}
 
-	if (streq(report, "")) {
-		talloc_free(report);
-		report = NULL;
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
 	}
-	return report;
-}
-
-static const char *describe_depends_accurage(struct manifest *m,
-					     void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "You include ccan modules you don't list as dependencies:\n"
-			       "%s", (char *)check_result);
 }
 
 struct ccanlint depends_accurate = {
 	.key = "depends-accurate",
 	.name = "Module's CCAN dependencies are the only ccan files #included",
-	.total_score = 1,
 	.check = check_depends_accurate,
-	.describe = describe_depends_accurage,
 };
 
 REGISTER_TEST(depends_accurate, &depends_exist, NULL);

tools/ccanlint/tests/examples_compile.c

@@ -15,6 +15,8 @@ static const char *can_run(struct manifest *m)
 {
 	if (safe_mode)
 		return "Safe mode enabled";
+	if (list_empty(&m->examples))
+		return "No examples to compile";
 	return NULL;
 }
 
@@ -128,11 +130,6 @@ static char *compile(const void *ctx,
 	return NULL;
 }
 
-struct score {
-	unsigned int score;
-	char *errors;
-};
-
 static char *start_main(char *ret, const char *why)
 {
 	return talloc_asprintf_append(ret,
@@ -434,22 +431,21 @@ static struct ccan_file *mangle_example(struct manifest *m,
 	return f;
 }
 
-static void *build_examples(struct manifest *m, bool keep,
-			    unsigned int *timeleft)
+static void build_examples(struct manifest *m, bool keep,
+			   unsigned int *timeleft, struct score *score)
 {
 	struct ccan_file *i;
-	struct score *score = talloc(m, struct score);
 	char **prev = NULL;
 
-	score->score = 0;
-	score->errors = talloc_strdup(score, "");
+	score->total = 0;
+	score->pass = true;
 
-	examples_compile.total_score = 0;
 	list_for_each(&m->examples, i, list) {
 		char *ret, *ret1, *ret2 = NULL;
 		struct ccan_file *mangle1, *mangle2 = NULL;
+		char *err;
 
-		examples_compile.total_score++;
+		score->total++;
 		/* Simplify our dumb parsing. */
 		strip_leading_whitespace(get_ccan_file_lines(i));
 		ret = compile(i, m, i, keep);
@@ -481,64 +477,46 @@ static void *build_examples(struct manifest *m, bool keep,
 			}
 		}
 
-		score->errors = talloc_asprintf_append(score->errors,
-				       "%s: tried standalone example:\n"
-				       "%s\n"
-				       "Errors: %s\n\n",
-				       i->name,
-				       get_ccan_file_contents(i),
-				       ret);
-		score->errors = talloc_asprintf_append(score->errors,
-				       "%s: tried adding headers, wrappers:\n"
-				       "%s\n"
-				       "Errors: %s\n\n",
-				       i->name,
-				       get_ccan_file_contents(mangle1),
-				       ret1);
-
-		if (mangle2) {
-			score->errors = talloc_asprintf_append(score->errors,
-				       "%s: tried combining with"
-				       " previous example:\n"
+		score->pass = false;
+		score->error = "Compiling extracted examples failed";
+		if (!verbose) {
+			if (mangle2) 
+				err = "Standalone, adding headers, "
+					"and including previous "
+					"example all failed";
+			else
+				err = "Standalone compile and"
+					" adding headers both failed";
+		} else {
+			err = talloc_asprintf("Standalone example:\n"
+					      "%s\n"
+					      "Errors: %s\n\n"
+					      "Adding headers, wrappers:\n"
+					      "%s\n"
+					      "Errors: %s\n\n",
+					      get_ccan_file_contents(i),
+					      ret,
+					      get_ccan_file_contents(mangle1),
+					      ret1);
+
+			if (mangle2)
+				err = talloc_asprintf_append(err, 
+				       "Combining with previous example:\n"
 				       "%s\n"
 				       "Errors: %s\n\n",
-				       i->name,
 				       get_ccan_file_contents(mangle2),
 				       ret2);
 		}
+		score_file_error(score, i, 0, err);
 		/* This didn't work, so not a candidate for combining. */
 		prev = NULL;
 	}
-
-	if (strcmp(score->errors, "") == 0) {
-		talloc_free(score);
-		return NULL;
-	}
-	return score;
-}
-
-static unsigned int score_examples(struct manifest *m, void *check_result)
-{
-	struct score *score = check_result;
-	return score->score;
-}
-
-static const char *describe(struct manifest *m, void *check_result)
-{
-	struct score *score = check_result;
-	if (verbose >= 2 && score->errors)
-		return talloc_asprintf(m, "Compile errors building examples:\n"
-				       "%s", score->errors);
-	return NULL;
 }
 
 struct ccanlint examples_compile = {
 	.key = "examples-compile",
 	.name = "Module examples compile",
-	.score = score_examples,
-	.total_score = 3, /* This gets changed to # examples, if they exist */
 	.check = build_examples,
-	.describe = describe,
 	.can_run = can_run,
 };
 

tools/ccanlint/tests/examples_run.c

@@ -14,16 +14,16 @@
 
 static const char *can_run(struct manifest *m)
 {
+	struct list_head *list;
+
 	if (safe_mode)
 		return "Safe mode enabled";
-	return NULL;
+	foreach_ptr(list, &m->examples, &m->mangled_examples)
+		if (!list_empty(list))
+			return NULL;
+	return "No examples";
 }
 
-struct score {
-	unsigned int score;
-	char *errors;
-};
-
 /* Very dumb scanner, allocates %s-strings. */
 static bool scan_forv(const void *ctx,
 		      const char *input, const char *fmt, const va_list *args)
@@ -224,26 +224,21 @@ static char *unexpected(struct ccan_file *i, const char *input,
 	return output;
 }
 
-static void *run_examples(struct manifest *m, bool keep,
-			  unsigned int *timeleft)
+static void run_examples(struct manifest *m, bool keep,
+			 unsigned int *timeleft, struct score *score)
 {
 	struct ccan_file *i;
 	struct list_head *list;
-	struct score *score = talloc(m, struct score);
 
-	score->score = 0;
-	score->errors = talloc_strdup(score, "");
+	score->total = 0;
+	score->pass = true;
 
-	examples_run.total_score = 0;
 	foreach_ptr(list, &m->examples, &m->mangled_examples) {
 		list_for_each(list, i, list) {
 			char **lines, *expect, *input, *output;
 			unsigned int linenum = 0;
 			bool exact;
 
-			if (i->compiled == NULL)
-				continue;
-
 			lines = get_ccan_file_lines(i);
 
 			for (expect = find_expect(i, lines, &input, &exact,
@@ -252,52 +247,34 @@ static void *run_examples(struct manifest *m, bool keep,
 			     linenum++,
 				     expect = find_expect(i, lines, &input,
 							  &exact, &linenum)) {
-				examples_run.total_score++;
+				char *err;
+				score->total++;
+				if (i->compiled == NULL)
+					continue;
+
 				output = unexpected(i, input, expect, exact);
-				if (!output)
+				if (!output) {
 					score->score++;
-				else {
-					score->errors = talloc_asprintf_append(
-						score->errors,
-						"%s: output '%s' didn't"
-						" %s '%s'\n",
-						i->name, output,
-						exact ? "match" : "contain",
-						expect);
+					continue;
 				}
+				err = talloc_asprintf(score,
+						      "output '%s' didn't"
+						      " %s '%s'\n",
+						      output,
+						      exact
+						      ? "match" : "contain",
+						      expect);
+				score_file_error(score, i, linenum+1, err);
+				score->pass = false;
 			}
 		}
 	}
-
-	if (strcmp(score->errors, "") == 0) {
-		talloc_free(score);
-		return NULL;
-	}
-	return score;
-}
-
-static unsigned int score_examples(struct manifest *m, void *check_result)
-{
-	struct score *score = check_result;
-	return score->score;
-}
-
-static const char *describe(struct manifest *m, void *check_result)
-{
-	struct score *score = check_result;
-	if (verbose)
-		return talloc_asprintf(m, "Wrong output running examples:\n"
-				       "%s", score->errors);
-	return NULL;
 }
 
 struct ccanlint examples_run = {
 	.key = "examples-run",
 	.name = "Module examples with expected output give that output",
-	.score = score_examples,
-	.total_score = 3, /* This gets changed to # testable, if we run. */
 	.check = run_examples,
-	.describe = describe,
 	.can_run = can_run,
 };
 

tools/ccanlint/tests/has_examples.c

@@ -56,28 +56,22 @@ static char *add_example(struct manifest *m, struct ccan_file *source,
 }
 
 /* FIXME: We should have one example per function in header. */
-struct score {
-	bool info_example, header_example;
-	char *error;
-};
-
-static void *extract_examples(struct manifest *m,
-			      bool keep,
-			      unsigned int *timeleft)
+static void extract_examples(struct manifest *m,
+			     bool keep,
+			     unsigned int *timeleft,
+			     struct score *score)
 {
 	struct ccan_file *f;
 	struct doc_section *d;
-	struct score *score = talloc(m, struct score);
-
-	score->info_example = score->header_example = false;
-	score->error = NULL;
+	bool have_info_example = false, have_header_example = false;
 
+	score->total = 2;
 	list_for_each(get_ccan_file_docs(m->info_file), d, list) {
 		if (streq(d->type, "example")) {
 			score->error = add_example(m, m->info_file, keep, d);
 			if (score->error)
-				return score;
-			score->info_example = true;
+				return;
+			have_info_example = true;
 		}
 	}
 
@@ -91,57 +85,35 @@ static void *extract_examples(struct manifest *m,
 			if (streq(d->type, "example")) {
 				score->error = add_example(m, f, keep, d);
 				if (score->error)
-					return score;
-				score->header_example = true;
+					return;
+				have_header_example = true;
 			}
 		}
 	}
-	return score;
-}
-
-static unsigned int score_examples(struct manifest *m, void *check_result)
-{
-	struct score *score = check_result;
-	int total = 0;
-
-	if (score->error)
-		return 0;
-	total += score->info_example;
-	total += score->header_example;
-	return total;
-}
-
-static const char *describe_examples(struct manifest *m,
-				     void *check_result)
-{
-	struct score *score = check_result;
-	char *descrip = NULL;
-
-	if (score->error)
-		return score->error;
 
-	if (!score->info_example)
-		descrip = talloc_asprintf(score,
-		"Your _info file has no module example.\n\n"
-		"There should be an Example: section of the _info documentation\n"
-		"which provides a concise toy program which uses your module\n");
-
-	if (!score->header_example)
-		descrip = talloc_asprintf(score,
-		 "%sMain header file file has no examples\n\n"
-		 "There should be an Example: section for each public function\n"
-		  "demonstrating its use\n", descrip ? descrip : "");
-
-	return descrip;
+	if (!have_info_example && !have_header_example) {
+		score->error = "You don't have any Example: sections";
+		score->score = 0;
+	} else if (!have_info_example) {
+		score->error = "You don't have an Example: section in _info";
+		score->score = 1;
+		score->pass = true;
+	} else if (!have_header_example) {
+		score->error = talloc_asprintf(score,
+			       "You don't have an Example: section in %s.h",
+			       m->basename);
+		score->score = 1;
+		score->pass = true;
+	} else {
+		score->score = score->total;
+		score->pass = true;
+	}
 }
 
 struct ccanlint has_examples = {
 	.key = "has-examples",
 	.name = "_info and header files have examples",
-	.score = score_examples,
 	.check = extract_examples,
-	.describe = describe_examples,
-	.total_score = 2,
 };
 
 REGISTER_TEST(has_examples, &has_info, NULL);

tools/ccanlint/tests/has_info_documentation.c

@@ -15,38 +15,7 @@
 #include <ccan/noerr/noerr.h>
 #include <ccan/grab_file/grab_file.h>
 
-struct info_docs
-{
-	bool summary;
-	bool description;
-};
-
-static void *check_has_info_documentation(struct manifest *m,
-					  bool keep,
-					  unsigned int *timeleft)
-{
-	struct list_head *infodocs = get_ccan_file_docs(m->info_file);
-	struct doc_section *d;
-	struct info_docs id = { false, false };
-
-	list_for_each(infodocs, d, list) {
-		if (!streq(d->function, m->basename))
-			continue;
-		if (streq(d->type, "summary"))
-			id.summary = true;
-		if (streq(d->type, "description"))
-			id.description = true;
-	}
-
-	if (id.summary && id.description)
-		return NULL;
-	return talloc_memdup(m, &id, sizeof(id));
-}
-
-/* This is defined below. */
-extern struct ccanlint has_info_documentation;
-
-static void create_info_template_doc(struct manifest *m, void *check_result)
+static void create_info_template_doc(struct manifest *m, struct score *score)
 {
 	int fd = open("_info.new", O_WRONLY|O_CREAT|O_EXCL, 0666);
 	FILE *new;
@@ -87,42 +56,44 @@ static void create_info_template_doc(struct manifest *m, void *check_result)
 	}
 }
 
-static const char *describe_has_info_documentation(struct manifest *m,
-						   void *check_result)
+static void check_has_info_documentation(struct manifest *m,
+					 bool keep,
+					 unsigned int *timeleft,
+					 struct score *score)
 {
-	struct info_docs *id = check_result;
-	char *reason = talloc_strdup(m, "");
+	struct list_head *infodocs = get_ccan_file_docs(m->info_file);
+	struct doc_section *d;
+	bool summary = false, description = false;
 
-	if (!id->summary) {
-		has_info_documentation.handle = create_info_template_doc;
-		reason = talloc_asprintf_append(reason,
-		"Your _info file has no module documentation.\n\n"
+	list_for_each(infodocs, d, list) {
+		if (!streq(d->function, m->basename))
+			continue;
+		if (streq(d->type, "summary"))
+			summary = true;
+		if (streq(d->type, "description"))
+			description = true;
+	}
+
+	if (summary && description) {
+		score->score = score->total;
+		score->pass = true;
+	} else if (!summary) {
+		score->error = "_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";
+		has_info_documentation.handle = create_info_template_doc;
 	}
-	if (!id->description)
-		reason = talloc_asprintf_append(reason,
-		"Your _info file has no module description.\n\n"
+	else if (!description) 
+		score->error = "_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");
-	return reason;
-}
-
-static unsigned int has_info_documentation_score(struct manifest *m,
-						 void *check_result)
-{
-	struct info_docs *id = check_result;
-	return (unsigned int)id->summary + id->description;
+		"overall package\n";
 }
 
 struct ccanlint has_info_documentation = {
 	.key = "info-documentation",
 	.name = "Module has documentation in _info",
-	.total_score = 2,
-	.score = has_info_documentation_score,
 	.check = check_has_info_documentation,
-	.describe = describe_has_info_documentation,
 };
 
 REGISTER_TEST(has_info_documentation, NULL);

tools/ccanlint/tests/has_tests.c

@@ -10,45 +10,20 @@
 #include <err.h>
 #include <ccan/talloc/talloc.h>
 
-static char test_is_not_dir[] = "test is not a directory";
-
-static void *check_has_tests(struct manifest *m,
-			     bool keep,
-			     unsigned int *timeleft)
+static void handle_no_tests(struct manifest *m, struct score *score)
 {
-	struct stat st;
+	FILE *run;
+	struct ccan_file *i;
 	char *test_dir = talloc_asprintf(m, "%s/test", m->dir);
 
-	if (lstat(test_dir, &st) != 0) {
-		if (errno != ENOENT)
-			err(1, "statting %s", test_dir);
-		return "You have no test directory";
-	}
-
-	if (!S_ISDIR(st.st_mode))
-		return test_is_not_dir;
-
-	if (list_empty(&m->api_tests)
-	    && list_empty(&m->run_tests)
-	    && list_empty(&m->compile_ok_tests)) {
-		if (list_empty(&m->compile_fail_tests))
-			return "You have no tests in the test directory";
-		else
-			return "You have no positive tests in the test directory";
-	}
-	return NULL;
-}
-
-static const char *describe_has_tests(struct manifest *m, void *check_result)
-{
-	return talloc_asprintf(m, "%s\n\n"
-        "CCAN modules have a directory called test/ which contains tests.\n"
+	printf(
+	"CCAN modules have a directory called test/ which contains tests.\n"
 	"There are four kinds of tests: api, run, compile_ok and compile_fail:\n"
 	"you can tell which type of test a C file is by its name, eg 'run.c'\n"
 	"and 'run-simple.c' are both run tests.\n\n"
 
 	"The simplest kind of test is a run test, which must compile with no\n"
-	"warnings, and then run: it is expected to use libtap to report its\n"
+	"warnings, and then run: it is expected to use ccan/tap to report its\n"
 	"results in a simple and portable format.  It should #include the C\n"
 	"files from the module directly (so it can probe the internals): the\n"
 	"module will not be linked in.  The test will be run in a temporary\n"
@@ -67,20 +42,8 @@ static const char *describe_has_tests(struct manifest *m, void *check_result)
 	"when it's not defined: this helps ensure unrelated errors don't make\n"
 	"compilation fail.\n\n"
 
-	"Note that the tests are not linked against the files in the\n"
-	"above: you should directly #include those C files you want.  This\n"
-	"allows access to static functions and use special effects inside\n"
-	"test files\n", (char *)check_result);
-}
-
-static void handle_no_tests(struct manifest *m, void *check_result)
-{
-	FILE *run;
-	struct ccan_file *i;
-	char *test_dir = talloc_asprintf(m, "%s/test", m->dir);
-
-	if (check_result == test_is_not_dir)
-		return;
+	"Note that only API tests are linked against the files in the module!\n"
+		);
 
 	if (!ask("Should I create a template test/run.c file for you?"))
 		return;
@@ -101,41 +64,70 @@ static void handle_no_tests(struct manifest *m, void *check_result)
 			fprintf(run, "#include <ccan/%s/%s>\n",
 				m->basename, i->name);
 	}
-	fputs("#include <ccan/tap/tap.h>\n", run);
-	fputs("\n", run);
-
-	fputs("int main(void)\n", run);
-	fputs("{\n", run);
-	fputs("\t/* This is how many tests you plan to run */\n", run);
-	fputs("\tplan_tests(3);\n", run);
-	fputs("\n", run);
-	fputs("\t/* Simple thing we expect to succeed */\n", run);
-	fputs("\tok1(some_test())\n", run);
-	fputs("\t/* Same, with an explicit description of the test. */\n", run);
-	fputs("\tok(some_test(), \"%s with no args should return 1\", \"some_test\")\n", run);
-	fputs("\t/* How to print out messages for debugging. */\n", run);
-	fputs("\tdiag(\"Address of some_test is %p\", &some_test)\n", run);
-	fputs("\t/* Conditional tests must be explicitly skipped. */\n", run);
-	fputs("#if HAVE_SOME_FEATURE\n", run);
-	fputs("\tok1(test_some_feature())\n", run);
-	fputs("#else\n", run);
-	fputs("\tskip(1, \"Don\'t have SOME_FEATURE\")\n", run);
-	fputs("#endif\n", run);
-	fputs("\n", run);
-	fputs("\t/* This exits depending on whether all tests passed */\n", run);
-	fputs("\treturn exit_status();\n", run);
-	fputs("}\n", run);
-
+	fprintf(run, "%s",
+		"#include <ccan/tap/tap.h>\n\n"
+		"int main(void)\n"
+		"{\n"
+		"	/* This is how many tests you plan to run */\n"
+		"	plan_tests(3);\n"
+		"\n"
+		"	/* Simple thing we expect to succeed */\n"
+		"	ok1(some_test())\n"
+		"	/* Same, with an explicit description of the test. */\n"
+		"	ok(some_test(), \"%s with no args should return 1\", \"some_test\")\n"
+		"	/* How to print out messages for debugging. */\n"
+		"	diag(\"Address of some_test is %p\", &some_test)\n"
+		"	/* Conditional tests must be explicitly skipped. */\n"
+		"#if HAVE_SOME_FEATURE\n"
+		"	ok1(test_some_feature())\n"
+		"#else\n"
+		"	skip(1, \"Don\'t have SOME_FEATURE\")\n"
+		"#endif\n"
+		"\n"
+		"	/* This exits depending on whether all tests passed */\n"
+		"	return exit_status();\n"
+		"}\n");
 	fclose(run);
 }
 
+static void check_has_tests(struct manifest *m,
+			    bool keep,
+			    unsigned int *timeleft, struct score *score)
+{
+	struct stat st;
+	char *test_dir = talloc_asprintf(m, "%s/test", m->dir);
+
+	if (lstat(test_dir, &st) != 0) {
+		score->error = "No test directory";
+		if (errno != ENOENT)
+			err(1, "statting %s", test_dir);
+		has_tests.handle = handle_no_tests;
+		return;
+	}
+
+	if (!S_ISDIR(st.st_mode)) {
+		score->error = "test is not a directory";
+		return;
+	}
+
+	if (list_empty(&m->api_tests)
+	    && 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";
+			has_tests.handle = handle_no_tests;
+		} else
+			score->error = "No positive tests in test directory";
+		return;
+	}
+	score->pass = true;
+	score->score = score->total;
+}
+
 struct ccanlint has_tests = {
 	.key = "has-tests",
 	.name = "Module has tests",
 	.check = check_has_tests,
-	.describe = describe_has_tests,
-	.total_score = 1,
-	.handle = handle_no_tests,
 };
 
 REGISTER_TEST(has_tests, NULL);

tools/ccanlint/tests/idempotent.c

@@ -17,12 +17,69 @@
 static const char explain[] 
 = "Headers usually start with the C preprocessor lines to prevent multiple\n"
   "inclusions.  These look like the following:\n"
-  "#ifndef MY_HEADER_H\n"
-  "#define MY_HEADER_H\n"
+  "#ifndef CCAN_<MODNAME>_H\n"
+  "#define CCAN_<MODNAME>_H\n"
   "...\n"
-  "#endif /* MY_HEADER_H */\n";
+  "#endif /* CCAN_<MODNAME>_H */\n";
 
-static char *report_idem(struct ccan_file *f, char *sofar)
+static void fix_name(char *name)
+{
+	unsigned int i, j;
+
+	for (i = j = 0; name[i]; i++) {
+		if (isalnum(name[i]) || name[i] == '_')
+			name[j++] = toupper(name[i]);
+	}
+	name[j] = '\0';
+}
+
+static void handle_idem(struct manifest *m, struct score *score)
+{
+	struct file_error *e;
+
+	list_for_each(&score->per_file_errors, e, list) {
+		char *name, *q, *tmpname;
+		FILE *out;
+		unsigned int i;
+
+		/* Main header gets CCAN_FOO_H, others CCAN_FOO_XXX_H */
+		if (strstarts(e->file->name, m->basename)
+		    || strlen(e->file->name) != strlen(m->basename) + 2)
+			name = talloc_asprintf(score, "CCAN_%s_H", m->basename);
+		else
+			name = talloc_asprintf(score, "CCAN_%s_%s_H",
+					       m->basename, e->file->name);
+		fix_name(name);
+
+		q = talloc_asprintf(score,
+			    "Should I wrap %s in #ifndef/#define %s for you?",
+			    e->file->name, name);
+		if (!ask(q))
+			continue;
+
+		tmpname = maybe_temp_file(score, ".h", false, e->file->name);
+		out = fopen(tmpname, "w");
+		if (!out)
+			err(1, "Opening %s", tmpname);
+		if (fprintf(out, "#ifndef %s\n#define %s\n", name, name) < 0)
+			err(1, "Writing %s", tmpname);
+
+		for (i = 0; i < e->file->num_lines; i++)
+			if (fprintf(out, "%s\n", e->file->lines[i]) < 0)
+				err(1, "Writing %s", tmpname);
+
+		if (fprintf(out, "#endif /* %s */\n", name) < 0)
+			err(1, "Writing %s", tmpname);
+		
+		if (fclose(out) != 0)
+			err(1, "Closing %s", tmpname);
+
+		if (!move_file(tmpname, e->file->fullname))
+			err(1, "Moving %s to %s", tmpname, e->file->fullname);
+	}
+}
+
+static bool check_idem(struct ccan_file *f, struct score *score)
 {
 	struct line_info *line_info;
 	unsigned int i, first_preproc_line;
@@ -31,44 +88,48 @@ static char *report_idem(struct ccan_file *f, char *sofar)
 	line_info = get_ccan_line_info(f);
 	if (f->num_lines < 3)
 		/* FIXME: We assume small headers probably uninteresting. */
-		return sofar;
+		return true;
 
+	score->error = "Headers are not idempotent";
 	for (i = 0; i < f->num_lines; i++) {
 		if (line_info[i].type == DOC_LINE
 		    || line_info[i].type == COMMENT_LINE)
 			continue;
-		if (line_info[i].type == CODE_LINE)
-			return talloc_asprintf_append(sofar,
-			      "%s:%u:expect first non-comment line to be #ifndef.\n", f->name, i+1);
-		else if (line_info[i].type == PREPROC_LINE)
+		if (line_info[i].type == CODE_LINE) {
+			score_file_error(score, f, i+1,
+					 "Expect first non-comment line to be"
+					 " #ifndef.");
+			return false;
+		} else if (line_info[i].type == PREPROC_LINE)
 			break;
 	}
 
 	/* No code at all?  Don't complain. */
 	if (i == f->num_lines)
-		return sofar;
+		return true;
 
 	first_preproc_line = i;
 	for (i = first_preproc_line+1; i < f->num_lines; i++) {
 		if (line_info[i].type == DOC_LINE
 		    || line_info[i].type == COMMENT_LINE)
 			continue;
-		if (line_info[i].type == CODE_LINE)
-			return talloc_asprintf_append(sofar,
-			      "%s:%u:expect second line to be #define.\n", f->name, i+1);
-		else if (line_info[i].type == PREPROC_LINE)
+		if (line_info[i].type == CODE_LINE) {
+			score_file_error(score, f, i+1,
+					 "Expect second non-comment line to be"
+					 " #define.");
+			return false;
+		} else if (line_info[i].type == PREPROC_LINE)
 			break;
 	}
 
 	/* No code at all?  Weird. */
 	if (i == f->num_lines)
-		return sofar;
+		return true;
 
 	/* We expect a condition on this line. */
 	if (!line_info[i].cond) {
-		return talloc_asprintf_append(sofar,
-					      "%s:%u:expected #ifndef.\n",
-					      f->name, first_preproc_line+1);
+		score_file_error(score, f, i+1, "Expected #ifndef");
+		return false;
 	}
 
 	line = f->lines[i];
@@ -76,24 +137,27 @@ static char *report_idem(struct ccan_file *f, char *sofar)
 	/* We expect the condition to be ! IFDEF <symbol>. */
 	if (line_info[i].cond->type != PP_COND_IFDEF
 	    || !line_info[i].cond->inverse) {
-		return talloc_asprintf_append(sofar,
-					      "%s:%u:expected #ifndef.\n",
-					      f->name, first_preproc_line+1);
+		score_file_error(score, f, i+1, "Expected #ifndef");
+		return false;
 	}
 
 	/* And this to be #define <symbol> */
 	if (!get_token(&line, "#"))
 		abort();
 	if (!get_token(&line, "define")) {
-		return talloc_asprintf_append(sofar,
-			      "%s:%u:expected '#define %s'.\n",
-			      f->name, i+1, line_info[i].cond->symbol);
+		char *str = talloc_asprintf(score,
+					    "expected '#define %s'",
+					    line_info[i].cond->symbol);
+		score_file_error(score, f, i+1, str);
+		return false;
 	}
 	sym = get_symbol_token(f, &line);
 	if (!sym || !streq(sym, line_info[i].cond->symbol)) {
-		return talloc_asprintf_append(sofar,
-			      "%s:%u:expected '#define %s'.\n",
-			      f->name, i+1, line_info[i].cond->symbol);
+		char *str = talloc_asprintf(score,
+					    "expected '#define %s'",
+					    line_info[i].cond->symbol);
+		score_file_error(score, f, i+1, str);
+		return false;
 	}
 
 	/* Rest of code should all be covered by that conditional. */
@@ -103,42 +167,35 @@ static char *report_idem(struct ccan_file *f, char *sofar)
 		    || line_info[i].type == COMMENT_LINE)
 			continue;
 		if (get_ccan_line_pp(line_info[i].cond, sym, &val, NULL)
-		    != NOT_COMPILED)
-			return talloc_asprintf_append(sofar,
-			      "%s:%u:code outside idempotent region.\n",
-			      f->name, i+1);
+		    != NOT_COMPILED) {
+			score_file_error(score, f, i+1, "code outside"
+					 " idempotent region");
+			return false;
+		}
 	}
 
-	return sofar;
+	return true;
 }
 
-static void *check_idempotent(struct manifest *m,
-			      bool keep,
-			      unsigned int *timeleft)
+static void check_idempotent(struct manifest *m,
+			     bool keep,
+			     unsigned int *timeleft, struct score *score)
 {
 	struct ccan_file *f;
-	char *report = NULL;
-
-	list_for_each(&m->h_files, f, list)
-		report = report_idem(f, report);
 
-	return report;
-}
-
-static const char *describe_idempotent(struct manifest *m, void *check_result)
-{
-	return talloc_asprintf(check_result, 
-			       "Some headers not idempotent:\n"
-			       "%s\n%s", (char *)check_result,
-			       explain);
+	list_for_each(&m->h_files, f, list) {
+		if (!check_idem(f, score))
+			return;
+	}
+	score->pass = true;
+	score->score = score->total;
 }
 
 struct ccanlint idempotent = {
 	.key = "idempotent",
 	.name = "Module headers are #ifndef/#define wrapped",
-	.total_score = 1,
 	.check = check_idempotent,
-	.describe = describe_idempotent,
+	.handle = handle_idem,
 };
 
 REGISTER_TEST(idempotent, NULL);

tools/ccanlint/tests/run-coverage.c

@@ -4,6 +4,7 @@
 #include <ccan/str_talloc/str_talloc.h>
 #include <ccan/grab_file/grab_file.h>
 #include <ccan/str/str.h>
+#include <ccan/foreach/foreach.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -16,15 +17,9 @@
 #include <string.h>
 #include <ctype.h>
 
-struct coverage_result {
-	float uncovered;
-	const char *what;
-	char *output;
-};
-
-static bool find_source_file(struct manifest *m, const char *filename)
+static bool find_source_file(const struct manifest *m, const char *filename)
 {
-	struct ccan_file *i;
+	const struct ccan_file *i;
 
 	list_for_each(&m->c_files, i, list) {
 		if (streq(i->fullname, filename))
@@ -37,12 +32,25 @@ static bool find_source_file(struct manifest *m, const char *filename)
 	return false;
 }
 
+/* 1 point for 50%, 2 points for 75%, 3 points for 87.5%... */
+static unsigned int score_coverage(float covered, unsigned total)
+{
+	float thresh, uncovered = 1.0 - covered;
+	unsigned int i;
+
+	for (i = 0, thresh = 0.5; i < total; i++, thresh /= 2) {
+		if (uncovered > thresh)
+			break;
+	}
+	return i;
+}
+
+
 /* FIXME: Don't know how stable this is.  Read cov files directly? */
-static void analyze_coverage(struct manifest *m,
-			     struct coverage_result *res, const char *output,
-			     bool full_gcov)
+static void analyze_coverage(struct manifest *m, bool full_gcov,
+			     const char *output, struct score *score)
 {
-	char **lines = strsplit(res, output, "\n", NULL);
+	char **lines = strsplit(score, output, "\n", NULL);
 	float covered_lines = 0.0;
 	unsigned int i, total_lines = 0;
 	bool lines_matter = false;
@@ -81,17 +89,14 @@ static void analyze_coverage(struct manifest *m,
 			apostrophe = strchr(filename, '\'');
 			*apostrophe = '\0';
 			if (lines_matter) {
-				file = grab_file(res, filename, NULL);
+				file = grab_file(score, filename, NULL);
 				if (!file) {
-					res->what = talloc_asprintf(res,
+					score->error = talloc_asprintf(score,
 							    "Reading %s",
 							    filename);
-					res->output = talloc_strdup(res,
-							    strerror(errno));
 					return;
 				}
-				res->output = talloc_append_string(res->output,
-								   file);
+				printf("%s", file);
 			}
 			if (tools_verbose)
 				printf("Unlinking %s", filename);
@@ -99,114 +104,65 @@ static void analyze_coverage(struct manifest *m,
 		}
 	}
 
+	score->pass = true;
+
 	/* Nothing covered?  We can't tell if there's a source file which
 	 * was never executed, or there really is no code to execute, so
 	 * assume the latter: this test deserves no score. */
-	if (total_lines == 0) {
-		res->uncovered = 1.0;
-		run_coverage_tests.total_score = 0;
-	} else
-		res->uncovered = 1.0 - covered_lines / total_lines;
+	if (total_lines == 0)
+		score->total = score->score = 0;
+	else {
+		score->total = 5;
+		score->score = score_coverage(covered_lines / total_lines,
+					      score->total);
+	}
 }
 
-static void *do_run_coverage_tests(struct manifest *m,
-				   bool keep,
-				   unsigned int *timeleft)
+static void do_run_coverage_tests(struct manifest *m,
+				  bool keep,
+				  unsigned int *timeleft, struct score *score)
 {
-	struct coverage_result *res;
 	struct ccan_file *i;
 	char *cmdout;
 	char *covcmd;
 	bool ok;
 	bool full_gcov = (verbose > 1);
-
-	res = talloc(m, struct coverage_result);
-	res->what = NULL;
-	res->output = talloc_strdup(res, "");
-	res->uncovered = 1.0;
+	struct list_head *list;
 
 	/* This tells gcov where we put those .gcno files. */
 	covcmd = talloc_asprintf(m, "gcov %s -o %s",
 				 full_gcov ? "" : "-n",
-				 talloc_dirname(res, m->info_file->compiled));
+				 talloc_dirname(score, m->info_file->compiled));
 
 	/* Run them all. */
-	list_for_each(&m->run_tests, i, list) {
-		cmdout = run_command(m, timeleft, i->cov_compiled);
-		if (cmdout) {
-			res->what = i->fullname;
-			res->output = talloc_steal(res, cmdout);
-			return res;
-		}
-		covcmd = talloc_asprintf_append(covcmd, " %s", i->fullname);
-	}
-
-	list_for_each(&m->api_tests, i, list) {
-		cmdout = run_command(m, timeleft, i->cov_compiled);
-		if (cmdout) {
-			res->what = i->fullname;
-			res->output = talloc_steal(res, cmdout);
-			return res;
+	foreach_ptr(list, &m->run_tests, &m->api_tests) {
+		list_for_each(list, i, list) {
+			cmdout = run_command(m, timeleft, i->cov_compiled);
+			if (cmdout) {
+				score->error = "Running test with coverage";
+				score_file_error(score, i, 0, cmdout);
+				return;
+			}
+			covcmd = talloc_asprintf_append(covcmd, " %s",
+							i->fullname);
 		}
-		covcmd = talloc_asprintf_append(covcmd, " %s", i->fullname);
 	}
 
 	/* Now run gcov: we want output even if it succeeds. */
 	cmdout = run_with_timeout(m, covcmd, &ok, timeleft);
 	if (!ok) {
-		res->what = "Running gcov";
-		res->output = talloc_steal(res, cmdout);
-		return res;
-	}
-
-	analyze_coverage(m, res, cmdout, full_gcov);
-
-	return res;
-}
-
-/* 1 point for 50%, 2 points for 75%, 3 points for 87.5%... */
-static unsigned int score_coverage(struct manifest *m, void *check_result)
-{
-	struct coverage_result *res = check_result;
-	float thresh;
-	unsigned int i;
-
-	for (i = 0, thresh = 0.5;
-	     i < run_coverage_tests.total_score;
-	     i++, thresh /= 2) {
-		if (res->uncovered > thresh)
-			break;
+		score->error = talloc_asprintf(score, "Running gcov: %s",
+					       cmdout);
+		return;
 	}
-	return i;
-}
-
-static const char *describe_run_coverage_tests(struct manifest *m,
-					       void *check_result)
-{
-	struct coverage_result *res = check_result;
-	bool full_gcov = (verbose > 1);
-	char *ret;
-
-	if (res->what)
-		return talloc_asprintf(m, "%s: %s", res->what, res->output);
-
-	if (!verbose)
-		return NULL;
 
-	ret = talloc_asprintf(m, "Tests achieved %0.2f%% coverage",
-			      (1.0 - res->uncovered) * 100);
-	if (full_gcov)
-		ret = talloc_asprintf_append(ret, "\n%s", res->output);
-	return ret;
+	analyze_coverage(m, full_gcov, cmdout, score);
 }
 
 struct ccanlint run_coverage_tests = {
 	.key = "test-coverage",
 	.name = "Code coverage of module tests",
-	.total_score = 5,
-	.score = score_coverage,
 	.check = do_run_coverage_tests,
-	.describe = describe_run_coverage_tests,
 };
 
 REGISTER_TEST(run_coverage_tests, &compile_coverage_tests, &run_tests, NULL);

tools/ccanlint/tests/run_tests.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>
@@ -21,91 +22,43 @@ static const char *can_run(struct manifest *m)
 	return NULL;
 }
 
-struct run_tests_result {
-	struct list_node list;
-	struct ccan_file *file;
-	const char *output;
-};
-
-static void *do_run_tests(struct manifest *m,
-			  bool keep,
-			  unsigned int *timeleft)
+static void do_run_tests(struct manifest *m,
+			 bool keep,
+			 unsigned int *timeleft,
+			 struct score *score)
 {
-	struct list_head *list = talloc(m, struct list_head);
-	struct run_tests_result *res;
+	struct list_head *list;
 	struct ccan_file *i;
 	char *cmdout;
 
-	list_head_init(list);
-	run_tests.total_score = 0;
-
-	list_for_each(&m->run_tests, i, list) {
-		run_tests.total_score++;
-		cmdout = run_command(m, timeleft, i->compiled);
-		if (cmdout) {
-			res = talloc(list, struct run_tests_result);
-			res->file = i;
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
-		}
-	}
-
-	list_for_each(&m->api_tests, i, list) {
-		run_tests.total_score++;
-		cmdout = run_command(m, timeleft, i->compiled);
-		if (cmdout) {
-			res = talloc(list, struct run_tests_result);
-			res->file = i;
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+	score->total = 0;
+	foreach_ptr(list, &m->run_tests, &m->api_tests) {
+		list_for_each(list, i, list) {
+			score->total++;
+			cmdout = run_command(m, timeleft, i->compiled);
+			if (cmdout)
+				score_file_error(score, i, 0, cmdout);
+			else
+				score->score++;
 		}
 	}
 
-	if (list_empty(list)) {
-		talloc_free(list);
-		list = NULL;
-	}
-
-	return list;
-}
-
-static unsigned int score_run_tests(struct manifest *m, void *check_result)
-{
-	struct list_head *list = check_result;
-	struct run_tests_result *i;
-	unsigned int score = run_tests.total_score;
-
-	list_for_each(list, i, list)
-		score--;
-	return score;
-}
-
-static const char *describe_run_tests(struct manifest *m,
-					  void *check_result)
-{
-	struct list_head *list = check_result;
-	char *descrip = talloc_strdup(check_result, "Running tests failed:\n");
-	struct run_tests_result *i;
-
-	list_for_each(list, i, list)
-		descrip = talloc_asprintf_append(descrip, "Running %s:\n%s",
-						 i->file->name, i->output);
-	return descrip;
+	if (score->score == score->total)
+		score->pass = true;
 }
 
 /* Gcc's warn_unused_result is fascist bullshit. */
 #define doesnt_matter()
 
-static void run_under_debugger(struct manifest *m, void *check_result)
+static void run_under_debugger(struct manifest *m, struct score *score)
 {
 	char *command;
-	struct list_head *list = check_result;
-	struct run_tests_result *first;
+	struct file_error *first;
 
 	if (!ask("Should I run the first failing test under the debugger?"))
 		return;
 
-	first = list_top(list, struct run_tests_result, list);
+	first = list_top(&score->per_file_errors, struct file_error, list);
 	command = talloc_asprintf(m, "gdb -ex 'break tap.c:136' -ex 'run' %s",
 				  first->file->compiled);
 	if (system(command))
@@ -115,12 +68,9 @@ static void run_under_debugger(struct manifest *m, void *check_result)
 struct ccanlint run_tests = {
 	.key = "run",
 	.name = "Module's run and api tests pass",
-	.score = score_run_tests,
-	.total_score = 1,
 	.check = do_run_tests,
-	.describe = describe_run_tests,
+	.handle = run_under_debugger,
 	.can_run = can_run,
-	.handle = run_under_debugger
 };
 
 REGISTER_TEST(run_tests, &compile_tests, NULL);

tools/ccanlint/tests/run_tests_valgrind.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>
@@ -18,102 +19,55 @@
 static const char *can_run_vg(struct manifest *m)
 {
 	unsigned int timeleft = default_timeout_ms;
-	char *output = run_command(m, &timeleft, "valgrind -q --error-exitcode=0 true");
+	char *output = run_command(m, &timeleft,
+				   "valgrind -q --error-exitcode=0 true");
 
 	if (output)
 		return talloc_asprintf(m, "No valgrind support: %s", output);
 	return NULL;
 }
 
-struct run_tests_result {
-	struct list_node list;
-	struct ccan_file *file;
-	const char *output;
-};
-
-static void *do_run_tests_vg(struct manifest *m,
+/* FIXME: Run examples, too! */
+static void do_run_tests_vg(struct manifest *m,
 			     bool keep,
-			     unsigned int *timeleft)
+			    unsigned int *timeleft,
+			    struct score *score)
 {
-	struct list_head *list = talloc(m, struct list_head);
-	struct run_tests_result *res;
 	struct ccan_file *i;
+	struct list_head *list;
 	char *cmdout;
 
-	list_head_init(list);
-	run_tests_vg.total_score = 0;
-
-	list_for_each(&m->run_tests, i, list) {
-		run_tests_vg.total_score++;
-		cmdout = run_command(m, timeleft,
-				     "valgrind -q --error-exitcode=100 %s",
-				     i->compiled);
-		if (cmdout) {
-			res = talloc(list, struct run_tests_result);
-			res->file = i;
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
-		}
-	}
-
-	list_for_each(&m->api_tests, i, list) {
-		run_tests_vg.total_score++;
-		cmdout = run_command(m, timeleft,
+	score->total = 0;
+	foreach_ptr(list, &m->run_tests, &m->api_tests) {
+		list_for_each(list, i, list) {
+			score->total++;
+			cmdout = run_command(m, timeleft,
 				     "valgrind -q --error-exitcode=100 %s",
 				     i->compiled);
-		if (cmdout) {
-			res = talloc(list, struct run_tests_result);
-			res->file = i;
-			res->output = talloc_steal(res, cmdout);
-			list_add_tail(list, &res->list);
+			if (cmdout) {
+				score->error = "Running under valgrind";
+				score_file_error(score, i, 0, cmdout);
+			} else
+				score->score++;
 		}
 	}
 
-	if (list_empty(list)) {
-		talloc_free(list);
-		list = NULL;
-	}
-
-	return list;
-}
-
-static unsigned int score_run_tests_vg(struct manifest *m, void *check_result)
-{
-	struct list_head *list = check_result;
-	struct run_tests_result *i;
-	unsigned int score = run_tests_vg.total_score;
-
-	list_for_each(list, i, list)
-		score--;
-	return score;
-}
-
-static const char *describe_run_tests_vg(struct manifest *m,
-					 void *check_result)
-{
-	struct list_head *list = check_result;
-	char *descrip = talloc_strdup(check_result, "Running tests under valgrind failed:\n");
-	struct run_tests_result *i;
-
-	list_for_each(list, i, list)
-		descrip = talloc_asprintf_append(descrip, "Running %s:\n%s",
-						 i->file->name, i->output);
-	return descrip;
+	if (score->score == score->total)
+		score->pass = true;
 }
 
 /* Gcc's warn_unused_result is fascist bullshit. */
 #define doesnt_matter()
 
-static void run_under_debugger_vg(struct manifest *m, void *check_result)
+static void run_under_debugger_vg(struct manifest *m, struct score *score)
 {
-	struct list_head *list = check_result;
-	struct run_tests_result *first;
+	struct file_error *first;
 	char *command;
 
 	if (!ask("Should I run the first failing test under the debugger?"))
 		return;
 
-	first = list_top(list, struct run_tests_result, list);
+	first = list_top(&score->per_file_errors, struct file_error, list);
 	command = talloc_asprintf(m, "valgrind --db-attach=yes %s",
 				  first->file->compiled);
 	if (system(command))
@@ -123,11 +77,8 @@ static void run_under_debugger_vg(struct manifest *m, void *check_result)
 struct ccanlint run_tests_vg = {
 	.key = "valgrind-tests",
 	.name = "Module's run and api tests succeed under valgrind",
-	.score = score_run_tests_vg,
-	.total_score = 1,
-	.check = do_run_tests_vg,
-	.describe = describe_run_tests_vg,
 	.can_run = can_run_vg,
+	.check = do_run_tests_vg,
 	.handle = run_under_debugger_vg
 };
 

tools/ccanlint/tests/trailing_whitespace.c

 /* Trailing whitespace test.  Almost embarrassing, but trivial. */
 #include <tools/ccanlint/ccanlint.h>
 #include <ccan/talloc/talloc.h>
+#include <ccan/foreach/foreach.h>
 #include <ccan/str/str.h>
 
 /* FIXME: only print full analysis if verbose >= 2.  */
-static char *report_on_trailing_whitespace(const char *line)
+static char *get_trailing_whitespace(const char *line)
 {
 	const char *e = strchr(line, 0);
 	while (e>line && (e[-1]==' ' || e[-1]=='\t'))
@@ -20,36 +21,38 @@ static char *report_on_trailing_whitespace(const char *line)
 	return talloc_asprintf(line, "'%s'", line);
 }
 
-static void *check_trailing_whitespace(struct manifest *m,
-				       bool keep,
-				       unsigned int *timeleft)
+static void check_trailing_whitespace(struct manifest *m,
+				      bool keep,
+				      unsigned int *timeleft,
+				      struct score *score)
 {
-	char *report;
-
-	report = report_on_lines(&m->c_files, report_on_trailing_whitespace,
-				 NULL);
-	report = report_on_lines(&m->h_files, report_on_trailing_whitespace,
-				 report);
-
-	return report;
-}
-
-static const char *describe_trailing_whitespace(struct manifest *m,
-						void *check_result)
-{
-	if (!verbose)
-		return NULL;
-	return talloc_asprintf(check_result, 
-			       "Some source files have trailing whitespace:\n"
-			       "%s", (char *)check_result);
+	struct list_head *list;
+	struct ccan_file *f;
+	unsigned int i;
+
+	foreach_ptr(list, &m->c_files, &m->h_files) {
+		list_for_each(list, f, list) {
+			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";
+					score_file_error(score, f, i+1, err);
+				}
+			}
+		}
+	}
+	if (!score->error) {
+		score->pass = true;
+		score->score = score->total;
+	}
 }
 
 struct ccanlint trailing_whitespace = {
 	.key = "trailing-whitespace",
 	.name = "Module's source code has no trailing whitespace",
-	.total_score = 1,
 	.check = check_trailing_whitespace,
-	.describe = describe_trailing_whitespace,
 };