Re: [Qemu-devel] [PATCH] SH7750/51: add register BCR3, NCR4, PCR, RTCOR, RTCNT, RTCSR, SDMR2, SDMR3 and fix BCR2 support

[Thiemo Seufer <ths@networkno.de>]

Shin-ichiro KAWASAKI wrote:
[snip]
>>>>>> Compared to 'if' statement, 'switch-case' might be more easy to
>>>>>> understand, like as follows.
>>>>>>   case SH7750_SDMR2 ... SH7750_SDMR2 + SDMR2_REGNB
>>>>> ok elvenif it's a gcc ony extension
>>>> Ah, gccism policy seems not clear in QEMU project.
>>>> This extension is used in many hw/*.c, so I guessed it
>>>> as a QEMU's usual implementation style.
>>>> I hope comments on it from main developers.
>>>>
>>> Yes it will be nice
>>
>> I believe "no new gcc-isms" would be a good policy.
>
> Thank you.  I'll follow it.

Be aware that this was only my personal opinion, and not well-stated at
that. From the recent discussion on IRC I believe we have now the rough
consensus:

"Use GCC extensions only when it improves the code significantly".

> So, my suggestion about 'if' vs 'switch-case' is not appropriate.

For the specific patch I believe the 'if' version is better. My opinion
is largely formed by the 'SH7750_SDMR2 + SDMR2_REGNB' term: If this
state doesn't deserve its own name, then it is probably not clear enough
to be used in a case statement.


Thiemo