Re: a bug in read

[Javier Martín]

Couldn't this have been worked around changing while to do-while and
refactoring the loop instead of creating a new variable, whose
handling takes space? I'm not sure it can be done because I'm on
vacation and reading mail through dialup access, but it might be worth
a try.

2008/7/9, Pavel Roskin <address@hidden>:
> On Tue, 2008-07-08 at 00:18 +0200, Yoshinori K. Okuji wrote:
> > Hello,
> >
> > I have noticed that read.c has a bug. In this line:
> >
> >   while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
> >
> > LINE is not initialized yet at the first time, so this refers to a
> > uninitialized location.
>
> Thank you!  What's worse, i is 0, so we are reading outside the buffer.
> I think this patch should do what the code was meant to do:
>
> diff --git a/commands/read.c b/commands/read.c
> index 1995918..96519f8 100644
> --- a/commands/read.c
> +++ b/commands/read.c
> @@ -30,15 +30,16 @@ grub_getline (void)
>   int i;
>   char *line;
>   char *tmp;
> +  char last = 0;
>
>   i = 0;
>   line = grub_malloc (1 + i + sizeof('\0'));
>   if (! line)
>     return NULL;
>
> -  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
> +  while ((last != '\n') && (last != '\r'))
>     {
> -      line[i] = grub_getkey ();
> +      last = line[i] = grub_getkey ();
>       if (grub_isprint (line[i]))
>        grub_putchar (line[i]);
>       i++;
>
>
> We should test all grub utilities in Valgrind to find such problems.
>
> By the way, read is not a part of grub-emu.  We'll need to improve the
> build system to make such oversights less likely.  We also need "exit"
> in grub-emu, as "reboot" doesn't sound right.
>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>