Re: [Qemu-devel] [PATCH 6/9] eepro100: Support compilation without EEPROM

[Richard Henderson]

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);
>>> +#endif
>>>     
...
> 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
is unchecked.

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
introduce more.


r~