Bean <[EMAIL PROTECTED]> writes:
> 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.
Ah, ok. Anyways, it looks fine to me in this context.
>> > /* 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.
Right, but do you want to see a menu if not everything is correctly executed?
>>
>> > /* 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 ?
Hopefully someone else knows a better name? :-)
>>
>> 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.
Ah ok, so there is a grub_script_lexer_record_start elsewhere that
forfills the same role?
> 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.
Of deref? Can you explain? The idea of commandblock was that this
structure might show up more often. But I have no objections for now
if a bug is fixed this way.
Btw, does this patch incorporate the previous patch you sent in
regarding scripting?
--
Marco
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
