From 03a596908b779bbb4b7c2f739c5e238f8c5d6390 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 14 Nov 2010 22:10:04 +1030 Subject: [PATCH] ccanlint: make compile commands return output. We want to distinguish between warnings and errors: the first step is to return the output even if the command doesn't fail. --- tools/ccanlint/compulsory_tests/build_objs.c | 9 ++--- tools/ccanlint/compulsory_tests/check_build.c | 12 ++++--- .../compulsory_tests/check_includes_build.c | 13 ++++---- tools/ccanlint/tests/build-coverage.c | 24 +++++++------- tools/ccanlint/tests/compile_test_helpers.c | 9 +++-- tools/ccanlint/tests/compile_tests.c | 10 +++--- tools/ccanlint/tests/examples_compile.c | 8 ++--- tools/ccanlint/tests/run-coverage.c | 13 ++++---- tools/ccanlint/tests/run_tests.c | 8 ++--- tools/ccanlint/tests/run_tests_valgrind.c | 18 +++++----- tools/compile.c | 33 +++++++++---------- tools/depends.c | 8 ++--- tools/tools.c | 30 ++++++++--------- tools/tools.h | 23 +++++++------ 14 files changed, 112 insertions(+), 106 deletions(-) diff --git a/tools/ccanlint/compulsory_tests/build_objs.c b/tools/ccanlint/compulsory_tests/build_objs.c index aa8e7b36..77f0a70c 100644 --- a/tools/ccanlint/compulsory_tests/build_objs.c +++ b/tools/ccanlint/compulsory_tests/build_objs.c @@ -31,16 +31,17 @@ static void check_objs_build(struct manifest *m, score->total = 0; list_for_each(&m->c_files, i, list) { - char *err; + char *output; char *fullfile = talloc_asprintf(m, "%s/%s", m->dir, i->name); i->compiled = maybe_temp_file(m, "", keep, fullfile); - err = compile_object(m, fullfile, ccan_dir, "", i->compiled); - if (err) { + if (!compile_object(m, fullfile, ccan_dir, "", i->compiled, + &output)) { talloc_free(i->compiled); score->error = "Compiling object files"; - score_file_error(score, i, 0, err); + score_file_error(score, i, 0, output); } + talloc_free(output); } if (!score->error) { score->pass = true; diff --git a/tools/ccanlint/compulsory_tests/check_build.c b/tools/ccanlint/compulsory_tests/check_build.c index 9175b918..830ea914 100644 --- a/tools/ccanlint/compulsory_tests/check_build.c +++ b/tools/ccanlint/compulsory_tests/check_build.c @@ -50,7 +50,7 @@ static void check_use_build(struct manifest *m, unsigned int *timeleft, struct score *score) { char *contents; - char *tmpfile; + char *tmpfile, *cmdout; char *basename = talloc_asprintf(m, "%s/example.c", m->dir); int fd; @@ -71,12 +71,14 @@ static void check_use_build(struct manifest *m, err(1, "Failure writing to temporary file %s", tmpfile); close(fd); - score->error = compile_and_link(m, tmpfile, ccan_dir, obj_list(m), "", - lib_list(m), - maybe_temp_file(m, "", keep, tmpfile)); - if (!score->error) { + if (compile_and_link(score, tmpfile, ccan_dir, obj_list(m), "", + lib_list(m), + maybe_temp_file(m, "", keep, tmpfile), + &cmdout)) { score->pass = true; score->score = score->total; + } else { + score->error = cmdout; } } diff --git a/tools/ccanlint/compulsory_tests/check_includes_build.c b/tools/ccanlint/compulsory_tests/check_includes_build.c index cc5edc4c..4e2badda 100644 --- a/tools/ccanlint/compulsory_tests/check_includes_build.c +++ b/tools/ccanlint/compulsory_tests/check_includes_build.c @@ -40,7 +40,7 @@ static void check_includes_build(struct manifest *m, unsigned int *timeleft, struct score *score) { char *contents; - char *tmpsrc, *tmpobj; + char *tmpsrc, *tmpobj, *cmdout; int fd; struct ccan_file *mainh = main_header(m); @@ -57,14 +57,13 @@ static void check_includes_build(struct manifest *m, err(1, "writing to temporary file %s", tmpsrc); close(fd); - 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 { + if (compile_object(score, tmpsrc, ccan_dir, "", tmpobj, &cmdout)) { score->pass = true; score->score = score->total; + } else { + score->error = talloc_asprintf(score, + "#include of the main header file:\n%s", + cmdout); } } diff --git a/tools/ccanlint/tests/build-coverage.c b/tools/ccanlint/tests/build-coverage.c index 6eb6ab5d..9415647f 100644 --- a/tools/ccanlint/tests/build-coverage.c +++ b/tools/ccanlint/tests/build-coverage.c @@ -18,9 +18,9 @@ static const char *can_run_coverage(struct manifest *m) { unsigned int timeleft = default_timeout_ms; - char *output = run_command(m, &timeleft, "gcov -h"); + char *output; - if (output) + if (!run_command(m, &timeleft, &output, "gcov -h")) return talloc_asprintf(m, "No gcov support: %s", output); return NULL; } @@ -37,9 +37,8 @@ static bool build_module_objs_with_coverage(struct manifest *m, bool keep, char *fullfile = talloc_asprintf(m, "%s/%s", m->dir, i->name); i->cov_compiled = maybe_temp_file(m, "", keep, fullfile); - err = compile_object(m, fullfile, ccan_dir, "", - i->cov_compiled); - if (err) { + if (!compile_object(m, fullfile, ccan_dir, "", + i->cov_compiled, &err)) { score_file_error(score, i, 0, err); talloc_free(i->cov_compiled); i->cov_compiled = NULL; @@ -95,19 +94,18 @@ static char *cov_compile(const void *ctx, const char *modobjs, bool keep) { - char *errmsg; + char *output; file->cov_compiled = maybe_temp_file(ctx, "", keep, file->fullname); - errmsg = compile_and_link(ctx, file->fullname, ccan_dir, - obj_list(m, modobjs), - COVERAGE_CFLAGS, - lib_list(m), file->cov_compiled); - if (errmsg) { + if (!compile_and_link(ctx, file->fullname, ccan_dir, + obj_list(m, modobjs), + COVERAGE_CFLAGS, + lib_list(m), file->cov_compiled, &output)) { talloc_free(file->cov_compiled); file->cov_compiled = NULL; - return errmsg; + return output; } - + talloc_free(output); return NULL; } diff --git a/tools/ccanlint/tests/compile_test_helpers.c b/tools/ccanlint/tests/compile_test_helpers.c index e2e3ed84..3d5a8a74 100644 --- a/tools/ccanlint/tests/compile_test_helpers.c +++ b/tools/ccanlint/tests/compile_test_helpers.c @@ -25,9 +25,14 @@ static char *compile(struct manifest *m, bool keep, struct ccan_file *cfile) { + char *output; cfile->compiled = maybe_temp_file(m, ".o", keep, cfile->fullname); - return compile_object(m, cfile->fullname, ccan_dir, "", - cfile->compiled); + if (compile_object(m, cfile->fullname, ccan_dir, "", + cfile->compiled, &output)) { + talloc_free(output); + return NULL; + } + return output; } static void do_compile_test_helpers(struct manifest *m, diff --git a/tools/ccanlint/tests/compile_tests.c b/tools/ccanlint/tests/compile_tests.c index b54ce958..3549cded 100644 --- a/tools/ccanlint/tests/compile_tests.c +++ b/tools/ccanlint/tests/compile_tests.c @@ -71,14 +71,14 @@ static char *compile(const void *ctx, char *errmsg; file->compiled = maybe_temp_file(ctx, "", keep, file->fullname); - errmsg = compile_and_link(ctx, file->fullname, ccan_dir, - obj_list(m, link_with_module), - fail ? "-DFAIL" : "", - lib_list(m), file->compiled); - if (errmsg) { + if (!compile_and_link(ctx, file->fullname, ccan_dir, + obj_list(m, link_with_module), + fail ? "-DFAIL" : "", + lib_list(m), file->compiled, &errmsg)) { talloc_free(file->compiled); return errmsg; } + talloc_free(errmsg); return NULL; } diff --git a/tools/ccanlint/tests/examples_compile.c b/tools/ccanlint/tests/examples_compile.c index 3946bdb3..1b2360ed 100644 --- a/tools/ccanlint/tests/examples_compile.c +++ b/tools/ccanlint/tests/examples_compile.c @@ -119,14 +119,14 @@ static char *compile(const void *ctx, char *errmsg; file->compiled = maybe_temp_file(ctx, "", keep, file->fullname); - errmsg = compile_and_link(ctx, file->fullname, ccan_dir, - obj_list(m, file), - "", lib_list(m), file->compiled); - if (errmsg) { + if (!compile_and_link(ctx, file->fullname, ccan_dir, + obj_list(m, file), + "", lib_list(m), file->compiled, &errmsg)) { talloc_free(file->compiled); file->compiled = NULL; return errmsg; } + talloc_free(errmsg); return NULL; } diff --git a/tools/ccanlint/tests/run-coverage.c b/tools/ccanlint/tests/run-coverage.c index 8e756c7d..a526d55e 100644 --- a/tools/ccanlint/tests/run-coverage.c +++ b/tools/ccanlint/tests/run-coverage.c @@ -125,7 +125,6 @@ static void do_run_coverage_tests(struct manifest *m, struct ccan_file *i; char *cmdout; char *covcmd; - bool ok; bool full_gcov = (verbose > 1); struct list_head *list; @@ -137,20 +136,20 @@ static void do_run_coverage_tests(struct manifest *m, /* Run them all. */ 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) { + if (run_command(score, timeleft, &cmdout, + "%s", i->cov_compiled)) { + covcmd = talloc_asprintf_append(covcmd, " %s", + i->fullname); + } else { score->error = "Running test with coverage"; score_file_error(score, i, 0, cmdout); return; } - 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) { + if (!run_command(score, timeleft, &cmdout, "%s", covcmd)) { score->error = talloc_asprintf(score, "Running gcov: %s", cmdout); return; diff --git a/tools/ccanlint/tests/run_tests.c b/tools/ccanlint/tests/run_tests.c index 0a8434c6..f32e0534 100644 --- a/tools/ccanlint/tests/run_tests.c +++ b/tools/ccanlint/tests/run_tests.c @@ -35,11 +35,11 @@ static void do_run_tests(struct manifest *m, 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 + if (run_command(m, timeleft, &cmdout, "%s", + i->compiled)) score->score++; + else + score_file_error(score, i, 0, cmdout); } } diff --git a/tools/ccanlint/tests/run_tests_valgrind.c b/tools/ccanlint/tests/run_tests_valgrind.c index e0c2ab91..7ab67dbc 100644 --- a/tools/ccanlint/tests/run_tests_valgrind.c +++ b/tools/ccanlint/tests/run_tests_valgrind.c @@ -19,10 +19,10 @@ 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; - if (output) + if (!run_command(m, &timeleft, &output, + "valgrind -q --error-exitcode=0 true")) return talloc_asprintf(m, "No valgrind support: %s", output); return NULL; } @@ -41,14 +41,14 @@ static void do_run_tests_vg(struct manifest *m, 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) { + if (run_command(score, timeleft, &cmdout, + "valgrind -q --error-exitcode=100 %s", + i->compiled)) { + score->score++; + } else { score->error = "Running under valgrind"; score_file_error(score, i, 0, cmdout); - } else - score->score++; + } } } diff --git a/tools/compile.c b/tools/compile.c index 8f80734b..3ab1afaa 100644 --- a/tools/compile.c +++ b/tools/compile.c @@ -4,7 +4,7 @@ bool compile_verbose = false; -/* Compile multiple object files into a single. Returns errmsg if fails. */ +/* Compile multiple object files into a single. Returns NULL if fails. */ char *link_objects(const void *ctx, const char *basename, bool in_pwd, const char *objs, char **errmsg) { @@ -13,35 +13,34 @@ char *link_objects(const void *ctx, const char *basename, bool in_pwd, if (compile_verbose) printf("Linking objects into %s\n", file); - *errmsg = run_command(ctx, NULL, "ld -r -o %s %s", file, objs); - if (*errmsg) { - talloc_free(file); - return NULL; - } - return file; + if (run_command(ctx, NULL, errmsg, "ld -r -o %s %s", file, objs)) + return file; + + talloc_free(file); + return NULL; } -/* Compile a single C file to an object file. Returns errmsg if fails. */ -char *compile_object(const void *ctx, const char *cfile, const char *ccandir, - const char *extra_cflags, - const char *outfile) +/* Compile a single C file to an object file. */ +bool compile_object(const void *ctx, const char *cfile, const char *ccandir, + const char *extra_cflags, + const char *outfile, char **output) { if (compile_verbose) printf("Compiling %s\n", outfile); - return run_command(ctx, NULL, CCAN_COMPILER " " CCAN_CFLAGS + return run_command(ctx, NULL, output, CCAN_COMPILER " " CCAN_CFLAGS " -I%s %s -c -o %s %s", ccandir, extra_cflags, outfile, cfile); } /* Compile and link single C file, with object files. - * Returns error message or NULL on success. */ -char *compile_and_link(const void *ctx, const char *cfile, const char *ccandir, - const char *objs, const char *extra_cflags, - const char *libs, const char *outfile) + * Returns false on failure. */ +bool compile_and_link(const void *ctx, const char *cfile, const char *ccandir, + const char *objs, const char *extra_cflags, + const char *libs, const char *outfile, char **output) { if (compile_verbose) printf("Compiling and linking %s\n", outfile); - return run_command(ctx, NULL, CCAN_COMPILER " " CCAN_CFLAGS + return run_command(ctx, NULL, output, CCAN_COMPILER " " CCAN_CFLAGS " -I%s %s -o %s %s %s %s", ccandir, extra_cflags, outfile, cfile, objs, libs); } diff --git a/tools/depends.c b/tools/depends.c index e2666d97..4bac1b06 100644 --- a/tools/depends.c +++ b/tools/depends.c @@ -39,7 +39,7 @@ lines_from_cmd(const void *ctx, unsigned int *num, char *format, ...) * temp_file helps here. */ static char *compile_info(const void *ctx, const char *dir) { - char *info_c_file, *info, *ccandir, *compiled; + char *info_c_file, *info, *ccandir, *compiled, *output; size_t len; int fd; @@ -63,9 +63,9 @@ static char *compile_info(const void *ctx, const char *dir) compiled = maybe_temp_file(ctx, "", false, "info"); if (compile_and_link(ctx, info_c_file, ccandir, "", "", "", - compiled)) - return NULL; - return compiled; + compiled, &output)) + return compiled; + return NULL; } static char **get_one_deps(const void *ctx, const char *dir, diff --git a/tools/tools.c b/tools/tools.c index 20fcc9bb..eeaff6f1 100644 --- a/tools/tools.c +++ b/tools/tools.c @@ -143,35 +143,35 @@ char *run_with_timeout(const void *ctx, const char *cmd, return ret; } -/* Returns output if command fails. */ -char *run_command(const void *ctx, unsigned int *time_ms, const char *fmt, ...) +/* Tallocs *output off ctx; return false if command fails. */ +bool run_command(const void *ctx, unsigned int *time_ms, char **output, + const char *fmt, ...) { va_list ap; - char *cmd, *contents; + char *cmd; bool ok; unsigned int default_time = default_timeout_ms; if (!time_ms) time_ms = &default_time; - else if (*time_ms == 0) - return talloc_strdup(ctx, "\n== TIMED OUT ==\n"); + else if (*time_ms == 0) { + *output = talloc_strdup(ctx, "\n== TIMED OUT ==\n"); + return false; + } va_start(ap, fmt); cmd = talloc_vasprintf(ctx, fmt, ap); va_end(ap); - contents = run_with_timeout(ctx, cmd, &ok, time_ms); - if (ok) { - talloc_free(contents); - return NULL; - } - - if (!contents) + *output = run_with_timeout(ctx, cmd, &ok, time_ms); + if (ok) + return true; + if (!*output) err(1, "Problem running child"); if (*time_ms == 0) - contents = talloc_asprintf_append(contents, - "\n== TIMED OUT ==\n"); - return contents; + *output = talloc_asprintf_append(*output, + "\n== TIMED OUT ==\n"); + return false; } static int unlink_all(char *dir) diff --git a/tools/tools.h b/tools/tools.h index 168a47bd..d39f0197 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -1,6 +1,7 @@ #ifndef CCAN_TOOLS_H #define CCAN_TOOLS_H #include +#include #include "config.h" #ifndef CCAN_COMPILER @@ -36,7 +37,10 @@ extern bool tools_verbose; char *talloc_basename(const void *ctx, const char *dir); char *talloc_dirname(const void *ctx, const char *dir); char *talloc_getcwd(const void *ctx); -char *run_command(const void *ctx, unsigned int *time_ms, const char *fmt, ...); +bool PRINTF_FMT(4,5) run_command(const void *ctx, + unsigned int *time_ms, + char **output, + const char *fmt, ...); char *run_with_timeout(const void *ctx, const char *cmd, bool *ok, unsigned *timeout_ms); char *temp_dir(const void *ctx); @@ -52,15 +56,14 @@ extern bool compile_verbose; /* Compile multiple object files into a single. */ char *link_objects(const void *ctx, const char *basename, bool in_pwd, const char *objs, char **errmsg); -/* Compile a single C file to an object file. Returns errmsg if fails. */ -char *compile_object(const void *ctx, const char *cfile, const char *ccandir, - const char *extra_cflags, - const char *outfile); -/* Compile and link single C file, with object files, libs, etc. NULL on - * success, error output on fail. */ -char *compile_and_link(const void *ctx, const char *cfile, const char *ccandir, - const char *objs, const char *extra_cflags, - const char *libs, const char *outfile); +/* Compile a single C file to an object file. Returns false if fails. */ +bool compile_object(const void *ctx, const char *cfile, const char *ccandir, + const char *extra_cflags, + const char *outfile, char **output); +/* Compile and link single C file, with object files, libs, etc. */ +bool compile_and_link(const void *ctx, const char *cfile, const char *ccandir, + const char *objs, const char *extra_cflags, + const char *libs, const char *outfile, char **output); /* If in_pwd is false, return a file int temp_dir, otherwise a local file. */ char *maybe_temp_file(const void *ctx, const char *extension, bool in_pwd, -- 2.39.2