[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] Bug fix for loadenv.c
From: |
Bean |
Subject: |
[PATCH] Bug fix for loadenv.c |
Date: |
Fri, 4 Sep 2009 03:20:30 +0800 |
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
- [PATCH] Bug fix for loadenv.c,
Bean <=