From 9f13afa20f91a7f2fd9393c29bb583aa8f748f0c Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Mon, 11 Aug 2014 17:25:09 -0500 Subject: Add jq_report_error() function; use it Put a stop to fprintf(stderr, ...) where we shouldn't. --- compile.c | 2 +- execute.c | 91 ++++++++++++++++++++++++++++++++++++++++++++------------------- jq.h | 2 ++ linker.c | 4 +-- locfile.c | 41 +++++----------------------- parser.y | 8 +++--- tests/run | 4 +-- 7 files changed, 82 insertions(+), 70 deletions(-) diff --git a/compile.c b/compile.c index 6005f7c3..92189687 100644 --- a/compile.c +++ b/compile.c @@ -818,7 +818,7 @@ static int expand_call_arglist(block* b) { for (inst* curr; (curr = block_take(b));) { if (opcode_describe(curr->op)->flags & OP_HAS_BINDING) { if (!curr->bound_by) { - locfile_locate(curr->locfile, curr->source, "error: %s/%d is not defined", curr->symbol, block_count_actuals(curr->arglist)); + locfile_locate(curr->locfile, curr->source, "jq: error: %s/%d is not defined", curr->symbol, block_count_actuals(curr->arglist)); errors++; // don't process this instruction if it's not well-defined ret = BLOCK(ret, inst_block(curr)); diff --git a/execute.c b/execute.c index ec88b092..53b67c18 100644 --- a/execute.c +++ b/execute.c @@ -256,15 +256,18 @@ static void jq_reset(jq_state *jq) { jq->subexp_nest = 0; } -static void print_error(jq_state *jq, jv value) { - assert(!jv_is_valid(value)); - if (jq->err_cb) { - // callback must jv_free() its jv argument - jq->err_cb(jq->err_cb_data, jv_invalid_get_msg(jv_copy(value))); - } +void jq_report_error(jq_state *jq, jv value) { + assert(jq->err_cb); + // callback must jv_free() its jv argument + jq->err_cb(jq->err_cb_data, value); +} + +static void set_error(jq_state *jq, jv value) { + // Record so try/catch can find it. jv_free(jq->error); jq->error = value; } + #define ON_BACKTRACK(op) ((op)+NUM_OPCODES) jv jq_next(jq_state *jq) { @@ -390,8 +393,8 @@ jv jq_next(jq_state *jq) { stack_push(jq, jv_object_set(objv, k, v)); stack_push(jq, stktop); } else { - print_error(jq, jv_invalid_with_msg(jv_string_fmt("Cannot use %s as object key", - jv_kind_name(jv_get_kind(k))))); + set_error(jq, jv_invalid_with_msg(jv_string_fmt("Cannot use %s as object key", + jv_kind_name(jv_get_kind(k))))); jv_free(stktop); jv_free(v); jv_free(k); @@ -410,7 +413,7 @@ jv jq_next(jq_state *jq) { if (raising) goto do_backtrack; if (jv_get_kind(*var) != JV_KIND_NUMBER || jv_get_kind(max) != JV_KIND_NUMBER) { - print_error(jq, jv_invalid_with_msg(jv_string_fmt("Range bounds must be numeric"))); + set_error(jq, jv_invalid_with_msg(jv_string_fmt("Range bounds must be numeric"))); jv_free(max); goto do_backtrack; } else if (jv_number_value(jv_copy(*var)) >= jv_number_value(jv_copy(max))) { @@ -524,7 +527,7 @@ jv jq_next(jq_state *jq) { stack_push(jq, v); } else { if (opcode == INDEX) - print_error(jq, v); + set_error(jq, v); else jv_free(v); goto do_backtrack; @@ -582,9 +585,9 @@ jv jq_next(jq_state *jq) { } else { assert(opcode == EACH || opcode == EACH_OPT); if (opcode == EACH) { - print_error(jq, - jv_invalid_with_msg(jv_string_fmt("Cannot iterate over %s", - jv_kind_name(jv_get_kind(container))))); + set_error(jq, + jv_invalid_with_msg(jv_string_fmt("Cannot iterate over %s", + jv_kind_name(jv_get_kind(container))))); } keep_going = 0; } @@ -681,7 +684,7 @@ jv jq_next(jq_state *jq) { if (jv_is_valid(top)) { stack_push(jq, top); } else { - print_error(jq, top); + set_error(jq, top); goto do_backtrack; } break; @@ -758,6 +761,48 @@ jv jq_next(jq_state *jq) { } } +jv jq_format_error(jv msg) { + if (jv_get_kind(msg) == JV_KIND_NULL || + (jv_get_kind(msg) == JV_KIND_INVALID && !jv_invalid_has_msg(jv_copy(msg)))) { + jv_free(msg); + fprintf(stderr, "jq: error: out of memory\n"); + return jv_null(); + } + + if (jv_get_kind(msg) == JV_KIND_STRING) + return msg; // expected to already be formatted + + if (jv_get_kind(msg) == JV_KIND_INVALID) + msg = jv_invalid_get_msg(msg); + + if (jv_get_kind(msg) == JV_KIND_NULL) + return jq_format_error(msg); // ENOMEM + + // Invalid with msg; prefix with "jq: error: " + + if (jv_get_kind(msg) != JV_KIND_INVALID) { + if (jv_get_kind(msg) == JV_KIND_STRING) + return jv_string_fmt("jq: error: %s", msg); + + msg = jv_dump_string(msg, JV_PRINT_INVALID); + if (jv_get_kind(msg) == JV_KIND_STRING) + return jv_string_fmt("jq: error: %s", msg); + return jq_format_error(jv_null()); // ENOMEM + } + + // An invalid inside an invalid! + return jq_format_error(jv_invalid_get_msg(msg)); +} + +// XXX Refactor into a utility function that returns a jv and one that +// uses it and then prints that jv's string as the complete error +// message. +static void default_err_cb(void *data, jv msg) { + msg = jq_format_error(msg); + fprintf((FILE *)data, "%s\n", jv_string_value(msg)); + jv_free(msg); +} + jq_state *jq_init(void) { jq_state *jq; jq = jv_mem_alloc_unguarded(sizeof(*jq)); @@ -772,8 +817,8 @@ jq_state *jq_init(void) { jq->curr_frame = 0; jq->error = jv_null(); - jq->err_cb = NULL; - jq->err_cb_data = NULL; + jq->err_cb = default_err_cb; + jq->err_cb_data = stderr; jq->attrs = jv_object(); jq->path = jv_null(); @@ -781,6 +826,7 @@ jq_state *jq_init(void) { } void jq_set_error_cb(jq_state *jq, jq_err_cb cb, void *data) { + assert(cb != NULL); jq->err_cb = cb; jq->err_cb_data = data; } @@ -925,17 +971,8 @@ int jq_compile_args(jq_state *jq, const char* str, jv args) { nerrors = block_compile(program, &jq->bc); } } - if (nerrors) { - jv s = jv_string_fmt("%d compile %s", nerrors, - nerrors > 1 ? "errors" : "error"); - if (jq->err_cb != NULL) - jq->err_cb(jq->err_cb_data, s); - else if (!jv_is_valid(s)) - fprintf(stderr, "Error formatting jq compilation errors: %s\n", strerror(errno)); - else - fprintf(stderr, "%s\n", jv_string_value(s)); - jv_free(s); - } + if (nerrors) + jq_report_error(jq, jv_string_fmt("jq: %d compile %s", nerrors, nerrors > 1 ? "errors" : "error")); if (jq->bc) jq->bc = optimize(jq->bc); jv_free(args); diff --git a/jq.h b/jq.h index eaadb597..1849f661 100644 --- a/jq.h +++ b/jq.h @@ -13,6 +13,8 @@ jq_state *jq_init(void); void jq_set_error_cb(jq_state *, jq_err_cb, void *); void jq_get_error_cb(jq_state *, jq_err_cb *, void **); void jq_set_nomem_handler(jq_state *, void (*)(void *), void *); +jv jq_format_error(jv msg); +void jq_report_error(jq_state *, jv); int jq_compile(jq_state *, const char* str); int jq_compile_args(jq_state *, const char* str, jv args); void jq_dump_disassembly(jq_state *, int); diff --git a/linker.c b/linker.c index 959b373d..016870d5 100644 --- a/linker.c +++ b/linker.c @@ -46,7 +46,7 @@ jv build_lib_search_chain(jq_state *jq, jv lib_path) { out_paths = jv_array_append(out_paths, path); } else { jv emsg = jv_invalid_get_msg(path); - fprintf(stderr, "jq: warning: skipping search path: %s\n", jv_string_value(emsg)); + jq_report_error(jq, jv_string_fmt("jq: warning: skipping search path: %s\n",jv_string_value(emsg))); jv_free(emsg); } } @@ -108,7 +108,7 @@ static int process_dependencies(jq_state *jq, jv lib_origin, block *src_block, s jv lib_path = find_lib(jq, name, search); if (!jv_is_valid(lib_path)) { jv emsg = jv_invalid_get_msg(lib_path); - fprintf(stderr, "jq: error: %s\n",jv_string_value(emsg)); + jq_report_error(jq, jv_string_fmt("jq: error: %s\n",jv_string_value(emsg))); jv_free(emsg); jv_free(lib_origin); jv_free(as); diff --git a/locfile.c b/locfile.c index eaead6bb..1a60f42e 100644 --- a/locfile.c +++ b/locfile.c @@ -59,8 +59,6 @@ static int locfile_line_length(struct locfile* l, int line) { } void locfile_locate(struct locfile* l, location loc, const char* fmt, ...) { - jq_err_cb cb; - void *cb_data; va_list fmtargs; va_start(fmtargs, fmt); int startline; @@ -71,45 +69,20 @@ void locfile_locate(struct locfile* l, location loc, const char* fmt, ...) { offset = l->linemap[startline]; } - jq_get_error_cb(l->jq, &cb, &cb_data); - jv m1 = jv_string_vfmt(fmt, fmtargs); if (!jv_is_valid(m1)) { - jv_free(m1); - goto enomem; + jq_report_error(l->jq, m1); + return; } - jv m2; if (loc.start == -1) { - m2 = jv_string_fmt("%s\n", jv_string_value(m1)); - if (cb) - cb(cb_data, m2); - else - fprintf(stderr, "%s", jv_string_value(m2)); + jq_report_error(l->jq, jv_string_fmt("jq: error: %s\n", jv_string_value(m1))); jv_free(m1); - jv_free(m2); return; } - m2 = jv_string_fmt("%s\n%.*s%*s", jv_string_value(m1), - locfile_line_length(l, startline), l->data + offset, - loc.start - offset, ""); + jv m2 = jv_string_fmt("%s\n%.*s%*s", jv_string_value(m1), + locfile_line_length(l, startline), l->data + offset, + loc.start - offset, ""); jv_free(m1); - if (!jv_is_valid(m2)) { - jv_free(m2); - goto enomem; - } - if (cb) - cb(cb_data, m2); - else - fprintf(stderr, "%s", jv_string_value(m2)); - jv_free(m2); - return; - -enomem: - if (cb != NULL) - cb(cb_data, jv_invalid()); - else if (errno == ENOMEM || errno == 0) - fprintf(stderr, "Error formatting jq compilation error: %s", strerror(errno ? errno : ENOMEM)); - else - fprintf(stderr, "Error formatting jq compilation error: %s", strerror(errno)); + jq_report_error(l->jq, m2); return; } diff --git a/parser.y b/parser.y index 9f925948..c2b1b478 100644 --- a/parser.y +++ b/parser.y @@ -122,12 +122,12 @@ void yyerror(YYLTYPE* loc, block* answer, int* errors, (*errors)++; if (strstr(s, "unexpected")) { #ifdef WIN32 - locfile_locate(locations, *loc, "error: %s (Windows cmd shell quoting issues?)", s); + locfile_locate(locations, *loc, "jq: error: %s (Windows cmd shell quoting issues?)", s); #else - locfile_locate(locations, *loc, "error: %s (Unix shell quoting issues?)", s); + locfile_locate(locations, *loc, "jq: error: %s (Unix shell quoting issues?)", s); #endif } else { - locfile_locate(locations, *loc, "error: %s", s); + locfile_locate(locations, *loc, "jq: error: %s", s); } } @@ -726,7 +726,7 @@ int jq_parse_library(struct locfile* locations, block* answer) { int errs = jq_parse(locations, answer); if (errs) return errs; if (block_has_main(*answer)) { - locfile_locate(locations, UNKNOWN_LOCATION, "error: library should only have function definitions, not a main expression"); + locfile_locate(locations, UNKNOWN_LOCATION, "jq: error: library should only have function definitions, not a main expression"); return 1; } assert(block_has_only_binders_and_imports(*answer, OP_IS_CALL_PSEUDO)); diff --git a/tests/run b/tests/run index 923478c6..878128de 100755 --- a/tests/run +++ b/tests/run @@ -129,13 +129,13 @@ if [ -n "$VALGRIND" ] && ! grep 'ERROR SUMMARY: 0 errors from 0 contexts' $d/out cat $d/out exit 1 fi -if ! grep '^error: syntax error,' $d/out > /dev/null; then +if ! grep '^jq: error: syntax error,' $d/out > /dev/null; then echo "Module system not detecting syntax errors in modules correctly" 1>&2 exit 1 fi if $VALGRIND ./jq -ner -L $d '%::wat' > $d/out 2>&1 || - ! grep '^error: syntax error,' $d/out > /dev/null; then + ! grep '^jq: error: syntax error,' $d/out > /dev/null; then echo "Syntax errors not detected?" 1>&2 exit 1 fi -- cgit v1.2.3