Hi, This is the bug fix for command/loadenv.c. Here is a small program to illustrate what happen:
#include <stdio.h>
#include <stdlib.h>
struct data
{
struct data *next;
int length;
};
struct data *
new_data ()
{
struct data *p;
p = malloc (sizeof (*p));
p->length = 1;
p->next = 0;
return p;
}
int
main ()
{
int index;
struct data *head, *p;
head = new_data ();
for (p = head, index = 0; p; p = p->next, index += p->length)
printf ("%d\n", index);
return 0;
}
Compile it with gcc-4.0 from OSX:
i686-apple-dawin8-gcc-4.0.1 -Os test.c
The assembly dump shows:
L4:
movl %edi, 4(%esp)
movl -28(%ebp), %eax
movl %eax, (%esp)
call L_printf$stub
movl (%esi), %esi
addl 4(%esi), %edi
jmp L4
Why would it uses unconditional jumps ? I think it's some over
optimization in gcc. We have index += p->length after p = p->next, if
p is null, p->length would be invalid, so gcc figure that p would
never be null and the end condition is always false. In fact, the bug
is in the code itself, even if it tests p, in OS environment, access
null pointer is not allowed, so index += p->length can't be executed
anyway.
The correct way is:
for (p = head, index = 0; p;)
{
printf ("%d\n", index);
p = p->next;
if (p)
index += p->length;
}
--
Bean
gitgrub home: http://github.com/grub/grub/
my fork page: http://github.com/bean123/grub/
loadenv.diff
Description: Binary data
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
