discover/grub2: improve handling of word & delimiter tokens

Currently, the delimiter token handling is a little fragile: we try to
ignore non-inter-word delimiters in the lexer with a selective set of
regexes on the possible delimiter characters.

This means we don't need to handle potential delimiters in every grammar
rule, but there are other situations (not regex-able) where we may see
delimters, and this will cause a parse error.

Instead of relying on the regex behaviour, we have an 'inter_word' flag,
which is set when we see the first word token, and cleared when we see
an end-of-line token. We only emit TOKEN_DELIM when this flag is set.

This means that we only get the delim tokens when they're required -
when we're looking for word separators (becuase WORD DELIM WORD is
distinct from WORD WORD - eg "linux /vmlinux" and "x$var").

We add a few new tests for the "menuentry" and "if" syntax, with
different delimiter configurations.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

diff --git a/discover/grub2/grub2-lexer.l b/discover/grub2/grub2-lexer.l
index 066af6073baa78012a8dd7d513696adb96eefaa9..52575e390713a3a6b1fe576a6fb0a76ae7abf0c0 100644 (file)
--- a/discover/grub2/grub2-lexer.l
+++ b/discover/grub2/grub2-lexer.l
@@ -22,14 +22,13 @@ void yyerror(struct grub2_parser *parser, const char *fmt, ...);
 %x dqstring
 
 WORD   [^{}|&$;<> \t\n'"#]+
+DELIM  [ \t]+
 VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
 
 %%
 
- /* discard leading & trailing whitespace, but keep inter-word delimeters */
-^[ \t]+        ;
-[ \t]+$        ;
-[ \t]+ return TOKEN_DELIM;
+ /* discard whitespace, unless we're looking for inter-word delimiters */
+{DELIM}             { if (yyget_extra(yyscanner)->inter_word) return TOKEN_DELIM; }
 
  /* reserved words */
 "[["         return TOKEN_LDSQBRACKET;
@@ -56,6 +55,7 @@ VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
  /* anything that's not a metachar: return as a plain word */
 {WORD} {
                yylval->word = create_word_text(yyget_extra(yyscanner), yytext);
+               yyget_extra(yyscanner)->inter_word = true;
                return TOKEN_WORD;
        }
 
@@ -68,6 +68,7 @@ VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
                yytext++;
                yylval->word = create_word_var(yyget_extra(yyscanner), yytext,
                                                true);
+               yyget_extra(yyscanner)->inter_word = true;
                return TOKEN_WORD;
        }
 
@@ -80,6 +81,7 @@ VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
        }
 <sqstring>[^']+ {
                yylval->word = create_word_text(yyget_extra(yyscanner), yytext);
+               yyget_extra(yyscanner)->inter_word = true;
                return TOKEN_WORD;
        }
 
@@ -92,6 +94,7 @@ VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
        }
 <dqstring>([^"\$]|\\\")+ {
                yylval->word = create_word_text(yyget_extra(yyscanner), yytext);
+               yyget_extra(yyscanner)->inter_word = true;
                return TOKEN_WORD;
        }
 <dqstring>\${VARNAME} |
@@ -103,17 +106,21 @@ VARNAME ([[:alpha:]][_[:alnum:]]*|[0-9]|[\?@\*#])
                yytext++;
                yylval->word = create_word_var(yyget_extra(yyscanner), yytext,
                                                false);
+               yyget_extra(yyscanner)->inter_word = true;
                return TOKEN_WORD;
        }
 
 
 
  /* blocks */
-"{"    return '{';
-"}"    return '}';
+"{"    { yyget_extra(yyscanner)->inter_word = false; return '{'; }
+"}"    { yyget_extra(yyscanner)->inter_word = false; return '}'; }
 
  /* end-of-line */
-[ \t]*(;|\n)[ \t]*     return TOKEN_EOL;
+[ \t]*(;|\n)[ \t]*     {
+               yyget_extra(yyscanner)->inter_word = false;
+               return TOKEN_EOL;
+       }
 
  /* strip comments */
 #.*    ;

diff --git a/discover/grub2/grub2-parser.y b/discover/grub2/grub2-parser.y
index 7f9961b7cf5a40a0c49eab1825ce6d9734dc5c70..598f0feb4d3b0c76cb68137b209743d05b519536 100644 (file)
--- a/discover/grub2/grub2-parser.y
+++ b/discover/grub2/grub2-parser.y
@@ -21,6 +21,11 @@ void yyerror(struct grub2_parser *parser, void *scanner, const char *fmt, ...);
        struct grub2_statements *statements;
 }
 
+%printer { fprintf(yyoutput, "%s%s:'%s'",
+               $$->type == GRUB2_WORD_VAR ? "var" : "text",
+               $$->type == GRUB2_WORD_VAR && !$$->split ? "[nosplit]" : "",
+               $$->name); } <word>
+
 /* reserved words */
 %token TOKEN_LDSQBRACKET       "[["
 %token TOKEN_RDSQBRACKET       "]]"
@@ -70,27 +75,21 @@ script:     statements {
                parser->script->statements = $1;
        }
 
-eol:   TOKEN_EOL | TOKEN_EOF;
-
-statements: /* empty */ {
+statements: statement {
                $$ = create_statements(parser);
+               statement_append($$, $1);
        }
-       | statements statement eol {
-               statement_append($1, $2);
-               $$ = $1;
-       }
-       | statements TOKEN_EOL {
+       | statements eol statement {
+               statement_append($1, $3);
                $$ = $1;
        }
 
-sep:   TOKEN_DELIM | TOKEN_EOL;
-
-conditional: statement TOKEN_EOL "then" sep statements {
-               $$ = create_statement_conditional(parser, $1, $5);
+conditional: statement eol "then" statements {
+               $$ = create_statement_conditional(parser, $1, $4);
        }
 
-elif: "elif" TOKEN_DELIM conditional {
-               $$ = $3;
+elif: "elif" conditional {
+               $$ = $2;
       }
 
 elifs: /* empty */ {
@@ -101,47 +100,49 @@ elifs: /* empty */ {
                $$ = $1;
        }
 
-statement:
-       words {
+statement: {
+                  $$ = NULL;
+       }
+       | words delim0 {
                   $$ = create_statement_simple(parser, $1);
        }
        | '{' statements '}' {
                $$ = create_statement_block(parser, $2);
        }
-       | "if" TOKEN_DELIM conditional elifs "fi" {
-               $$ = create_statement_if(parser, $3, $4, NULL);
+       | "if" conditional elifs "fi" {
+               $$ = create_statement_if(parser, $2, $3, NULL);
        }
-       | "if" TOKEN_DELIM conditional
+       | "if" conditional
                elifs
-               "else" sep
+               "else"
                statements
                "fi" {
-               $$ = create_statement_if(parser, $3, $4, $7);
+               $$ = create_statement_if(parser, $2, $3, $5);
        }
-       | "function" TOKEN_DELIM word TOKEN_DELIM '{' statements '}' {
-               $$ = create_statement_function(parser, $3, $6);
+       | "function" word delim '{' statements '}' {
+               $$ = create_statement_function(parser, $2, $5);
        }
-       | "menuentry" TOKEN_DELIM words TOKEN_DELIM
+       | "menuentry" words delim
                '{' statements '}' {
-               $$ = create_statement_menuentry(parser, $3, $6);
+               $$ = create_statement_menuentry(parser, $2, $5);
        }
-       | "submenu" TOKEN_DELIM words TOKEN_DELIM
+       | "submenu" words delim
                '{' statements '}' {
                /* we just flatten everything */
-               $$ = create_statement_block(parser, $6);
+               $$ = create_statement_block(parser, $5);
        }
-       | "for" TOKEN_DELIM word TOKEN_DELIM "in" TOKEN_DELIM words TOKEN_EOL
-               "do" sep
+       | "for" word delim "in" delim words eol
+               "do"
                statements
                "done" {
-               $$ = create_statement_for(parser, $3, $7, $11);
+               $$ = create_statement_for(parser, $2, $6, $9);
        }
 
 words: word {
                $$ = create_argv(parser);
                argv_append($$, $1);
        }
-       | words TOKEN_DELIM word {
+       | words delim word {
                argv_append($1, $3);
                $$ = $1;
        }
@@ -152,6 +153,13 @@ word:      TOKEN_WORD
                $$ = $1;
        }
 
+delim0:        /* empty */ |
+       delim
+
+delim: TOKEN_DELIM |
+       delim TOKEN_DELIM
+
+eol:   TOKEN_EOL;
 %%
 void yyerror(struct grub2_parser *parser, void *scanner, const char *fmt, ...)
 {
@@ -265,9 +273,8 @@ struct grub2_statement *create_statement_for(struct grub2_parser *parser,
 void statement_append(struct grub2_statements *stmts,
                struct grub2_statement *stmt)
 {
-       if (!stmt)
-               return;
-       list_add_tail(&stmts->list, &stmt->list);
+       if (stmt)
+               list_add_tail(&stmts->list, &stmt->list);
 }
 
 struct grub2_word *create_word_text(struct grub2_parser *parser,
@@ -319,6 +326,7 @@ struct grub2_parser *grub2_parser_create(struct discover_context *ctx)
        parser = talloc(ctx, struct grub2_parser);
        yylex_init_extra(parser, &parser->scanner);
        parser->script = create_script(parser, ctx);
+       parser->inter_word = false;
 
        return parser;
 }

diff --git a/discover/grub2/grub2.h b/discover/grub2/grub2.h
index e79bf41d4b655598a3019394a0c2f5ccbbac0ca3..0a8932426588b1a8e90b5f575a6ffcc542e94eba 100644 (file)
--- a/discover/grub2/grub2.h
+++ b/discover/grub2/grub2.h
@@ -103,6 +103,7 @@ struct grub2_script {
 struct grub2_parser {
        void                    *scanner;
        struct grub2_script     *script;
+       bool                    inter_word;
 };
 
 /* type for builtin functions */

diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am
index eca4c0a6df9ee61368aa19dcb3b6ae250b20d1dc..072eba6fa9bfc56ec268de5faf32cd741f9261b2 100644 (file)
--- a/test/parser/Makefile.am
+++ b/test/parser/Makefile.am
@@ -16,6 +16,9 @@ parser_TESTS = \
        test/parser/test-grub2-single \
        test/parser/test-grub2-default \
        test/parser/test-grub2-empty \
+       test/parser/test-grub2-noeol \
+       test/parser/test-grub2-menuentry-formats \
+       test/parser/test-grub2-if-formats \
        test/parser/test-grub2-default-index \
        test/parser/test-grub2-default-multiword \
        test/parser/test-grub2-multiple-resolve \

diff --git a/test/parser/test-grub2-if-formats.c b/test/parser/test-grub2-if-formats.c
new file mode 100644 (file)
index 0000000..cbf1afb
--- /dev/null
+++ b/test/parser/test-grub2-if-formats.c
@@ -0,0 +1,40 @@
+
+#include "parser-test.h"
+
+#if 0 /* PARSER_EMBEDDED_CONFIG */
+if true;then t1=1;fi
+if true ;then t2=2;fi
+if true;then t3=3 ;fi
+if true;then t4=4; ;fi
+if true
+then t5=5
+fi
+if true
+then t6=6;
+fi
+if true
+ then t7=7
+fi
+if true
+then t8=8; fi
+if true
+then
+t9=9
+
+fi
+
+menuentry $t1$t2$t3$t4$t5$t6$t7$t8$t9 {linux /vmlinux}
+#endif
+
+void run_test(struct parser_test *test)
+{
+       struct discover_boot_option *opt;
+
+       test_read_conf_embedded(test, "/grub2/grub.cfg");
+
+       test_run_parser(test, "grub2");
+
+       check_boot_option_count(test->ctx, 1);
+       opt = get_boot_option(test->ctx, 0);
+       check_name(opt, "123456789");
+}

diff --git a/test/parser/test-grub2-menuentry-formats.c b/test/parser/test-grub2-menuentry-formats.c
new file mode 100644 (file)
index 0000000..132ce8d
--- /dev/null
+++ b/test/parser/test-grub2-menuentry-formats.c
@@ -0,0 +1,41 @@
+
+#include "parser-test.h"
+
+#if 0 /* PARSER_EMBEDDED_CONFIG */
+menuentry "test.0" {linux /vmlinux;}
+menuentry "test.1" {linux /vmlinux}
+menuentry "test.2" {linux /vmlinux }
+menuentry "test.3" { linux /vmlinux; }
+menuentry "test.4" {linux /vmlinux ;}
+menuentry "test.5" {
+linux /vmlinux;}
+menuentry "test.6" {linux /vmlinux
+}
+menuentry "test.7" {
+linux /vmlinux
+}
+menuentry "test.8" {
+ linux /vmlinux
+}
+menuentry "test.9" {
+ linux /vmlinux
+ }
+#endif
+
+void run_test(struct parser_test *test)
+{
+       struct discover_boot_option *opt;
+       char str[] = "test.0";
+       int i;
+
+       test_read_conf_embedded(test, "/grub2/grub.cfg");
+
+       test_run_parser(test, "grub2");
+
+       check_boot_option_count(test->ctx, 10);
+       for (i = 0; i < 8; i++) {
+               opt = get_boot_option(test->ctx, i);
+               str[5] = i + '0';
+               check_name(opt, str);
+       }
+}

diff --git a/test/parser/test-grub2-noeol.c b/test/parser/test-grub2-noeol.c
new file mode 100644 (file)
index 0000000..e2db85e
--- /dev/null
+++ b/test/parser/test-grub2-noeol.c
@@ -0,0 +1,15 @@
+
+#include "parser-test.h"
+
+void run_test(struct parser_test *test)
+{
+       const char data[] = "true";
+
+       test_add_file_data(test, test->ctx->device, "/boot/grub/grubenv",
+                       data, sizeof(data));
+
+       __test_read_conf_data(test, test->ctx->device,
+                       "/boot/grub/grub.cfg", data, sizeof(data));
+
+       test_run_parser(test, "grub2");
+}