On Fri, Jul 31, 2009 at 6:17 AM, Pavel Roskin<pro...@gnu.org> wrote:
> On Fri, 2009-07-31 at 00:46 +0200, Vladimir 'phcoder' Serbinenko wrote:
>> This patch fixes the parsing of two strings like following ones:
>> "echo 1 " was parsed into "echo", "1", ""
>> "echo $root" was parsed into "echo" (variable just disappeared)
>
> It would be helpful if you explain how to see the difference without
> tracing grub_parser_split_cmdline() in the debugger.
cat /hello
error: file name required
echo $root
Empty line
>
> Also, it would be great if you explain the change. A comment for the
> newly added code would help understand it. Otherwise it looks like the
> previous comment still applies ("A special case for when the last
> character was part of a variable").
>
Sorry I forgot to move the comment together with code
> Since you looked at the problem, perhaps you know why argc is
> decremented before the exit. I think it needs a comment.
It's because till that point argc was count of tokens and afterwards
it's a count of argument. Example
"echo hello" gives 2 tokes "echo" and "hello" but command "echo"
recieves only one argument: "hello". But probably this should be moved
to ./kern/rescue_parser.c, ./script/lua/grub_lib.c and
./normal/completion.c (3 callers of this function)
>
> Also, grub_malloc() appears to allocate two extra pointers for argv (if
> we consider that argc is decremented). argv is not supposed to be null
> terminated. I'd rather allocate just enough memory so that we could
> catch abusers by running grub-emu in valgrind.
>
> Anyway, the patch doesn't pass even minimal testing. Pressing Tab in
> grub-emu crashes it at normal/completion.c:424
>
Ok. I looked into it. I triggered another bug. When someone parses any
empty string he gets 0 tokens which means -1 arguments. Here is a fix:
diff --git a/kern/rescue_parser.c b/kern/rescue_parser.c
index 79f32b8..a93ca36 100644
--- a/kern/rescue_parser.c
+++ b/kern/rescue_parser.c
@@ -35,6 +35,9 @@ grub_rescue_parse_line (char *line,
grub_reader_getline_t getline)
if (grub_parser_split_cmdline (line, getline, &n, &args) || n < 0)
return grub_errno;
+ if (n == -1)
+ return GRUB_ERR_NONE;
+
/* In case of an assignment set the environment accordingly
instead of calling a function. */
if (n == 0 && grub_strchr (line, '='))
diff --git a/normal/completion.c b/normal/completion.c
index 405976a..5c2e0d1 100644
--- a/normal/completion.c
+++ b/normal/completion.c
@@ -399,14 +399,18 @@ grub_normal_do_completion (char *buf, int *restore,
if (grub_parser_split_cmdline (buf, 0, &argc, &argv))
return 0;
-
- current_word = argv[argc];
+
+ if (argc == -1)
+ current_word = "";
+ else
+ current_word = argv[argc];
+
/* Determine the state the command line is in, depending on the
state, it can be determined how to complete. */
cmdline_state = get_state (buf);
- if (argc == 0)
+ if (argc == 0 || argc == -1)
{
/* Complete a command. */
if (grub_command_iterate (iterate_command))
@@ -476,13 +480,15 @@ grub_normal_do_completion (char *buf, int *restore,
goto fail;
}
- grub_free (argv[0]);
+ if (argc != -1)
+ grub_free (argv[0]);
grub_free (match);
return ret;
}
fail:
- grub_free (argv[0]);
+ if (argc != -1)
+ grub_free (argv[0]);
grub_free (match);
grub_errno = GRUB_ERR_NONE;
--
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
