Bean <[EMAIL PROTECTED]> writes:
> This patch fixes the following bugs:
>
> 1. Token parser
>
> echo aa"bb"cc
> old: aa"bb"
> new: aa"bb"cc
>
> set BB=1
> echo aa$BB
> old: aa 1
> new: aa1
>
> set BB=1
> echo ${BB}aa
> old: 1 aa
> new: 1aa
>
> set AA=1
> set BB=$AA
> echo $BB
> old: empty
> new: 1
>
> AA is not defined
> echo $AA
> old: trash
> new: empty
>
> 2. function
>
> function aa {
> echo $AA
> }
> old: infinite loop
> new: ok
>
> function bb {
> echo Hello
> }
> old: Unknown command `echo'
> new: ok
>
> BTW, the echo module needs to be enabled manually in conf/common.rmk.
>
> --
> Bean
> --
> Bean
The header of the changelog entry is missing.
> * kern/parser.c (state_transitions): Exit GRUB_PARSER_STATE_VARNAME
> state when "/" is encountered.
>
> * normal/execute.c (grub_script_argument_to_string): Output empty
> string when variable does not exist.
>
> * normal/execute.c (grub_script_execute_cmdline): Reset error
> number.
>
> * normal/lexer.c (grub_script_yylex2): Return arg structure when
> mixed text is detected.
Can you leave this part out of the patch for now? See the text below.
> * normal/parser.y : token GRUB_PARSER_TOKEN_VAR returns arg
> instead of text.
>
>
> Index: normal/execute.c
> ===================================================================
> RCS file: /sources/grub/grub2/normal/execute.c,v
> retrieving revision 1.4
> diff -u -r1.4 execute.c
> --- normal/execute.c 28 May 2006 21:58:34 -0000 1.4
> +++ normal/execute.c 25 Jun 2007 11:43:57 -0000
> @@ -50,7 +50,8 @@
> if (argi->type == 1)
> {
> val = grub_env_get (argi->str);
> - size += grub_strlen (val);
> + if (val)
> + size += grub_strlen (val);
ok
> }
> else
> size += grub_strlen (argi->str);
> @@ -68,7 +69,8 @@
> if (argi->type == 1)
> {
> val = grub_env_get (argi->str);
> - grub_strcat (chararg, val);
> + if (val)
> + grub_strcat (chararg, val);
ok
> }
> else
> grub_strcat (chararg, argi->str);
> @@ -100,6 +102,9 @@
> grubcmd = grub_command_find (cmdline->cmdname);
> if (! grubcmd)
> {
> + /* Reset error number */
> + grub_errno = 0;
> +
Please use GRUB_ERR_NONE. I know this isn't done consistently, but
this is what I prefer.
Why is this change needed and whih problem does it fix?
> /* It's not a GRUB command, try all functions. */
> func = grub_script_function_find (cmdline->cmdname);
> if (! func)
> Index: normal/lexer.c
> ===================================================================
> RCS file: /sources/grub/grub2/normal/lexer.c,v
> retrieving revision 1.6
> diff -u -r1.6 lexer.c
> --- normal/lexer.c 4 Jun 2006 15:56:55 -0000 1.6
> +++ normal/lexer.c 25 Jun 2007 11:43:58 -0000
> @@ -161,6 +161,8 @@
> char *buffer;
> char *bp;
> struct grub_lexer_param *state = parsestate->lexerstate;
> + struct grub_script_arg *arg = 0;
> + int done = 0;
>
> if (state->done)
> return 0;
> @@ -196,13 +198,14 @@
> }
> }
>
> +back:
> newstate = grub_parser_cmdline_state (state->state, *state->script, &use);
>
> /* Check if it is a text. */
> if (check_textstate (newstate))
> {
> /* In case the string is not quoted, this can be a one char
> - length symbol. */
> + length symbol. */
> if (newstate == GRUB_PARSER_STATE_TEXT)
> {
> switch (*state->script)
> @@ -213,7 +216,7 @@
> newstate = grub_parser_cmdline_state (state->state,
> *state->script, &use);
> if (! (state->state == GRUB_PARSER_STATE_TEXT
> - && *state->script == ' '))
> + && *state->script == ' '))
Why do you change indention here?
> {
> grub_dprintf ("scripting", "token=` '\n");
> return ' ';
> @@ -260,8 +263,6 @@
> if (newstate == GRUB_PARSER_STATE_TEXT
> && state->state != GRUB_PARSER_STATE_ESC)
> {
> - int breakout = 0;
> -
> switch (use)
> {
> case ' ':
> @@ -269,13 +270,13 @@
> case '}':
> case ';':
> case '\n':
> - breakout = 1;
> + done = 1;
> }
> - if (breakout)
> + if (done)
> break;
> - *(bp++) = use;
> }
> - else if (use)
> +
> + if (use)
> *(bp++) = use;
>
> state->state = newstate;
> @@ -285,27 +286,39 @@
> /* A string of text was read in. */
> *bp = '\0';
> grub_dprintf ("scripting", "token=`%s'\n", buffer);
> - yylval->string = buffer;
>
> - /* Detect some special tokens. */
> - if (! grub_strcmp (buffer, "while"))
> - return GRUB_PARSER_TOKEN_WHILE;
> - else if (! grub_strcmp (buffer, "if"))
> - return GRUB_PARSER_TOKEN_IF;
> - else if (! grub_strcmp (buffer, "function"))
> - return GRUB_PARSER_TOKEN_FUNCTION;
> - else if (! grub_strcmp (buffer, "menuentry"))
> - return GRUB_PARSER_TOKEN_MENUENTRY;
> - else if (! grub_strcmp (buffer, "@"))
> - return GRUB_PARSER_TOKEN_MENUENTRY;
> - else if (! grub_strcmp (buffer, "else"))
> - return GRUB_PARSER_TOKEN_ELSE;
> - else if (! grub_strcmp (buffer, "then"))
> - return GRUB_PARSER_TOKEN_THEN;
> - else if (! grub_strcmp (buffer, "fi"))
> - return GRUB_PARSER_TOKEN_FI;
> - else
> - return GRUB_PARSER_TOKEN_NAME;
> + if (!*state->script)
> + done = 1;
> +
> + if ((done) && (!arg))
> + {
> + yylval->string = buffer;
> +
> + /* Detect some special tokens. */
> + if (! grub_strcmp (buffer, "while"))
> + return GRUB_PARSER_TOKEN_WHILE;
> + else if (! grub_strcmp (buffer, "if"))
> + return GRUB_PARSER_TOKEN_IF;
> + else if (! grub_strcmp (buffer, "function"))
> + return GRUB_PARSER_TOKEN_FUNCTION;
> + else if (! grub_strcmp (buffer, "menuentry"))
> + return GRUB_PARSER_TOKEN_MENUENTRY;
> + else if (! grub_strcmp (buffer, "@"))
> + return GRUB_PARSER_TOKEN_MENUENTRY;
> + else if (! grub_strcmp (buffer, "else"))
> + return GRUB_PARSER_TOKEN_ELSE;
> + else if (! grub_strcmp (buffer, "then"))
> + return GRUB_PARSER_TOKEN_THEN;
> + else if (! grub_strcmp (buffer, "fi"))
> + return GRUB_PARSER_TOKEN_FI;
> + else
> + return GRUB_PARSER_TOKEN_NAME;
> + }
> +
> + if (bp != buffer)
> + arg =
> + grub_script_arg_add (parsestate, arg, GRUB_SCRIPT_ARG_TYPE_STR,
> + buffer);
You are still doing parsing in the lexer. I don't like that... If
you make a separate patch that excludes this, be can apply that first
before we work on this specific problem. I don't want to hold back
applying the fixes for these other bugs because of this.
> }
> else if (newstate == GRUB_PARSER_STATE_VAR
> || newstate == GRUB_PARSER_STATE_QVAR)
> @@ -341,19 +354,33 @@
> }
>
> *bp = '\0';
> - state->state = newstate;
> - yylval->string = buffer;
> - grub_dprintf ("scripting", "vartoken=`%s'\n", buffer);
> + if (newstate == GRUB_PARSER_STATE_VARNAME)
> + state->state = GRUB_PARSER_STATE_TEXT;
>
> - return GRUB_PARSER_TOKEN_VAR;
> + if (bp != buffer)
> + arg = grub_script_arg_add (parsestate, arg, GRUB_SCRIPT_ARG_TYPE_VAR,
> + buffer);
> }
> else
> {
> /* There is either text or a variable name. In the case you
> - arrive here there is a serious problem with the lexer. */
> + arrive here there is a serious problem with the lexer. */
> grub_error (GRUB_ERR_BAD_ARGUMENT, "Internal error\n");
> return 0;
> }
> +
> + if ((done) || (!*state->script))
> + {
> + if (arg)
> + {
> + yylval->arg = arg;
> + return GRUB_PARSER_TOKEN_VAR;
> + }
> + else
> + return ' ';
> + }
> +
> + goto back;
> }
>
> void
> Index: normal/parser.y
> ===================================================================
> RCS file: /sources/grub/grub2/normal/parser.y,v
> retrieving revision 1.6
> diff -u -r1.6 parser.y
> --- normal/parser.y 4 Jun 2006 15:56:55 -0000 1.6
> +++ normal/parser.y 25 Jun 2007 11:43:58 -0000
> @@ -46,9 +46,9 @@
> %token GRUB_PARSER_TOKEN_VAR
> %type <cmd> script grubcmd command commands commandblock menuentry if
> %type <arglist> arguments;
> -%type <arg> argument;
> +%type <arg> argument GRUB_PARSER_TOKEN_VAR;
> %type <string> "if" "while" "function" "else" "then" "fi"
> -%type <string> text GRUB_PARSER_TOKEN_NAME GRUB_PARSER_TOKEN_VAR
> +%type <string> text GRUB_PARSER_TOKEN_NAME
>
> %pure-parser
> %lex-param { struct grub_parser_param *state };
> @@ -86,7 +86,7 @@
> for example: `foo${bar}baz'. */
> argument: GRUB_PARSER_TOKEN_VAR
> {
> - $$ = grub_script_arg_add (state, 0,
> GRUB_SCRIPT_ARG_TYPE_VAR, $1);
> + $$ = $1;
> }
> | text
> {
> Index: kern/parser.c
> ===================================================================
> RCS file: /sources/grub/grub2/kern/parser.c,v
> retrieving revision 1.2
> diff -u -r1.2 parser.c
> --- kern/parser.c 23 Nov 2005 03:36:24 -0000 1.2
> +++ kern/parser.c 25 Jun 2007 11:43:58 -0000
> @@ -44,6 +44,7 @@
> { GRUB_PARSER_STATE_VAR, GRUB_PARSER_STATE_VARNAME2, '{', 0},
> { GRUB_PARSER_STATE_VAR, GRUB_PARSER_STATE_VARNAME, 0, 1},
> { GRUB_PARSER_STATE_VARNAME, GRUB_PARSER_STATE_TEXT, ' ', 1},
> + { GRUB_PARSER_STATE_VARNAME, GRUB_PARSER_STATE_TEXT, '/', 1},
> { GRUB_PARSER_STATE_VARNAME2, GRUB_PARSER_STATE_TEXT, '}', 0},
>
> { GRUB_PARSER_STATE_QVAR, GRUB_PARSER_STATE_QVARNAME2, '{', 0},
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
--
Marco
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
