On Fri, Dec 4, 2009 at 9:42 PM, Robert Millan <r...@aybabtu.com> wrote:
> On Fri, Dec 04, 2009 at 07:12:12AM +0000, BVK wrote:
>>
>> Attached patch (experimental) adds a new tool, grub-script-check, that
>> can be used to check grub.cfg file syntax.  Idea is to use this tool
>> during update-grub to catch any syntax errors and reject the update.
>
> Excellent idea.


Thank you. This was requested by Felix, so credit goes to him too :-)


>
>> As part of this tool, I found few bugs in parser.y script and is
>> updated as necessary.
>
> Could you send this as a separate patch?
>

Attached patch is only for parser code, that is much richer and has
fixes for few more parsing cases.  For example, old parser doesn't
accept commands like, "echo if" because 'if' is a keyword, etc.

There are few cases, where this parser fails because handcoded lexer
is sending wrong inputs (eg: try "function foo { true; }" in single
line); I am thinking of replacing it with flex generated lexer and I
will try to fix these cases there.  Is that okay?

As a side note, since minor changes in grammar rules can have
unexpected side effects, I suggest to commit this only after having
proper unit tests.  Since we now have a syntax checker tool
(grub-script-check) that can be run at make-check-time, I am planning
to write some scripted testcases (based on unit-testing framework
patches I sent earlier), that would ensure parser changes do not break
already working grub-scripts.

Let me know your comments.




thanks
-- 
bvk.chaitanya
=== modified file 'script/parser.y'
--- old/script/parser.y 2009-11-23 15:37:33 +0000
+++ new/script/parser.y 2009-12-06 12:20:01 +0000
@@ -21,10 +21,10 @@
 #include <grub/script_sh.h>
 #include <grub/mm.h>
 
-#define YYFREE         grub_free
-#define YYMALLOC       grub_malloc
+#define YYFREE          grub_free
+#define YYMALLOC        grub_malloc
 #define YYLTYPE_IS_TRIVIAL      0
-#define YYENABLE_NLS   0
+#define YYENABLE_NLS    0
 
 %}
 
@@ -35,163 +35,185 @@
   char *string;
 }
 
-%token GRUB_PARSER_TOKEN_IF            "if"
-%token GRUB_PARSER_TOKEN_WHILE         "while"
-%token GRUB_PARSER_TOKEN_FUNCTION      "function"
-%token GRUB_PARSER_TOKEN_MENUENTRY     "menuentry"
-%token GRUB_PARSER_TOKEN_ELSE          "else"
-%token GRUB_PARSER_TOKEN_THEN          "then"
-%token GRUB_PARSER_TOKEN_FI            "fi"
+%token GRUB_PARSER_TOKEN_IF             "if"
+%token GRUB_PARSER_TOKEN_WHILE          "while"
+%token GRUB_PARSER_TOKEN_FUNCTION       "function"
+%token GRUB_PARSER_TOKEN_MENUENTRY      "menuentry"
+%token GRUB_PARSER_TOKEN_ELSE           "else"
+%token GRUB_PARSER_TOKEN_THEN           "then"
+%token GRUB_PARSER_TOKEN_FI             "fi"
+%token GRUB_PARSER_TOKEN_EOF            0
 %token GRUB_PARSER_TOKEN_ARG
-%type <cmd> script_init script grubcmd command commands commandblock menuentry 
if
-%type <arglist> arguments;
-%type <arg> GRUB_PARSER_TOKEN_ARG;
+
+%type <arg> GRUB_PARSER_TOKEN_IF
+%type <arg> GRUB_PARSER_TOKEN_WHILE
+%type <arg> GRUB_PARSER_TOKEN_FUNCTION
+%type <arg> GRUB_PARSER_TOKEN_MENUENTRY
+%type <arg> GRUB_PARSER_TOKEN_ELSE
+%type <arg> GRUB_PARSER_TOKEN_THEN
+%type <arg> GRUB_PARSER_TOKEN_FI
+%type <arg> GRUB_PARSER_TOKEN_ARG
+
+%type <arglist> argument arguments0 arguments1
+
+%type <cmd> script_init script grubcmd ifcmd command
+%type <cmd> commands1 menuentry statement
 
 %pure-parser
-%lex-param { struct grub_parser_param *state };
+%error-verbose
+
+%lex-param   { struct grub_parser_param *state };
 %parse-param { struct grub_parser_param *state };
 
+%start script_init
+
 %%
 /* It should be possible to do this in a clean way...  */
-script_init:   { state->err = 0; } script
-                 {
-                   state->parsed = $2;
-                 }
-;
-
-script:                { $$ = 0; }
-                | '\n' { $$ = 0; }
-                | commands { $$ = $1; }
-               | function '\n' { $$ = 0; }
-               | menuentry '\n' { $$ = $1; }
-               | error
-                 {
-                   $$ = 0;
-                   yyerror (state, "Incorrect command");
-                   state->err = 1;
-                   yyerrok;
-                 }
-;
-
-delimiter:     '\n'
-               | ';'
-               | delimiter '\n'
-;
-
-newlines:      /* Empty */
-               | newlines '\n'
-;
-
-
-
-arguments:     GRUB_PARSER_TOKEN_ARG
-                 {
-                   $$ = grub_script_add_arglist (state, 0, $1);
-                 }
-               | arguments GRUB_PARSER_TOKEN_ARG
-                 {
-                   $$ = grub_script_add_arglist (state, $1, $2);
-                 }
-;
-
-grubcmd:       arguments
-                 {
-                   $$ = grub_script_create_cmdline (state, $1);
-                 }
+script_init: { state->err = 0; } script { state->parsed = $2; }
+;
+
+script: /* Empty */
+        {
+          $$ = 0;
+        }
+      | '\n' script
+        {
+          $$ = 0;
+        }
+      | statement delimiter script
+        {
+          struct grub_script_cmdblock *cmd;
+          cmd = (struct grub_script_cmdblock *) $3;
+          $$ = grub_script_add_cmd (state, cmd, $1);
+        }
+      | error
+        {
+          $$ = 0;
+          yyerror (state, "Incorrect command");
+          yyerrok;
+        }
+;
+
+newlines0: /* Empty */ | newlines1 ;
+newlines1: newlines0 '\n' ;
+delimiter: ';' | '\n' ;
+delimiters: delimiter | delimiters '\n' ;
+
+statement: command   { $$ = $1; }
+         | function  { $$ = 0;  }
+         | menuentry { $$ = $1; }
+;
+
+argument: GRUB_PARSER_TOKEN_ARG       { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_IF        { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_ELSE      { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_THEN      { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_FI        { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_WHILE     { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_FUNCTION  { $$ = grub_script_add_arglist (state, 
0, $1); }
+        | GRUB_PARSER_TOKEN_MENUENTRY { $$ = grub_script_add_arglist (state, 
0, $1); }
+;
+
+arguments0: /* Empty */ { $$ = 0; }
+          | arguments1  { $$ = $1; }
+;
+arguments1: argument arguments0
+            {
+              if ($2)
+                {
+                  $1->next = $2;
+                  $1->argcount += $2->argcount;
+                  $2->argcount = 0;
+                }
+              $$ = $1;
+            }      
+;
+
+grubcmd: GRUB_PARSER_TOKEN_ARG arguments0
+         {
+           struct grub_script_arglist *args;
+           args = grub_script_add_arglist (state, 0, $1);
+           if ($2) {
+             args->next = $2;
+             args->argcount += $2->argcount;
+             $2->argcount = 0;
+           }
+           $$ = grub_script_create_cmdline (state, args);
+         }
 ;
 
 /* A single command.  */
-command:       grubcmd delimiter { $$ = $1; }
-               | if delimiter  { $$ = $1; }
-               | commandblock delimiter { $$ = $1; }
-;
-
-/* A block of commands.  */
-commands:      command
-                 {
-                   $$ = grub_script_add_cmd (state, 0, $1);
-                 }
-               | command commands
-                 {
-                   struct grub_script_cmdblock *cmd;
-                   cmd = (struct grub_script_cmdblock *) $2;
-                   $$ = grub_script_add_cmd (state, cmd, $1);
-                 }
-;
-
-/* A function.  Carefully save the memory that is allocated.  Don't
-   change any stuff because it might seem like a fun thing to do!
-   Special care was take to make sure the mid-rule actions are
-   executed on the right moment.  So the `commands' rule should be
-   recognized after executing the `grub_script_mem_record; and before
-   `grub_script_mem_record_stop'.  */
-function:      "function" GRUB_PARSER_TOKEN_ARG
-                 {
-                   grub_script_lexer_ref (state->lexerstate);
-                 } newlines '{'
-                 {
-                   /* The first part of the function was recognized.
-                      Now start recording the memory usage to store
-                      this function.  */
-                   state->func_mem = grub_script_mem_record (state);
-                 } newlines commands '}'
-                 {
-                   struct grub_script *script;
-
-                   /* All the memory usage for parsing this function
-                      was recorded.  */
-                   state->func_mem = grub_script_mem_record_stop (state,
-                                                                  
state->func_mem);
-                   script = grub_script_create ($8, state->func_mem);
-                   if (script)
-                     grub_script_function_create ($2, script);
-                   grub_script_lexer_deref (state->lexerstate);
-                 }
-;
-
-/* Carefully designed, together with `menuentry' so everything happens
-   just in the expected order.  */
-commandblock:  '{'
-                 {
-                   grub_script_lexer_ref (state->lexerstate);
-                 }
-                newlines commands '}'
-                  {
-                   grub_script_lexer_deref (state->lexerstate);
-                   $$ = $4;
-                 }
-;
-
-/* A menu entry.  Carefully save the memory that is allocated.  */
-menuentry:     "menuentry"
-                 {
-                   grub_script_lexer_ref (state->lexerstate);
-                 } arguments newlines '{'
-                 {
-                   grub_script_lexer_record_start (state->lexerstate);
-                 } newlines commands '}'
-                 {
-                   char *menu_entry;
-                   menu_entry = grub_script_lexer_record_stop 
(state->lexerstate);
-                   grub_script_lexer_deref (state->lexerstate);
-                   $$ = grub_script_create_cmdmenu (state, $3, menu_entry, 0);
-                 }
-;
-
-/* The first part of the if statement.  It's used to switch the lexer
-   to a state in which it demands more tokens.  */
-if_statement:  "if" { grub_script_lexer_ref (state->lexerstate); }
-;
-
-/* The if statement.  */
-if:             if_statement commands "then" newlines commands "fi"
-                 {
-                   $$ = grub_script_create_cmdif (state, $2, $5, 0);
-                   grub_script_lexer_deref (state->lexerstate);
-                 }
-                | if_statement commands "then" newlines commands "else" 
newlines commands  "fi"
-                 {
-                   $$ = grub_script_create_cmdif (state, $2, $5, $8);
-                   grub_script_lexer_deref (state->lexerstate);
-                 }
+command: grubcmd { $$ = $1; }
+       | ifcmd   { $$ = $1; }
+;
+
+/* A list of commands. */
+commands1: newlines0 command
+           {
+             $$ = $2;
+           }
+         | commands1 delimiters command
+           {
+             struct grub_script_cmdblock *cmd;
+             cmd = (struct grub_script_cmdblock *) $3;
+             $$ = grub_script_add_cmd (state, cmd, $1);
+           }
+;
+
+function: "function" GRUB_PARSER_TOKEN_ARG
+          {
+            grub_script_lexer_ref (state->lexerstate);
+            state->func_mem = grub_script_mem_record (state);
+          }
+          '{' commands1 delimiters '}'
+          {
+            struct grub_script *script;
+            state->func_mem = grub_script_mem_record_stop (state,
+                                                           state->func_mem);
+            script = grub_script_create ($5, state->func_mem);
+            if (script)
+              grub_script_function_create ($2, script);
+
+            grub_script_lexer_deref (state->lexerstate);
+          }
+;
+
+menuentry: "menuentry" GRUB_PARSER_TOKEN_ARG
+           {
+             grub_script_lexer_ref (state->lexerstate);
+           }
+           arguments0
+           {
+             grub_script_lexer_record_start (state->lexerstate);
+           }
+           '{' commands1 delimiters '}'
+           {
+             char *menu_entry;
+             struct grub_script_arglist *args;
+
+             menu_entry = grub_script_lexer_record_stop (state->lexerstate);
+             grub_script_lexer_deref (state->lexerstate);
+             
+             args = grub_script_add_arglist (state, 0, $1);
+             if ($4) {
+               args->next = $4;
+               args->argcount += $4->argcount;
+               $4->argcount = 0;
+             }
+             $$ = grub_script_create_cmdmenu (state, args, menu_entry, 0);
+           }
+;
+
+if: "if" { grub_script_lexer_ref (state->lexerstate); }
+;
+ifcmd: if commands1 delimiters "then" commands1 delimiters "fi"
+       {
+         $$ = grub_script_create_cmdif (state, $2, $5, 0);
+         grub_script_lexer_deref (state->lexerstate);
+       }
+     | if commands1 delimiters "then" commands1 delimiters "else" commands1 
delimiters "fi"
+       {
+         $$ = grub_script_create_cmdif (state, $2, $5, $8);
+         grub_script_lexer_deref (state->lexerstate);
+       }
 ;

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to