diff --git a/src/ef-args.c b/src/ef-args.c index d42bae6..d55b90e 100644 --- a/src/ef-args.c +++ b/src/ef-args.c @@ -32,8 +32,13 @@ int argc_frame(int argc, const char *argv[], frame_t *f) { } if (!h) { - //po("ERROR: Invalid parameter: %s\n", argv[i]); - //return -1; + // Only overwrite if hdr_parse_fields didn't already set context + if (!PARSE_ERR_CTX.token) { + PARSE_ERR_CTX.token = argv[i]; + PARSE_ERR_CTX.hdr_name = NULL; + PARSE_ERR_CTX.fields = NULL; + PARSE_ERR_CTX.fields_size = 0; + } return i; } @@ -264,12 +269,83 @@ int argc_cmd(int argc, const char *argv[], cmd_t *c) { return i; } +static int is_known_hdr(const char *name) +{ + int j; + + for (j = 0; j < HDR_TMPL_SIZE; j++) + if (hdr_tmpls[j] && hdr_tmpls[j]->name && + strcmp(name, hdr_tmpls[j]->name) == 0) + return 1; + return 0; +} + +static void print_parse_error(int argc, const char *argv[], int err_idx) +{ + int j, col; + const char *tok = PARSE_ERR_CTX.token ? PARSE_ERR_CTX.token : argv[err_idx]; + + // Find the actual argv position of the error token. PARSE_ERR_CTX.token + // is a pointer into a sub-array of argv, so pointer comparison works. + int tok_idx = err_idx; + if (PARSE_ERR_CTX.token) { + for (j = 0; j < argc; j++) { + if (argv[j] == PARSE_ERR_CTX.token) { + tok_idx = j; + break; + } + } + } + + // Error headline + if (PARSE_ERR_CTX.hdr_name && !is_known_hdr(tok)) { + pe("error: '%s' is not a field of '%s' or a recognized header\n", + tok, PARSE_ERR_CTX.hdr_name); + } else if (PARSE_ERR_CTX.hdr_name) { + pe("error: '%s' is not a field of '%s'\n", + tok, PARSE_ERR_CTX.hdr_name); + } else if (PARSE_ERR_CTX.token) { + pe("error: '%s' is not a recognized header or command\n", tok); + } else { + pe("error: unexpected token '%s'\n", tok); + } + + // Show the full command with a caret pointing to the bad token + pe(" "); + for (j = 0; j < argc; j++) + pe("%s ", argv[j]); + pe("\n"); + + // Compute column offset of the error token + col = 2; // leading " " + for (j = 0; j < tok_idx; j++) + col += strlen(argv[j]) + 1; + + pe("%*s", col, ""); + for (j = 0; j < (int)strlen(tok); j++) + pe("^"); + pe("\n"); + + // Hint: list available fields + if (PARSE_ERR_CTX.hdr_name && PARSE_ERR_CTX.fields) { + pe(" valid fields for '%s':", PARSE_ERR_CTX.hdr_name); + for (j = 0; j < PARSE_ERR_CTX.fields_size; j++) { + if (PARSE_ERR_CTX.fields[j].bit_width == 0) + continue; + pe(" %s", PARSE_ERR_CTX.fields[j].name); + } + pe("\n"); + } +} + int argc_cmds(int argc, const char *argv[]) { struct timeval tv_now, tv_left, tv_begin, tv_end; int res, i = 0, cmd_idx = 0; cmd_t cmds[100] = {}; + memset(&PARSE_ERR_CTX, 0, sizeof(PARSE_ERR_CTX)); + while (i < argc && cmd_idx < 100) { //po("%d, cmd[%d]\n", __LINE__, cmd_idx); res = argc_cmd(argc - i, argv + i, &cmds[cmd_idx]); @@ -288,8 +364,8 @@ int argc_cmds(int argc, const char *argv[]) { } if (i != argc) { - po("Parse error! arg# %d out of %d, cmd_idx = %d\n", i, argc, cmd_idx); - return -1; + print_parse_error(argc, argv, i); + goto err; } capture_all_start(); @@ -318,9 +394,17 @@ int argc_cmds(int argc, const char *argv[]) { } return res; + +err: + for (i = 0; i < cmd_idx; ++i) { + cmd_destruct(&cmds[i]); + } + + return -1; } int TIME_OUT_MS = 100; +parse_err_ctx_t PARSE_ERR_CTX; int main_(int argc, const char *argv[]) { int opt; diff --git a/src/ef.c b/src/ef.c index 5a1d5d9..17b9160 100644 --- a/src/ef.c +++ b/src/ef.c @@ -348,8 +348,13 @@ int hdr_parse_fields(frame_t *frame, struct hdr *hdr, int offset, f = find_field(hdr, argv[i]); - if (!f) + if (!f) { + PARSE_ERR_CTX.token = argv[i]; + PARSE_ERR_CTX.hdr_name = hdr->name; + PARSE_ERR_CTX.fields = hdr->fields; + PARSE_ERR_CTX.fields_size = hdr->fields_size; return i; + } if (field_ignore) { field_ignore = 0; diff --git a/src/ef.h b/src/ef.h index 10a3863..808c420 100644 --- a/src/ef.h +++ b/src/ef.h @@ -130,6 +130,15 @@ int field_copy(field_t *dst, const field_t *src); void field_destruct(field_t *f); GEN_ALLOC_CLONE_FREE(field); +typedef struct { + const char *token; // token that didn't match + const char *hdr_name; // header being parsed (NULL if at header level) + int fields_size; // number of fields in header + field_t *fields; // pointer to header's field array +} parse_err_ctx_t; + +extern parse_err_ctx_t PARSE_ERR_CTX; + typedef struct hdr { const char *name; const char *help; diff --git a/test/parser-tests.rb b/test/parser-tests.rb index 95e315f..1943312 100755 --- a/test/parser-tests.rb +++ b/test/parser-tests.rb @@ -1,5 +1,27 @@ #!/usr/bin/env ruby +require 'open3' + +# Verify that invalid input produces the expected error on stderr and a +# non-zero exit code. +patterns+ is an array of strings that must all +# appear in the stderr output (order-independent, substring match). +def err cmd, *patterns + out, err, status = Open3.capture3("./ef #{cmd}") + + if status.success? + raise "Command './ef hex #{cmd}' exited with 0, expected non-zero" + end + + patterns.each do |pat| + unless err.include?(pat) + puts "STDERR: #{err}" + raise "Expected stderr to contain '#{pat}' for: ./ef hex #{cmd}" + end + end + + puts "OK (err): #{cmd}" +end + def ok cmd, expect actual = %x[./ef hex #{cmd}] actual.chomp! @@ -287,3 +309,30 @@ def ok_mask cmd, expect, expect_mask #ok("coap-opt num 4", # "000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") +# Parse error tests, verify helpful error messages on stderr +# Typo in field name: "vud" is not a field of "ctag" +err("hex eth smac 0xdead ctag vud 10 pcp 3", + "is not a field of 'ctag'", + "vud", + "valid fields for 'ctag'") + +# Unknown header after a valid header +err("hex eth foo", + "is not a field of 'eth' or a recognized header", + "foo") + +# Completely unknown first token (at command level) +err("hex bogus eth", + "bogus") + +# name command: error token is further in (caret must not point at "name") +err("name f veth smac 0xdead", + "is not a recognized header or command", + "veth") + +# Typo in field name of a second header in the frame +err("hex eth smac 1 ipv4 sop 1.2.3.4", + "is not a field of 'ipv4'", + "sop", + "valid fields for 'ipv4'") +