On Sun, May 3, 2009 at 2:52 PM, Bean <bean12...@gmail.com> wrote:
> On Sun, May 3, 2009 at 8:39 PM, Vladimir 'phcoder' Serbinenko
> <phco...@gmail.com> wrote:
> >
> >
> > 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
>
> 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
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'phcoder' Serbinenko
diff --git a/kern/parser.c b/kern/parser.c
index 685ab22..ee6b169 100644
--- a/kern/parser.c
+++ b/kern/parser.c
@@ -47,8 +47,8 @@ static struct grub_parser_state_transition state_transitions[] =
{ GRUB_PARSER_STATE_QVAR, GRUB_PARSER_STATE_QVARNAME2, '{', 0},
{ GRUB_PARSER_STATE_QVAR, GRUB_PARSER_STATE_QVARNAME, 0, 1},
- { GRUB_PARSER_STATE_QVARNAME, GRUB_PARSER_STATE_DQUOTE, ' ', 1},
{ GRUB_PARSER_STATE_QVARNAME, GRUB_PARSER_STATE_TEXT, '\"', 0},
+ { GRUB_PARSER_STATE_QVARNAME, GRUB_PARSER_STATE_DQUOTE, ' ', 1},
{ GRUB_PARSER_STATE_QVARNAME2, GRUB_PARSER_STATE_DQUOTE, '}', 0},
{ 0, 0, 0, 0}
@@ -60,9 +60,7 @@ grub_parser_state_t
grub_parser_cmdline_state (grub_parser_state_t state, char c, char *result)
{
struct grub_parser_state_transition *transition;
- struct grub_parser_state_transition *next_match = 0;
struct grub_parser_state_transition default_transition;
- int found = 0;
default_transition.to_state = state;
default_transition.keep_value = 1;
@@ -70,26 +68,24 @@ grub_parser_cmdline_state (grub_parser_state_t state, char c, char *result)
/* Look for a good translation. */
for (transition = state_transitions; transition->from_state; transition++)
{
+ if (transition->from_state != state)
+ continue;
/* An exact match was found, use it. */
- if (transition->from_state == state && transition->input == c)
- {
- found = 1;
- break;
- }
+ if (transition->input == c)
+ break;
+
+ if (transition->input == ' ' && ! grub_isalpha (c)
+ && ! grub_isdigit (c) && c != '_')
+ break;
/* A less perfect match was found, use this one if no exact
match can be found. */
- if (transition->from_state == state && transition->input == 0)
- next_match = transition;
+ if (transition->input == 0)
+ break;
}
- if (! found)
- {
- if (next_match)
- transition = next_match;
- else
- transition = &default_transition;
- }
+ if (! transition->from_state)
+ transition = &default_transition;
if (transition->keep_value)
*result = c;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
