Re: [Qemu-devel] [RFC 16/19] target-ppc: Refactor debug output macros

[Alexander Graf]

On 27.01.2013, at 15:35, Andreas Färber wrote:

> Am 27.01.2013 15:14, schrieb Anthony Liguori:
>> Andreas Färber <address@hidden> writes:
>>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>>> index 0a1ac86..54722c4 100644
>>> --- a/target-ppc/excp_helper.c
>>> +++ b/target-ppc/excp_helper.c
>>> @@ -21,14 +21,14 @@
>>> 
>>> #include "helper_regs.h"
>>> 
>>> -//#define DEBUG_OP
>>> -//#define DEBUG_EXCEPTIONS
>>> +#define DEBUG_OP 0
>>> +#define DEBUG_EXCEPTIONS 0
>>> 
>>> -#ifdef DEBUG_EXCEPTIONS
>>> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
>>> -#else
>>> -#  define LOG_EXCP(...) do { } while (0)
>>> -#endif
>>> +#define LOG_EXCP(...) G_STMT_START \
>>> +    if (DEBUG_EXCEPTIONS) { \
>>> +        qemu_log(__VA_ARGS__); \
>>> +    } \
>>> +    G_STMT_END
>> 
>> Just thinking out loud a bit..  This form becomes pretty common and it's
>> ashame to use a macro here if we don't have to.
>> 
>> I think:
>> 
>> static inline void LOG_EXCP(const char *fmt, ...)
>> {
>>    if (debug_exceptions) {
>>       va_list ap;
>>       va_start(ap, fmt);
>>       qemu_logv(fmt, ap);
>>       va_end(ap);
>>    }
>> }
>> 
>> Probably would have equivalent performance.  debug_exception would be
>> read-mostly and ought to be very predictable as a result.  I strongly
>> expect that the compiler would actually inline LOG_EXCP too.
> 
> Thanks for your early feedback. I merely tried to stay close to the
> original code. I wouldn't mind inline functions either. Or even more
> harmonization for that matter.

I fully agree. Just recently Scott revamped the openpic debug print code:


//#define DEBUG_OPENPIC

#ifdef DEBUG_OPENPIC
static const int debug_openpic = 1;
#else
static const int debug_openpic = 0;
#endif

#define DPRINTF(fmt, ...) do { \
        if (debug_openpic) { \
            printf(fmt , ## __VA_ARGS__); \
        } \
    } while (0)

I like that approach. It keeps all users identical. The #define stays 
identical. The callers stay identical. But we do get proper compile time 
checks. Of course Anthony's approach works too, but the thing I'd definitely 
like to see is that the #defines don't become numerical, but rather stay of an 
#ifdef basis.


Alex