[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/9] eepro100: Support compilation without EEPRO
Re: [Qemu-devel] [PATCH 6/9] eepro100: Support compilation without EEPROM
Tue, 06 Apr 2010 09:35:20 -0700
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:220.127.116.11) Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4
On 04/06/2010 09:01 AM, Stefan Weil wrote:
> Richard Henderson schrieb:
>> On 04/06/2010 04:44 AM, Stefan Weil wrote:
>>> +#if EEPROM_SIZE > 0
>>> /* Add 64 * 2 EEPROM. i82557 and i82558 support a 64 word EEPROM,
>>> * i82559 and later support 64 or 256 word EEPROM. */
>>> s->eeprom = eeprom93xx_new(EEPROM_SIZE);
> Why do you think that a C if is better than a CPP if
> in this case?
> Some compilers give warnings for code which will
> never be reached. Try gcc -Wunreachable-code.
> It's a really useful option which sometimes even
> detects programming errors, so maybe it would
> be good for qemu, too.
Having the compiler detect syntax and type mismatch errors in all
code paths is generally far more valuable than warnings for unreachable
code. These tend to creep in very easily with ifdef'ed code.
This has follow-on effects as well. Suppose this instance above is
the only place that eeprom93xx_new is called. With an ifdef here at
the use site, the compiler will complain that eeprom93xx_new is unused,
leading you to introduce more ifdefs, which means that even more code
If you use a C if, then the compiler will see that eeprom93xx_new
is used under other circumstances, not complain, and eliminate
it as unused -- after having verified that it is both syntactically
and semantically correct.
Preprocessor spaghetti code is extremely hard to read. I know that
QEMU is chock full of it, but let's try to reduce that rather than
[Qemu-devel] [PATCH 4/9] eepro100: Add new device variant i82801, Stefan Weil, 2010/04/06
[Qemu-devel] [PATCH 3/9] eepro100: Simplified device instantiation, Stefan Weil, 2010/04/06
[Qemu-devel] [PATCH 6/9] eepro100: Support compilation without EEPROM, Stefan Weil, 2010/04/06
Re: [Qemu-devel] [PATCH 6/9] eepro100: Support compilation without EEPROM, Paul Brook, 2010/04/06
[Qemu-devel] [PATCH 5/9] eepro100: Set configuration bit for standard TCB, Stefan Weil, 2010/04/06
[Qemu-devel] [PATCH 7/9] eepro100: Set power management capability using pci_reserve_capability, Stefan Weil, 2010/04/06
[Qemu-devel] [PATCH 8/9] eepro100: Fix mapping of flash memory, Stefan Weil, 2010/04/06
- [Qemu-devel] Re: [PATCH 2/9] eepro100: Simplify status handling, (continued)