[Top][All Lists]
[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 21:38:20 +0800 |
Hi,
Oh right, it looks more compact. Although you should test the corner
cases to make sure no problem is introduced by the new code.
On Sun, May 3, 2009 at 9:28 PM, Vladimir 'phcoder' Serbinenko
<address@hidden> wrote:
>
> On Sun, May 3, 2009 at 2:52 PM, Bean <address@hidden> wrote:
>>
>> On Sun, May 3, 2009 at 8:39 PM, Vladimir 'phcoder' Serbinenko
>> <address@hidden> wrote:
>> >
>> >
>> > On Sun, May 3, 2009 at 2:37 PM, Bean <address@hidden> wrote:
>> >>
>> >> 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.
>> >
>> > What about making it rely on order of rules? It should make it more
>> > compact
>>
>> Hi,
>>
>> I'm sorry I don't quite follow, could you give an example ?
>
> Something like this. Compile tested only
>
>>
>> --
>> 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