| Message ID | AANLkTim-7Pj=S+pCDD-knA=usWoCWZjVxZBm9ughfVYZ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Blue Swirl <blauwirbel@gmail.com> 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 don't like sweeping bugs under the carpet like that. The initial value is used when and only when the emulation is buggy. We doubt it can happen. If we truly believe it can't happen, assert it. If we just doubt it, log it.
Am 11.10.2010 16:53, schrieb Markus Armbruster: > Blue Swirl <blauwirbel@gmail.com> 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 don't like sweeping bugs under the carpet like that. The initial > value is used when and only when the emulation is buggy. We doubt it > can happen. If we truly believe it can't happen, assert it. If we just > doubt it, log it. Markus, that patch would only be an intermediate solution which helps to fix a certain class of compiler warnings. I already promised to test the code with assertions and started doing so (see my qemu repository http://repo.or.cz/w/qemu/ar7.git/history/HEAD:/hw/eepro100.c). Testing takes some time, so the intermediate solution can be reasonable. But nothing will be swept under the carpet!
Stefan Weil <weil@mail.berlios.de> writes: > Am 11.10.2010 16:53, schrieb Markus Armbruster: >> Blue Swirl <blauwirbel@gmail.com> 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 don't like sweeping bugs under the carpet like that. The initial >> value is used when and only when the emulation is buggy. We doubt it >> can happen. If we truly believe it can't happen, assert it. If we just >> doubt it, log it. > > Markus, that patch would only be an intermediate solution > which helps to fix a certain class of compiler warnings. > > I already promised to test the code with assertions > and started doing so (see my qemu repository > http://repo.or.cz/w/qemu/ar7.git/history/HEAD:/hw/eepro100.c). > Testing takes some time, so the intermediate solution > can be reasonable. > > But nothing will be swept under the carpet! An intermediate solution could use a comment, but as long as you take care of the real solution, it's not that important.
diff --git a/hw/eepro100.c b/hw/eepro100.c index 2b75c8f..adc579f 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -1282,7 +1282,7 @@ static void eepro100_write_port(EEPRO100State * s, uint32_t val) static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr) { - uint8_t val; + uint8_t val = 0; if (addr <= sizeof(s->mem) - sizeof(val)) { memcpy(&val, &s->mem[addr], sizeof(val));
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. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- hw/eepro100.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) } @@ -1325,7 +1325,7 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr) static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr) { - uint16_t val; + uint16_t val = 0; if (addr <= sizeof(s->mem) - sizeof(val)) { memcpy(&val, &s->mem[addr], sizeof(val)); } @@ -1348,7 +1348,7 @@ static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr) static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr) { - uint32_t val; + uint32_t val = 0; if (addr <= sizeof(s->mem) - sizeof(val)) { memcpy(&val, &s->mem[addr], sizeof(val)); }