Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all cases

[Blue Swirl]

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.