| Message ID | 20111031145311.13723.63607.stgit@s20.home |
|---|---|
| State | New |
| Headers | show |
Alex Williamson <alex.williamson@redhat.com> writes: > Spotted via code review, we initialize offset to 0 to avoid a > compiler warning, but in the unlikely case that offset is > never set to something else, we should abort instead of return > a value that will almost certainly cause problems. Compiler warning pointed to the problem until commit 09d7ae90 "Fix warning about uninitialized variable" papered over it. > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > exec.c | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index 9dc4edb..70f6fb8 100644 > --- a/exec.c > +++ b/exec.c > @@ -2874,7 +2874,7 @@ static void *file_ram_alloc(RAMBlock *block, > static ram_addr_t find_ram_offset(ram_addr_t size) > { > RAMBlock *block, *next_block; > - ram_addr_t offset = 0, mingap = RAM_ADDR_MAX; > + ram_addr_t offset = RAM_ADDR_MAX, mingap = RAM_ADDR_MAX; > > if (QLIST_EMPTY(&ram_list.blocks)) > return 0; > @@ -2890,10 +2890,17 @@ static ram_addr_t find_ram_offset(ram_addr_t size) > } > } > if (next - end >= size && next - end < mingap) { > - offset = end; > + offset = end; > mingap = next - end; > } > } > + > + if (offset == RAM_ADDR_MAX) { > + fprintf(stderr, "Failed to find gap of requested size: %" PRIu64 "\n", > + (uint64_t)size); > + abort(); > + } > + > return offset; > } The loop can't yield offset RAM_ADDR_MAX, because size needs to be zero for next - end >= size to succeed, and that's not possible. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 10/31/2011 09:54 AM, Alex Williamson wrote: > Spotted via code review, we initialize offset to 0 to avoid a > compiler warning, but in the unlikely case that offset is > never set to something else, we should abort instead of return > a value that will almost certainly cause problems. > > Signed-off-by: Alex Williamson<alex.williamson@redhat.com> Applied. Thanks. Regards, Anthony Liguori > --- > > exec.c | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index 9dc4edb..70f6fb8 100644 > --- a/exec.c > +++ b/exec.c > @@ -2874,7 +2874,7 @@ static void *file_ram_alloc(RAMBlock *block, > static ram_addr_t find_ram_offset(ram_addr_t size) > { > RAMBlock *block, *next_block; > - ram_addr_t offset = 0, mingap = RAM_ADDR_MAX; > + ram_addr_t offset = RAM_ADDR_MAX, mingap = RAM_ADDR_MAX; > > if (QLIST_EMPTY(&ram_list.blocks)) > return 0; > @@ -2890,10 +2890,17 @@ static ram_addr_t find_ram_offset(ram_addr_t size) > } > } > if (next - end>= size&& next - end< mingap) { > - offset = end; > + offset = end; > mingap = next - end; > } > } > + > + if (offset == RAM_ADDR_MAX) { > + fprintf(stderr, "Failed to find gap of requested size: %" PRIu64 "\n", > + (uint64_t)size); > + abort(); > + } > + > return offset; > } > > > >
diff --git a/exec.c b/exec.c index 9dc4edb..70f6fb8 100644 --- a/exec.c +++ b/exec.c @@ -2874,7 +2874,7 @@ static void *file_ram_alloc(RAMBlock *block, static ram_addr_t find_ram_offset(ram_addr_t size) { RAMBlock *block, *next_block; - ram_addr_t offset = 0, mingap = RAM_ADDR_MAX; + ram_addr_t offset = RAM_ADDR_MAX, mingap = RAM_ADDR_MAX; if (QLIST_EMPTY(&ram_list.blocks)) return 0; @@ -2890,10 +2890,17 @@ static ram_addr_t find_ram_offset(ram_addr_t size) } } if (next - end >= size && next - end < mingap) { - offset = end; + offset = end; mingap = next - end; } } + + if (offset == RAM_ADDR_MAX) { + fprintf(stderr, "Failed to find gap of requested size: %" PRIu64 "\n", + (uint64_t)size); + abort(); + } + return offset; }
Spotted via code review, we initialize offset to 0 to avoid a compiler warning, but in the unlikely case that offset is never set to something else, we should abort instead of return a value that will almost certainly cause problems. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- exec.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)