Bean <[EMAIL PROTECTED]> writes:
Hi,
> I write a patch for parser related bug, it fix the following problems:
>
> 1, major modification for parse.y, now it should work better. For example,
>
> if echo aa; echo bb; then echo cc; fi
>
> it works properly now.
Nice :-)
> 2, Check for undefined variable. for example, if AA is not defined,
>
> echo $AA
>
> caused problem in the old version. Now it shows blank line.
Ah, good :)
> 3, This following commands:
>
> function aa { echo bb; }
> aa
>
> Shows
>
> bb
> error: unknown command `aa'
>
> Now it is ok.
:-)
> 4, If an command return error in grub.cfg, the following content is
> not parsed, for example
>
> test A=B
> menuentry aa {
> set root=(hd0,1)
> chainloader +1
> }
>
> The menu is not parsed as test A=B set errno to 1.
>
> This bug can be quite tricky, for example, i remember someone report
> that update-grub has problem as the menu is not showed at all, but in
> fact, the problem is caused by the font command.
How did you deal with this? It's hard to determine if errors should
mean the script is stopped or should continue.
> 5, echo module not included in command.lst
> yes, this is problem is very old, but nobody seems to be fixing it.
:-)
> --
> Bean
>
> 2007-12-31 Bean <[EMAIL PROTECTED]>
>
> * normal/execute.c (grub_script_exec_argument_to_string): Check for
> undefined variable.
> (grub_script_execute_cmdline): Reset grub_errno.
>
> * normal/main.c (read_config_file): Reset grub_errno.
I am not sure if you do not introduce new problems this way...?
> * normal/parse.y (script): Changed.
> (script_1): New.
> (delimiter): New.
> (command): Changed.
> (commands): Changed.
> (commandblock): Changed.
> (menuentry): Changed.
> (if): Changed.
Please describe what was changed.
>
> * conf/common.rmk (pkgdata_MODULES): Add echo.mod.
>
>
> diff --git a/conf/common.rmk b/conf/common.rmk
> index 4773f7d..994d560 100644
> --- a/conf/common.rmk
> +++ b/conf/common.rmk
> @@ -208,7 +208,7 @@ lvm_mod_LDFLAGS = $(COMMON_LDFLAGS)
> # Commands.
> pkglib_MODULES += hello.mod boot.mod terminal.mod ls.mod \
> cmp.mod cat.mod help.mod font.mod search.mod \
> - loopback.mod configfile.mod \
> + loopback.mod configfile.mod echo.mod \
> terminfo.mod test.mod blocklist.mod hexdump.mod
>
> # For hello.mod.
> diff --git a/normal/execute.c b/normal/execute.c
> index 4462ddd..ab0897c 100644
> --- a/normal/execute.c
> +++ b/normal/execute.c
> @@ -49,7 +49,8 @@ grub_script_execute_argument_to_string (struct
> grub_script_arg *arg)
> if (argi->type == 1)
> {
> val = grub_env_get (argi->str);
> - size += grub_strlen (val);
> + if (val)
> + size += grub_strlen (val);
> }
> else
> size += grub_strlen (argi->str);
> @@ -67,7 +68,8 @@ grub_script_execute_argument_to_string (struct
> grub_script_arg *arg)
> if (argi->type == 1)
> {
> val = grub_env_get (argi->str);
> - grub_strcat (chararg, val);
> + if (val)
> + grub_strcat (chararg, val);
> }
> else
> grub_strcat (chararg, argi->str);
> @@ -99,6 +101,9 @@ grub_script_execute_cmdline (struct grub_script_cmd *cmd)
> grubcmd = grub_command_find (cmdline->cmdname);
> if (! grubcmd)
> {
> + /* Ignore errors. */
> + grub_errno = GRUB_ERR_NONE;
> +
What are the implications of this?
> /* It's not a GRUB command, try all functions. */
> func = grub_script_function_find (cmdline->cmdname);
> if (! func)
> diff --git a/normal/main.c b/normal/main.c
> index ccea447..32e649c 100644
> --- a/normal/main.c
> +++ b/normal/main.c
> @@ -261,6 +261,9 @@ read_config_file (const char *config, int nested)
> /* Execute the command(s). */
> grub_script_execute (parsed_script);
>
> + /* Ignore errors. */
> + grub_errno = GRUB_ERR_NONE;
Same for this. Errors are not shown this way, for example.
> /* The parsed script was executed, throw it away. */
> grub_script_free (parsed_script);
> }
> diff --git a/normal/parser.y b/normal/parser.y
> index 19cd1bd..8771773 100644
> --- a/normal/parser.y
> +++ b/normal/parser.y
> @@ -43,7 +43,7 @@
> %token GRUB_PARSER_TOKEN_FI "fi"
> %token GRUB_PARSER_TOKEN_NAME
> %token GRUB_PARSER_TOKEN_VAR
> -%type <cmd> script grubcmd command commands commandblock menuentry if
> +%type <cmd> script script_1 grubcmd command commands commandblock menuentry
> if
> %type <arglist> arguments;
> %type <arg> argument;
> %type <string> "if" "while" "function" "else" "then" "fi"
> @@ -55,12 +55,22 @@
>
> %%
> /* It should be possible to do this in a clean way... */
> -script: { state->err = 0} newlines commands
> +script: { state->err = 0} script_1
> {
> - state->parsed = $3;
> + state->parsed = $2;
> }
> ;
>
> +script_1: commands { $$ = $1; }
> + | function '\n' { $$ = 0; }
> + | menuentry '\n' { $$ = $1; }
> +;
I do not like the name "script_1".
> +delimiter: '\n'
> + | ';'
> + | delimiter '\n'
> +;
ok
> newlines: /* Empty */
> | newlines '\n'
> ;
> @@ -124,39 +134,29 @@ grubcmd: GRUB_PARSER_TOKEN_NAME arguments
> ;
>
> /* A single command. */
> -command: grubcmd { $$ = $1; }
> - | if { $$ = $1; }
> - | function { $$ = 0; }
> - | menuentry { $$ = $1; }
> +command: grubcmd delimiter { $$ = $1; }
> + | if delimiter { $$ = $1; }
> + | commandblock delimiter { $$ = $1; }
> + | error delimiter
> + {
> + $$ = 0;
> + yyerror (state, "Incorrect command");
> + state->err = 1;
> + yyerrok;
> + }
> ;
Ok.
> /* A block of commands. */
> -commands: command '\n'
> - {
> - $$ = grub_script_add_cmd (state, 0, $1);
> - }
> - | command
> - {
> +commands: command
> + {
> $$ = grub_script_add_cmd (state, 0, $1);
> }
> - | command ';' commands
> - {
> - struct grub_script_cmdblock *cmd;
> - cmd = (struct grub_script_cmdblock *) $3;
> - $$ = grub_script_add_cmd (state, cmd, $1);
> - }
> - | command '\n' newlines commands
> - {
> + | command commands
> + {
> struct grub_script_cmdblock *cmd;
> - cmd = (struct grub_script_cmdblock *) $4;
> + cmd = (struct grub_script_cmdblock *) $2;
> $$ = grub_script_add_cmd (state, cmd, $1);
> }
> - | error
> - {
> - yyerror (state, "Incorrect command");
> - state->err = 1;
> - yyerrok;
> - }
> ;
Looks fine to me at first sight.
> /* A function. Carefully save the memory that is allocated. Don't
> @@ -194,7 +194,6 @@ function: "function" GRUB_PARSER_TOKEN_NAME
> commandblock: '{'
> {
> grub_script_lexer_ref (state->lexerstate);
> - grub_script_lexer_record_start (state->lexerstate);
Ehm? Can you explain this? If I am not mistaken, this will result in
a memory leak.
> }
> newlines commands '}'
> {
> @@ -204,10 +203,17 @@ commandblock: '{'
> ;
>
> /* A menu entry. Carefully save the memory that is allocated. */
> -menuentry: "menuentry" argument newlines commandblock
> +menuentry: "menuentry" argument
> + {
> + grub_script_lexer_ref (state->lexerstate);
> + } newlines '{'
> + {
> + grub_script_lexer_record_start (state->lexerstate);
> + } newlines commands '}'
What was the problem here?
> {
> char *menu_entry;
> menu_entry = grub_script_lexer_record_stop
> (state->lexerstate);
> + grub_script_lexer_deref (state->lexerstate);
> $$ = grub_script_create_cmdmenu (state, $2, menu_entry, 0);
> }
> ;
> @@ -218,14 +224,14 @@ if_statement: "if" { grub_script_lexer_ref
> (state->lexerstate); }
> ;
>
> /* The if statement. */
> -if: if_statement grubcmd ';' "then" commands "fi"
> +if: if_statement commands "then" newlines commands "fi"
> {
> $$ = grub_script_create_cmdif (state, $2, $5, 0);
> grub_script_lexer_deref (state->lexerstate);
> }
> - | if_statement grubcmd ';' "then" commands "else" commands
> "fi"
> + | if_statement commands "then" newlines commands "else"
> newlines commands "fi"
> {
> - $$ = grub_script_create_cmdif (state, $2, $5, $7);
> + $$ = 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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
