From 3cb909fa62b0e57df26f9ae7bc0174e5751e7c12 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Tue, 20 Nov 2012 17:09:03 +0100 Subject: [PATCH] config parser: recover after invalid input This is done by ignoring the rest of the current line and jumping to the nearest token. fixes #879 --- generate-command-parser.pl | 1 - parser-specs/config.spec | 3 ++ src/commands_parser.c | 1 + src/config_parser.c | 57 +++++++++++++++++++++++++++++++-- testcases/t/201-config-parser.t | 42 ++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 4 deletions(-) diff --git a/generate-command-parser.pl b/generate-command-parser.pl index 66e44b6c..5cdebf34 100755 --- a/generate-command-parser.pl +++ b/generate-command-parser.pl @@ -193,7 +193,6 @@ say $callfh ' default:'; say $callfh ' printf("BUG in the parser. state = %d\n", call_identifier);'; say $callfh ' assert(false);'; say $callfh ' }'; -say $callfh ' state = result->next_state;'; say $callfh '}'; close($callfh); diff --git a/parser-specs/config.spec b/parser-specs/config.spec index 1c11bf9d..8622c62e 100644 --- a/parser-specs/config.spec +++ b/parser-specs/config.spec @@ -15,6 +15,7 @@ state INITIAL: # We have an end token here for all the commands which just call some # function without using an explicit 'end' token. end -> + error -> '#' -> IGNORE_LINE 'set' -> IGNORE_LINE bindtype = 'bindsym', 'bindcode', 'bind' -> BINDING @@ -298,6 +299,7 @@ state MODEBRACE: state MODE: end -> + error -> '#' -> MODE_IGNORE_LINE 'set' -> MODE_IGNORE_LINE bindtype = 'bindsym', 'bindcode', 'bind' @@ -336,6 +338,7 @@ state BARBRACE: state BAR: end -> + error -> '#' -> BAR_IGNORE_LINE 'set' -> BAR_IGNORE_LINE 'i3bar_command' -> BAR_BAR_COMMAND diff --git a/src/commands_parser.c b/src/commands_parser.c index 1720fd6f..93ee3889 100644 --- a/src/commands_parser.c +++ b/src/commands_parser.c @@ -190,6 +190,7 @@ static void next_state(const cmdp_token *token) { subcommand_output.json_gen = command_output.json_gen; subcommand_output.needs_tree_render = false; GENERATED_call(token->extra.call_identifier, &subcommand_output); + state = subcommand_output.next_state; /* If any subcommand requires a tree_render(), we need to make the * whole parser result request a tree_render(). */ if (subcommand_output.needs_tree_render) diff --git a/src/config_parser.c b/src/config_parser.c index 889179a9..6f96cc5f 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -21,6 +21,9 @@ * 3. config_parser recognizes \n and \r as 'end' token, while commands_parser * ignores them. * + * 4. config_parser skips the current line on invalid inputs and follows the + * nearest token. + * */ #include #include @@ -221,23 +224,46 @@ static Match current_match; static struct ConfigResult subcommand_output; static struct ConfigResult command_output; +/* A list which contains the states that lead to the current state, e.g. + * INITIAL, WORKSPACE_LAYOUT. + * When jumping back to INITIAL, statelist_idx will simply be set to 1 + * (likewise for other states, e.g. MODE or BAR). + * This list is used to process the nearest error token. */ +static cmdp_state statelist[10] = { INITIAL }; +/* NB: statelist_idx points to where the next entry will be inserted */ +static int statelist_idx = 1; + #include "GENERATED_config_call.h" static void next_state(const cmdp_token *token) { + cmdp_state _next_state = token->next_state; + //printf("token = name %s identifier %s\n", token->name, token->identifier); //printf("next_state = %d\n", token->next_state); if (token->next_state == __CALL) { subcommand_output.json_gen = command_output.json_gen; GENERATED_call(token->extra.call_identifier, &subcommand_output); + _next_state = subcommand_output.next_state; clear_stack(); - return; } - state = token->next_state; + state = _next_state; if (state == INITIAL) { clear_stack(); } + + /* See if we are jumping back to a state in which we were in previously + * (statelist contains INITIAL) and just move statelist_idx accordingly. */ + for (int i = 0; i < statelist_idx; i++) { + if (statelist[i] != _next_state) + continue; + statelist_idx = i+1; + return; + } + + /* Otherwise, the state is new and we add it to the list */ + statelist[statelist_idx++] = _next_state; } /* @@ -284,6 +310,7 @@ struct ConfigResult *parse_config(const char *input, struct context *context) { linecnt++; } state = INITIAL; + statelist_idx = 1; /* A YAJL JSON generator used for formatting replies. */ #if YAJL_MAJOR >= 2 @@ -450,6 +477,10 @@ struct ConfigResult *parse_config(const char *input, struct context *context) { tokenwalk += strlen(token->name + 1); *tokenwalk++ = '\''; } else { + /* Skip error tokens in error messages, they are used + * internally only and might confuse users. */ + if (strcmp(token->name, "error") == 0) + continue; /* Any other token is copied to the error message enclosed * with angle brackets. */ *tokenwalk++ = '<'; @@ -531,10 +562,30 @@ struct ConfigResult *parse_config(const char *input, struct context *context) { ystr(position); y(map_close); + /* Skip the rest of this line, but continue parsing. */ + while ((walk - input) <= len && *walk != '\n') + walk++; + free(position); free(errormessage); clear_stack(); - break; + + /* To figure out in which state to go (e.g. MODE or INITIAL), + * we find the nearest state which contains an token + * and follow that one. */ + bool error_token_found = false; + for (int i = statelist_idx-1; (i >= 0) && !error_token_found; i--) { + cmdp_token_ptr *errptr = &(tokens[statelist[i]]); + for (int j = 0; j < errptr->n; j++) { + if (strcmp(errptr->array[j].name, "error") != 0) + continue; + next_state(&(errptr->array[j])); + error_token_found = true; + break; + } + } + + assert(error_token_found); } } diff --git a/testcases/t/201-config-parser.t b/testcases/t/201-config-parser.t index a850b7e8..ab3d7314 100644 --- a/testcases/t/201-config-parser.t +++ b/testcases/t/201-config-parser.t @@ -362,6 +362,47 @@ is(parser_calls($config), $expected, 'colors ok'); +################################################################################ +# Verify that errors don’t harm subsequent valid statements +################################################################################ + +$config = <<'EOT'; +hide_edge_border both +client.focused #4c7899 #285577 #ffffff #2e9ef4 +EOT + +$expected = <<'EOT'; +ERROR: CONFIG: Expected one of these tokens: , '#', 'set', 'bindsym', 'bindcode', 'bind', 'bar', 'font', 'mode', 'floating_minimum_size', 'floating_maximum_size', 'floating_modifier', 'default_orientation', 'workspace_layout', 'new_window', 'new_float', 'hide_edge_borders', 'for_window', 'assign', 'focus_follows_mouse', 'force_focus_wrapping', 'force_xinerama', 'force-xinerama', 'workspace_auto_back_and_forth', 'fake_outputs', 'fake-outputs', 'force_display_urgency_hint', 'workspace', 'ipc_socket', 'ipc-socket', 'restart_state', 'popup_during_fullscreen', 'exec_always', 'exec', 'client.background', 'client.focused_inactive', 'client.focused', 'client.unfocused', 'client.urgent' +ERROR: CONFIG: (in file ) +ERROR: CONFIG: Line 1: hide_edge_border both +ERROR: CONFIG: ^^^^^^^^^^^^^^^^^^^^^ +ERROR: CONFIG: Line 2: client.focused #4c7899 #285577 #ffffff #2e9ef4 +cfg_color(client.focused, #4c7899, #285577, #ffffff, #2e9ef4) +EOT + +is(parser_calls($config), + $expected, + 'errors dont harm subsequent statements'); + +$config = <<'EOT'; +hide_edge_borders FOOBAR +client.focused #4c7899 #285577 #ffffff #2e9ef4 +EOT + +$expected = <<'EOT'; +ERROR: CONFIG: Expected one of these tokens: 'none', 'vertical', 'horizontal', 'both', '1', 'yes', 'true', 'on', 'enable', 'active' +ERROR: CONFIG: (in file ) +ERROR: CONFIG: Line 1: hide_edge_borders FOOBAR +ERROR: CONFIG: ^^^^^^ +ERROR: CONFIG: Line 2: client.focused #4c7899 #285577 #ffffff #2e9ef4 +cfg_color(client.focused, #4c7899, #285577, #ffffff, #2e9ef4) +EOT + +is(parser_calls($config), + $expected, + 'errors dont harm subsequent statements'); + + ################################################################################ # Error message with 2+2 lines of context ################################################################################ @@ -524,6 +565,7 @@ ERROR: CONFIG: Line 2: output LVDS-1 ERROR: CONFIG: Line 3: unknown qux ERROR: CONFIG: ^^^^^^^^^^^ ERROR: CONFIG: Line 4: } +cfg_bar_finish() EOT is(parser_calls($config),