[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all ca
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all cases |
Date: |
Thu, 7 Oct 2010 17:54:23 +0000 |
On Thu, Oct 7, 2010 at 9:31 AM, Markus Armbruster <address@hidden> wrote:
> Blue Swirl <address@hidden> writes:
>
>> Compiling with GCC 4.6.0 20100925 produced warnings:
>> /src/qemu/hw/eepro100.c: In function 'eepro100_read4':
>> /src/qemu/hw/eepro100.c:1351:14: error: 'val' may be used
>> uninitialized in this function [-Werror=uninitialized]
>> /src/qemu/hw/eepro100.c: In function 'eepro100_read2':
>> /src/qemu/hw/eepro100.c:1328:14: error: 'val' may be used
>> uninitialized in this function [-Werror=uninitialized]
>> /src/qemu/hw/eepro100.c: In function 'eepro100_read1':
>> /src/qemu/hw/eepro100.c:1285:13: error: 'val' may be used
>> uninitialized in this function [-Werror=uninitialized]
>>
>> Fix by initializing 'val' at start.
>
> I'm worried this sweeps bugs under the carpet.
>
> When addr is out of bounds, these function return garbage. Your patch
> makes them return 0 instead. Can that happen? Shouldn't we catch and
> flag it?
0 is a nice default value.
Adding an assert is not OK, guest shouldn't be able to terminate QEMU.
Adding a check which spams stdio is not so nice either.
I think it's even impossible with current QEMU for addr to be out of
bounds, the area is registered with size PCI_MEM_SIZE.