grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [BUGFIX] lexer waits for space to terminate the varname


From: Bean
Subject: Re: [BUGFIX] lexer waits for space to terminate the varname
Date: Sun, 3 May 2009 20:37:13 +0800

Hi,

On Sun, May 3, 2009 at 5:12 PM, Vladimir 'phcoder' Serbinenko
<address@hidden> wrote:
> Hello
>
> On Sat, May 2, 2009 at 4:08 PM, Bean <address@hidden> wrote:
>>
>> Hi,
>>
>> I think there is problem with this patch. Consider ${aa}, the closing
>> character "}" would be left out.
>>
>> Although you can remedy this by swapping:
>>
>>  { GRUB_PARSER_STATE_VARNAME, GRUB_PARSER_STATE_TEXT, ' ', 1},
>>  { GRUB_PARSER_STATE_VARNAME2, GRUB_PARSER_STATE_TEXT, '}', 0},
>
> It's not the same starting state. However similar issue exists with ' ' and
> '\"'

Oh right, I copy the wrong sample.

>>
>> so that '}' would be handled before ' ', but it's still not a good
>> practice, as you have added a logic in code that would depend on the
>> order of state_transitions, which is not apparent for casual code
>> viewer.
>
> Perhaps we should switch to just ordering the transition rules instead of
> separate code for default value
>>
>> Also, ' ' might be used for other transition in the future,
>> this code could break that. I suggest you use {} to enclose the
>> variable that doesn't terminated with space,
>
> It can be a workaround  but the bug is still here. The following pieces of
> code fail:
> 1) echo $hello;
> 2) echo $hello
>

Yeah, and there are other varieties as well, such as -. Perhaps it'd
be simpler to use your fix for now, but you should add some comment in
the source code to indicate its purpose.

>> or add the terminating
>> character explicitly in the transition table:
>>
>>
>>  { GRUB_PARSER_STATE_VARNAME, GRUB_PARSER_STATE_TEXT, '/ ', 1},
>>
>>
>> On Sat, May 2, 2009 at 8:02 PM, Vladimir 'phcoder' Serbinenko
>> <address@hidden> wrote:
>> > Hello. A varname may be terminated by any character which isn't in a set
>> > [A-Za-z0-9_] and not only space. Here is a fix
>> >
>> > --
>> > Regards
>> > Vladimir 'phcoder' Serbinenko
>> >
>> > _______________________________________________
>> > Grub-devel mailing list
>> > address@hidden
>> > http://lists.gnu.org/mailman/listinfo/grub-devel
>> >
>> >
>>
>>
>>
>> --
>> Bean
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
> --
> Regards
> Vladimir 'phcoder' Serbinenko
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



-- 
Bean




reply via email to

[Prev in Thread] Current Thread [Next in Thread]