On Sun, May 3, 2009 at 2:37 PM, Bean <bean12...@gmail.com> wrote:
> Hi,
>
> On Sun, May 3, 2009 at 5:12 PM, Vladimir 'phcoder' Serbinenko
> <phco...@gmail.com> wrote:
> > Hello
> >
> > On Sat, May 2, 2009 at 4:08 PM, Bean <bean12...@gmail.com> 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
>
>
> >> 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
> >> <phco...@gmail.com> 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
> >> > Grub-devel@gnu.org
> >> > http://lists.gnu.org/mailman/listinfo/grub-devel
> >> >
> >> >
> >>
> >>
> >>
> >> --
> >> Bean
> >>
> >>
> >> _______________________________________________
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> http://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> >
> > --
> > Regards
> > Vladimir 'phcoder' Serbinenko
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > http://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
>
>
>
> --
> Bean
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'phcoder' Serbinenko
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
