Re: [Qemu-devel] [RFC 10/19] target-alpha: Refactor debug output macros

[Andreas Färber]

Am 29.01.2013 10:56, schrieb Peter Maydell:
> On 29 January 2013 08:42, Andreas Färber <address@hidden> wrote:
>> Analysing the reasons for the obscured suggestion:
>>
>> a) "if (foo) MACRO1(); else MACRO2();" is forbidden by Coding Style.
>> Thus, if careful review indicates there are no such Coding Style
>> violations, it is entirely possible not to add any
>> fault-that-may-not-happen-obscuring macro statements.
> 
> This requires analysis of all the callsites (including any that
> may be incoming in not-yet-applied patchsets) -- that's a lot
> harder and more error prone than just getting the macro right
> in the first place.

What I said. Converting these macros required me to review any use of
the defines anyway for possible #ifdef -> #if conversion (which we have
decided against already).

>> b) Working around an issue resulting from hiding C statements inside a
>> preprocessor macro is totally backwards compared to properly using the C
>> language in the first place. Its mechanism for reuse are functions, and
>> for performance considerations static inline functions.
>>
>> Therefore I disagree with you that b) is not an entirely different
>> disussion as you repeatedly suggest
> 
> It clearly is, because *you didn't submit a patch using an inline
> function*.

Then spare your virtual breath and see my reply to Anthony. I clearly
indicated that I intend to adopt his and Alex' proposal in a v2, so this
discussion you're leading here about my v1 patch (for which this was
suggested to me!) is pointless, we are debating general statements and
opinions only.

My opinion is that do { ... } while (0) is stupid in light of other
options, and it is unlikely that anything you reply here will change my
feelings.

> The whole reason we're having this argument is that you
> submitted a patch that uses a multistatement macro. Feel free
> to resubmit something using an inline function if you like. Once
> again:
> 
>  C macro with do..while(0): OK
>  inline function: also OK
>  C macro missing do..while(0) protection: not OK

Disagree, it depends. For random code "not OK".

>  C macro using glib obfuscated macros: also not OK

That's your opinion, possibly that of the majority of QEMU developers,
but not a natural fact if you put it.
Like I said, I don't see much difference to do { ... } while (0).

> This really doesn't seem particularly complicated to me.

No it isn't and it's going nowhere. I would rather spend my time fixing
issues for 1.4 and you haven't reviewed the ARM CPU type rename yet either.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg