On Jan 14, 2008 2:42 AM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> > @@ -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?
here is the function :
/* Lookup the command. */
grubcmd = grub_command_find (cmdline->cmdname);
if (! grubcmd)
{
/* Ignore errors. */
+ grub_errno = GRUB_ERR_NONE;
/* It's not a GRUB command, try all functions. */
func = grub_script_function_find (cmdline->cmdname);
if (! func)
{
if the command is not found, grub_errno would be set, this will effect
the later grub_script_function_find when it try to load the module
from disk.
> > /* 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.
if grub_error is set, it will effect the parser, caused menu not to be
showed, etc.
>
> > /* 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".
what do you suggest ?
>
> 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.
I split commandblock from menuentry, now it's a standalone statement.
commandblock: '{'
{
grub_script_lexer_ref (state->lexerstate);
}
newlines commands '}'
{
grub_script_lexer_deref (state->lexerstate);
$$ = $4;
}
>
> > }
> > 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?
I don't like the idea of deref in another statement, and now we can
use {} separately.
--
Bean
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
